feat(insight): add multi-language support for insight reports#2061
feat(insight): add multi-language support for insight reports#2061pomelo-nwu wants to merge 1 commit intomainfrom
Conversation
- LLM-generated content now respects user's UI language setting - HTML report static text is localized for en/zh/ja/de/pt/ru - Added language display message when generating insights - Set HTML lang attribute based on user language - Frontend React components use translation dictionary Closes #2022 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
📋 Review SummaryThis PR implements multi-language support for the 🔍 General Feedback
🎯 Specific Feedback🟡 High
🟢 Medium
🔵 Low
✅ Highlights
|
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
1. Broken string interpolation in Header.tsx (Bug)
This word-by-word translation approach produces grammatically incorrect output in most non-English languages. For example, in Chinese this renders as "42 条消息,跨越 5 个会话" — but the number placement and particles don't work naturally when split like this. Suggestion: Use a single translation key with placeholders: This lets translators control the full sentence structure per language. 2. Duplicate SupportedLanguage type definitionSupportedLanguage is defined in:
These two definitions could drift out of sync. Consider having a single source of truth — either re-export from one place, or define it in a shared types package. 3. Module-level currentLang is evaluated once at import timeIn both Header.tsx and Qualitative.tsx: This is a top-level constant. If window.INSIGHT_LANGUAGE is set after the module loads (e.g., due to script ordering), currentLang will be 'en' forever. Currently the data script is placed before the app script in the HTML, so this likely works — but it's fragile. Suggestion: Either move the read inside the tr function, or add a comment documenting the script ordering dependency. 4. Hardcoded pt-BR assumptionconst HTML_LANG_CODES: Record<SupportedLanguage, string> = { The language code pt maps to pt-BR specifically. Portuguese users from Portugal (pt-PT) may find this unexpected. Consider using pt as the HTML lang attribute, or making the distinction configurable. 5. Large i18n.ts file (419 lines) with no fallback mechanism for missing keys in non-English localesThe t() function does fall back to English, which is good: However, there's no compile-time or runtime check that all locales have the same keys. If a key is added to English but missed in other locales, it silently falls back. Consider adding a type-safe approach or at minimum a dev-time assertion. 6. Inconsistent comment formatting across locale filesSome locale files (en, ja, zh) have a blank line before the comment block; others (de, pt, ru) don't. Minor but worth normalizing for consistency. 7. LANGUAGE_DISPLAY_NAMES duplicated across layersDataProcessor.ts defines LANGUAGE_DISPLAY_NAMES and TemplateRenderer.ts defines HTML_LANG_CODES. These language metadata mappings could be consolidated into a single shared constant alongside the SupportedLanguage type. |
What this PR does
This PR implements multi-language support for the
/insightHTML report, addressing issue #2022.Changes Made
LLM-generated content localization
DataProcessor.tsto pass user language setting to LLM promptsStatic text localization
i18n.tswith translations for 6 languages (en, zh, ja, de, pt, ru)Language indication
HTML lang attribute
langattribute based on user's language settingSupported Languages
Testing
Related Issue
Closes #2022