langtool: Validate translated strings, and check AI output before writing it

Placeholders like %1 and %d have to survive translation intact, and they don't
always: the new "validate" command finds 37 strings across four language files
where one got dropped, localized into another script, or split with a space.
It also catches empty translations, line breaks and stray quotes, and exits
non-zero so it can be used as a check in a script.

The same checks now run on everything the AI returns, before it's written to a
file - plus a check that it didn't just echo the English string back at us. A
language that fails is skipped instead of aborting the whole run.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_0164UKoXpz9r155TRfQsoc3H

Tools/langtool/README.md

@@ -16,6 +16,18 @@ To see command line usage, type:
 cargo run -- --help
 ```
 
+## Validating
+
+```bash
+cargo run -- validate
+```
+
+Checks every translated string in every language file against the English one it came from:
+placeholders (`%1`, `%d`, ...) must survive translation, no line breaks or stray quotes, nothing
+empty. Exits with a non-zero code if it finds anything, so it can be used as a check in a script.
+
+The same checks run on anything the AI produces, before it gets written to a file.
+
 ## AI translation
 
 Some commands (`add-new-key-ai`, `add-new-key-value-ai`, `finish-language-with-ai`) use an LLM to

Tools/langtool/src/main.rs

@@ -14,6 +14,7 @@ mod claude;
 use clap::Parser;
 
 mod util;
+mod validate;
 
 use crate::{
     ai::{Ai, Provider},
@@ -123,6 +124,8 @@ enum Command {
         section: String,
         key: String,
     },
+    /// Check every translated string against the English one, see validate.rs.
+    Validate,
 }
 
 fn copy_missing_lines(
@@ -284,6 +287,33 @@ fn add_new_key(
     Ok(())
 }
 
+/// Runs the validation checks over a whole language file, printing what it finds.
+/// Returns the number of problems, so the caller can set the exit code.
+fn validate_ini(target_ini: &IniFile, reference_ini: &IniFile, language: &str) -> usize {
+    let mut count = 0;
+    for section in &target_ini.sections {
+        let Some(ref_section) = reference_ini.get_section(&section.name) else {
+            continue;
+        };
+        for line in &section.lines {
+            let Some((key, _)) = split_line(line) else {
+                continue;
+            };
+            let (Some(value), Some(ref_value)) = (section.get_value(key), ref_section.get_value(key))
+            else {
+                continue;
+            };
+            for issue in validate::check(&ref_value, &value) {
+                println!("{language} [{}] {key}: {issue}", section.name);
+                println!("      en: {ref_value}");
+                println!("   {language}: {value}");
+                count += 1;
+            }
+        }
+    }
+    count
+}
+
 fn check_keys(target_ini: &IniFile) -> io::Result<()> {
     for section in &target_ini.sections {
         let mut mismatches = Vec::new();
@@ -537,6 +567,14 @@ fn finish_language_with_ai(
                         if let Some((key, value)) = split_line(line) {
                             // Put the key through the inverse alias map.
                             let original_key = alias_inverse_map.get(key).unwrap_or(&key);
+                            let ref_value = ref_section.get_value(original_key).unwrap_or_default();
+                            let issues = validate::check_ai_translation(&ref_value, value);
+                            if !issues.is_empty() {
+                                for issue in issues {
+                                    println!("Rejecting '{original_key}' = '{value}': {issue}");
+                                }
+                                continue;
+                            }
                             print!("Updating '{}': {}", original_key, value);
                             if key != *original_key {
                                 println!(" ({})", key);
@@ -759,6 +797,8 @@ fn execute_command(cmd: Command, ai: Option<&Ai>, dry_run: bool, verbose: bool)
         None
     };
 
+    let mut issue_count = 0;
+
     for filename in &filenames {
         let reference_ini = &reference_ini;
         if filename == "langtool" {
@@ -796,6 +836,12 @@ fn execute_command(cmd: Command, ai: Option<&Ai>, dry_run: bool, verbose: bool)
             } => {
                 split_key(&mut target_ini, section, key).unwrap();
             }
+            Command::Validate => {
+                if !is_reference {
+                    let language = filename.split_once('.').unwrap().0;
+                    issue_count += validate_ini(&target_ini, reference_ini, language);
+                }
+            }
             Command::FinishLanguageWithAI {
                 language: _,
                 section: _,
@@ -878,8 +924,8 @@ fn execute_command(cmd: Command, ai: Option<&Ai>, dry_run: bool, verbose: bool)
                             )
                             .unwrap();
                         } else {
-                            println!("Language {lang} not found in response. Bailing.");
-                            return;
+                            println!("No usable translation for {lang}, skipping it.");
+                            continue;
                         }
                     }
                 } else {
@@ -912,8 +958,8 @@ fn execute_command(cmd: Command, ai: Option<&Ai>, dry_run: bool, verbose: bool)
                             )
                             .unwrap();
                         } else {
-                            println!("Language {lang} not found in response. Bailing.");
-                            return;
+                            println!("No usable translation for {lang}, skipping it.");
+                            continue;
                         }
                     }
                 } else {
@@ -1002,6 +1048,14 @@ fn execute_command(cmd: Command, ai: Option<&Ai>, dry_run: bool, verbose: bool)
         }
     }
 
+    if let Command::Validate = cmd {
+        println!("Found {issue_count} problems.");
+        if issue_count > 0 {
+            // Makes this usable as a check in scripts.
+            std::process::exit(1);
+        }
+    }
+
     // Don't need to additionally process reference here like we did before, this is done
     // in the main loop.
 }
@@ -1027,9 +1081,22 @@ fn generate_ai_response(
             .map_err(|e| anyhow::anyhow!("chat failed: {e}"))
             .unwrap();
         println!("AI response: {response}");
-        if let Some(parsed) = parse_response(&response) {
+        if let Some(mut parsed) = parse_response(&response) {
             println!("Parsed: {:?}", parsed);
 
+            // The English string we asked for a translation of is in the prompt as 'key'.
+            parsed.retain(|language, translation| {
+                let issues = if language == "en_US" {
+                    validate::check(key, translation)
+                } else {
+                    validate::check_ai_translation(key, translation)
+                };
+                for issue in &issues {
+                    println!("Rejecting {language} translation '{translation}': {issue}");
+                }
+                issues.is_empty()
+            });
+
             if parsed.len() < filenames.len() {
                 println!(
                     "Not enough languages generated! {} vs {}",

Tools/langtool/src/validate.rs

@@ -0,0 +1,134 @@
+use regex::Regex;
+use std::fmt;
+use std::sync::LazyLock;
+
+// %1-%9 are positional placeholders, %s/%d/%f are printf-style ones. Either way they need to
+// survive translation intact, or we get garbage (or worse) on screen at runtime.
+static PLACEHOLDER: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"%[0-9sdf]").unwrap());
+
+#[derive(Debug, PartialEq, Eq)]
+pub enum Issue {
+    Placeholders { expected: String, found: String },
+    Empty,
+    Newline,
+    Quoted,
+    Untranslated,
+}
+
+impl fmt::Display for Issue {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            Issue::Placeholders { expected, found } => {
+                write!(f, "placeholder mismatch, expected [{expected}], got [{found}]")
+            }
+            Issue::Empty => write!(f, "empty translation"),
+            Issue::Newline => write!(f, "contains a line break"),
+            Issue::Quoted => write!(f, "wrapped in quotes or a markdown fence"),
+            Issue::Untranslated => write!(f, "same as the English string"),
+        }
+    }
+}
+
+fn placeholders(str: &str) -> Vec<&str> {
+    let mut found: Vec<&str> = PLACEHOLDER.find_iter(str).map(|m| m.as_str()).collect();
+    found.sort_unstable();
+    found
+}
+
+/// Checks a translated string against the English one it was translated from. Cheap enough to
+/// run over every string in every language file, see the Validate command.
+///
+/// Note that Untranslated is never returned here - plenty of strings are legitimately identical
+/// to the English one, so only the AI paths check for it (see check_ai_translation).
+pub fn check(reference: &str, translation: &str) -> Vec<Issue> {
+    let mut issues = vec![];
+
+    if translation.trim().is_empty() {
+        // Nothing else is going to be meaningful.
+        return vec![Issue::Empty];
+    }
+
+    if translation.contains('\n') || translation.contains('\r') {
+        issues.push(Issue::Newline);
+    }
+
+    let expected = placeholders(reference);
+    let found = placeholders(translation);
+    if expected != found {
+        issues.push(Issue::Placeholders {
+            expected: expected.join(" "),
+            found: found.join(" "),
+        });
+    }
+
+    // The AI likes to quote things, and the quotes are not part of the translation.
+    if is_quoted(translation.trim()) && !is_quoted(reference.trim()) {
+        issues.push(Issue::Quoted);
+    }
+
+    issues
+}
+
+/// Same as check(), but also rejects a translation that's just the English string echoed back,
+/// which is a common enough AI failure that it's worth catching before we write it to a file
+/// (and mark it as translated).
+pub fn check_ai_translation(reference: &str, translation: &str) -> Vec<Issue> {
+    let mut issues = check(reference, translation);
+    if translation.trim() == reference.trim() {
+        issues.push(Issue::Untranslated);
+    }
+    issues
+}
+
+fn is_quoted(str: &str) -> bool {
+    (str.len() > 1 && str.starts_with('"') && str.ends_with('"')) || str.starts_with("```")
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn accepts_normal_translations() {
+        assert!(check("Graphics", "Grafik").is_empty());
+        assert!(check("Slot %1", "Kortplats %1").is_empty());
+        assert!(check("%d (%d per core, %d cores)", "%d (%d per kärna, %d kärnor)").is_empty());
+        // Reordering placeholders is fine, plenty of languages need to.
+        assert!(check("Submitted %1 for %2", "%2 till %1 inskickad").is_empty());
+        // Percent signs that aren't placeholders shouldn't bother it.
+        assert!(check("Speed: 100%", "Hastighet: 100%").is_empty());
+    }
+
+    #[test]
+    fn catches_broken_placeholders() {
+        // A dropped placeholder, as seen in ko_KR before this was added.
+        assert_eq!(
+            check("Submitted %1 for %2", "2에 %1을(를) 제출함"),
+            vec![Issue::Placeholders {
+                expected: "%1 %2".to_string(),
+                found: "%1".to_string()
+            }]
+        );
+        // A localized digit, as seen in fa_IR.
+        assert!(!check("Quick chat %1", "گپ سریع ۱").is_empty());
+        // A space snuck in between the % and the digit, as seen all over km_KH.
+        assert!(!check("Slot %1", "រន្ធ% 1").is_empty());
+        // Same placeholder, wrong number of them.
+        assert!(!check("%d of %d", "%d").is_empty());
+    }
+
+    #[test]
+    fn catches_bad_ai_output() {
+        assert_eq!(check("Graphics", ""), vec![Issue::Empty]);
+        assert_eq!(check("Graphics", "  "), vec![Issue::Empty]);
+        assert_eq!(check("Graphics", "\"Grafik\""), vec![Issue::Quoted]);
+        assert_eq!(check("Graphics", "```Grafik```"), vec![Issue::Quoted]);
+        assert_eq!(check("Graphics", "Grafik\nGrafik"), vec![Issue::Newline]);
+        assert_eq!(
+            check_ai_translation("Graphics", "Graphics"),
+            vec![Issue::Untranslated]
+        );
+        // But check() itself allows it, lots of strings are the same in both languages.
+        assert!(check("Graphics", "Graphics").is_empty());
+    }
+}