Merge pull request Expensify#64973 from Expensify/mollfpr-validate-html-chatgpt-translator

[NoQA] Validate HTML string for ChatGPT Translator

Author: roryabraham

cspell.json

@@ -98,6 +98,7 @@
         "Charleson",
         "Checkmark",
         "checkmarked",
+        "chien",
         "Chronos",
         "citi",
         "clawback",
@@ -136,6 +137,7 @@
         "deeplinks",
         "delegators",
         "delish",
+        "describedby",
         "Deutsch",
         "devportal",
         "DFOLLY",
@@ -295,6 +297,7 @@
         "killall",
         "Kowalski",
         "Krasoń",
+        "labelledby",
         "Lagertha",
         "laggy",
         "lastiPhoneLogin",

scripts/utils/Translator/ChatGPTTranslator.ts

@@ -35,15 +35,15 @@ class ChatGPTTranslator extends Translator {
                     userPrompt: text,
                 });
 
-                if (this.validateTemplatePlaceholders(text, result)) {
+                if (this.validateTemplatePlaceholders(text, result) && this.validateTemplateHTML(text, result)) {
                     if (attempt > 0) {
                         console.log(`🙃 Translation succeeded after ${attempt + 1} attempts`);
                     }
                     console.log(`🧠 Translated "${text}" to ${targetLang}: "${result}"`);
                     return result;
                 }
 
-                console.warn(`⚠️ Translation for "${text}" failed placeholder validation (attempt ${attempt + 1}/${ChatGPTTranslator.MAX_RETRIES + 1})`);
+                console.warn(`⚠️ Translation for "${text}" failed validation (attempt ${attempt + 1}/${ChatGPTTranslator.MAX_RETRIES + 1})`);
 
                 if (attempt === ChatGPTTranslator.MAX_RETRIES) {
                     console.error(`❌ Final attempt failed placeholder validation. Falling back to original.`);
@@ -63,19 +63,6 @@ class ChatGPTTranslator extends Translator {
         // Should never hit this, but fallback just in case
         return text;
     }
-
-    /**
-     * Validate that placeholders are all present and unchanged before and after translation.
-     */
-    private validateTemplatePlaceholders(original: string, translated: string): boolean {
-        const extractPlaceholders = (s: string) =>
-            Array.from(s.matchAll(/\$\{[^}]*}/g))
-                .map((m) => m[0])
-                .sort();
-        const originalSpans = extractPlaceholders(original);
-        const translatedSpans = extractPlaceholders(translated);
-        return JSON.stringify(originalSpans) === JSON.stringify(translatedSpans);
-    }
 }
 
 export default ChatGPTTranslator;

scripts/utils/Translator/Translator.ts

@@ -1,3 +1,4 @@
+import {DomUtils, parseDocument} from 'htmlparser2';
 import type {TranslationTargetLocale} from '@src/CONST/LOCALES';
 
 /**
@@ -33,6 +34,58 @@ abstract class Translator {
     private trimForLogs(text: string) {
         return `${text.slice(0, 80)}${text.length > 80 ? '...' : ''}`;
     }
+
+    /**
+     * Validate that placeholders are all present and unchanged before and after translation.
+     */
+    public validateTemplatePlaceholders(original: string, translated: string): boolean {
+        const extractPlaceholders = (s: string) =>
+            Array.from(s.matchAll(/\$\{[^}]*}/g))
+                .map((m) => m[0])
+                .sort();
+        const originalSpans = extractPlaceholders(original);
+        const translatedSpans = extractPlaceholders(translated);
+        return JSON.stringify(originalSpans) === JSON.stringify(translatedSpans);
+    }
+
+    /**
+     * Validate that the HTML structure is the same before and after translation.
+     */
+    public validateTemplateHTML(original: string, translated: string): boolean {
+        // Attributes that are allowed to be translated
+        const TRANSLATABLE_ATTRIBUTES = new Set(['alt', 'title', 'placeholder', 'aria-label', 'aria-describedby', 'aria-labelledby', 'value']);
+
+        const parseHTMLStructure = (s: string) => {
+            const doc = parseDocument(s);
+            const elements = DomUtils.getElementsByTagName(() => true, doc, true);
+
+            return elements.map((element) => {
+                const tagName = element.name.toLowerCase();
+
+                // Extract attributes, excluding translatable ones
+                const attributes: string[] = [];
+                if (element.attribs) {
+                    for (const [attrName, attrValue] of Object.entries(element.attribs ?? {})) {
+                        const normalizedAttrName = attrName.toLowerCase();
+                        if (!TRANSLATABLE_ATTRIBUTES.has(normalizedAttrName)) {
+                            attributes.push(`${normalizedAttrName}="${attrValue ?? ''}"`);
+                        }
+                    }
+                }
+
+                return {
+                    tagName,
+                    attributes: attributes.sort(),
+                };
+            });
+        };
+
+        const originalStructure = parseHTMLStructure(original);
+        const translatedStructure = parseHTMLStructure(translated);
+
+        // Compare structures (tag names and non-translatable attributes)
+        return JSON.stringify(originalStructure) === JSON.stringify(translatedStructure);
+    }
 }
 
 export default Translator;

tests/unit/ChatGPTTranslatorTest.ts

@@ -18,6 +18,10 @@ describe('ChatGPTTranslator.performTranslation', () => {
     const validTranslation = '[it] Hello ${name}!';
     const invalidTranslation = '[it] Hello name!'; // missing ${...}
 
+    const originalHTML = '<img src="photo.jpg" alt="A dog" class="photo">';
+    const validHTMLTranslation = '[it] <img src="photo.jpg" alt="Un chien" class="photo">';
+    const invalidHTMLTranslation = '[it] <img src="different.jpg" alt="Un chien" class="photo">'; // different src
+
     let translator: ChatGPTTranslator;
 
     beforeEach(() => {
@@ -50,4 +54,16 @@ describe('ChatGPTTranslator.performTranslation', () => {
         expect(MockedOpenAIUtils.prototype.promptChatCompletions).toHaveBeenCalledTimes(ChatGPTTranslator.MAX_RETRIES + 1);
         expect(result).toBe(original);
     });
+
+    it('retries if translated HTML has incorrect attributes, then succeeds', async () => {
+        // First attempt returns invalid HTML format, second returns valid
+        (MockedOpenAIUtils.prototype.promptChatCompletions as jest.Mock).mockResolvedValueOnce(invalidHTMLTranslation).mockResolvedValueOnce(validHTMLTranslation);
+
+        // @ts-expect-error TS2445
+        const result = await translator.performTranslation(targetLang, originalHTML);
+
+        // eslint-disable-next-line @typescript-eslint/unbound-method
+        expect(MockedOpenAIUtils.prototype.promptChatCompletions).toHaveBeenCalledTimes(2);
+        expect(result).toBe(validHTMLTranslation);
+    });
 });