Skip to content

Commit b154553

Browse files
committed
fix: review feedback — shared locale validation, tuner error reporting
- Extract extract_locale_from_filename() to util.rs with languages.json validation (prevents false positives like "foo.setup.md" → "setup") - Remove duplicate implementations from corpus.rs and language.rs - Add failure counter to parallel tuner — surfaces error count in summary instead of silently dropping failed ways
1 parent 9693556 commit b154553

4 files changed

Lines changed: 37 additions & 49 deletions

File tree

tools/ways-cli/src/cmd/corpus.rs

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ fn scan_ways_dir(dir: &Path, id_prefix: &str, excluded: &[String], w: &mut impl
188188
}
189189

190190
// Detect locale override files ({name}.{lang}.md)
191-
if let Some(lang) = detect_locale_in_filename(fname) {
191+
if let Some(lang) = crate::util::extract_locale_from_filename(fname) {
192192
if let Some(parent) = path.parent() {
193193
locale_overrides.insert((parent.to_path_buf(), lang));
194194
}
@@ -271,25 +271,6 @@ fn scan_ways_dir(dir: &Path, id_prefix: &str, excluded: &[String], w: &mut impl
271271
Ok(count)
272272
}
273273

274-
/// Detect a locale code in a filename like "security.ja.md" → Some("ja")
275-
fn detect_locale_in_filename(filename: &str) -> Option<String> {
276-
if filename.contains(".check.") {
277-
return None;
278-
}
279-
let stem = filename.strip_suffix(".md")?;
280-
let parts: Vec<&str> = stem.split('.').collect();
281-
if parts.len() >= 2 {
282-
let candidate = parts[parts.len() - 1];
283-
if candidate.len() >= 2
284-
&& candidate.len() <= 5
285-
&& candidate.chars().all(|c| c.is_ascii_lowercase() || c == '-')
286-
{
287-
return Some(candidate.to_string());
288-
}
289-
}
290-
None
291-
}
292-
293274
/// Find the BM25 threshold from the parent way's frontmatter.
294275
fn find_parent_threshold(dir: &Path) -> f64 {
295276
if let Ok(entries) = std::fs::read_dir(dir) {

tools/ways-cli/src/cmd/language.rs

Lines changed: 1 addition & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ fn scan_way_dirs(
210210
}
211211

212212
// Check for locale override files: {name}.{lang}.md
213-
if let Some(locale) = extract_locale(&fname) {
213+
if let Some(locale) = crate::util::extract_locale_from_filename(&fname) {
214214
locales.insert(locale.clone());
215215
all_locales.insert(locale);
216216
continue;
@@ -235,33 +235,6 @@ fn scan_way_dirs(
235235
Ok(())
236236
}
237237

238-
/// Extract locale code from filename like "security.ja.md" → "ja"
239-
/// Validates against languages.json to avoid false matches like ".check.md"
240-
fn extract_locale(filename: &str) -> Option<String> {
241-
// Skip check files explicitly
242-
if filename.contains(".check.") {
243-
return None;
244-
}
245-
246-
let parts: Vec<&str> = filename.strip_suffix(".md")?.split('.').collect();
247-
if parts.len() >= 2 {
248-
let candidate = parts[parts.len() - 1];
249-
// Validate it looks like a locale code (2-5 chars, lowercase/hyphen)
250-
if candidate.len() >= 2
251-
&& candidate.len() <= 5
252-
&& candidate.chars().all(|c| c.is_ascii_lowercase() || c == '-')
253-
{
254-
// Verify against languages.json
255-
let parsed: serde_json::Value =
256-
serde_json::from_str(agents::LANGUAGES_JSON).ok()?;
257-
if parsed.get("languages")?.as_object()?.contains_key(candidate) {
258-
return Some(candidate.to_string());
259-
}
260-
}
261-
}
262-
None
263-
}
264-
265238
/// Best-effort reverse lookup: language name → code
266239
fn resolve_to_code(lang: &str) -> String {
267240
let lower = lang.to_lowercase();

tools/ways-cli/src/cmd/tune.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ pub fn run(
133133
// Shared state
134134
let work_queue: Arc<Mutex<Vec<(String, PathBuf)>>> = Arc::new(Mutex::new(locale_files));
135135
let completed: Arc<Mutex<usize>> = Arc::new(Mutex::new(0));
136+
let failed: Arc<Mutex<usize>> = Arc::new(Mutex::new(0));
136137
let all_results: Arc<Mutex<Vec<WayTuneResult>>> = Arc::new(Mutex::new(Vec::new()));
137138

138139
// Spawn workers
@@ -141,6 +142,7 @@ pub fn run(
141142
let queue = Arc::clone(&work_queue);
142143
let results = Arc::clone(&all_results);
143144
let completed = Arc::clone(&completed);
145+
let failed = Arc::clone(&failed);
144146
let embed_bin = embed_bin.clone();
145147
let multi_corpus = multi_corpus.clone();
146148
let multi_model = multi_model.clone();
@@ -163,7 +165,9 @@ pub fn run(
163165
r.push(result);
164166
}
165167
Err(e) => {
166-
eprintln!("ERROR tuning {}: {}", way_id, e);
168+
eprintln!("\nERROR tuning {}: {}", way_id, e);
169+
let mut f = failed.lock().unwrap();
170+
*f += 1;
167171
}
168172
}
169173

@@ -183,6 +187,11 @@ pub fn run(
183187
}
184188
eprintln!();
185189

190+
let fail_count = *failed.lock().unwrap();
191+
if fail_count > 0 {
192+
eprintln!("WARNING: {} ways failed to tune", fail_count);
193+
}
194+
186195
// Collect and sort
187196
let mut way_results = Arc::try_unwrap(all_results)
188197
.map_err(|_| anyhow::anyhow!("failed to unwrap results"))

tools/ways-cli/src/util.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,31 @@ pub fn load_excluded_segments() -> Vec<String> {
4949
.unwrap_or_default()
5050
}
5151

52+
/// Extract a locale code from a filename like "security.ja.md" → Some("ja").
53+
/// Validates against languages.json to avoid false matches (e.g., "foo.setup.md").
54+
pub fn extract_locale_from_filename(filename: &str) -> Option<String> {
55+
if filename.contains(".check.") {
56+
return None;
57+
}
58+
let stem = filename.strip_suffix(".md")?;
59+
let parts: Vec<&str> = stem.split('.').collect();
60+
if parts.len() >= 2 {
61+
let candidate = parts[parts.len() - 1];
62+
if candidate.len() >= 2
63+
&& candidate.len() <= 5
64+
&& candidate.chars().all(|c| c.is_ascii_lowercase() || c == '-')
65+
{
66+
// Validate against languages.json
67+
let parsed: serde_json::Value =
68+
serde_json::from_str(crate::agents::LANGUAGES_JSON).ok()?;
69+
if parsed.get("languages")?.as_object()?.contains_key(candidate) {
70+
return Some(candidate.to_string());
71+
}
72+
}
73+
}
74+
None
75+
}
76+
5277
/// Check if a path should be excluded based on schema-defined segments.
5378
pub fn is_excluded_path(path: &Path, excluded_segments: &[String]) -> bool {
5479
let path_str = match path.to_str() {

0 commit comments

Comments
 (0)