fix: Replace with a supported robust solution#2
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a Python script for internationalization (i18n) management, including functions to check for missing translations, translate them using OpenAI, patch locale files, and perform quality assurance. The review identifies several critical areas for improvement: the translation function lacks a target language parameter and uses deprecated OpenAI APIs, the missing translation checker and locale patcher do not correctly handle nested JSON structures, and the QA function uses an ineffective difflib.ndiff approach that can lead to errors. Additionally, the script needs proper handling for loading the OpenAI API key.
| # i18n_translator.py | ||
| import openai | ||
|
|
||
| def translate_missing_translations(missing_translations, source_locale_path): |
| response = openai.Completion.create( | ||
| engine="text-davinci-003", | ||
| prompt=prompt, | ||
| max_tokens=60 | ||
| ) | ||
| translations[key] = response.choices[0].text.strip() |
There was a problem hiding this comment.
The openai.Completion API and the text-davinci-003 model are deprecated and no longer functional. Since the project uses openai version ^1.50.2, you should use the ChatCompletion API with a modern model like gpt-4o-mini. Note that the response structure also differs in the new SDK.
| response = openai.Completion.create( | |
| engine="text-davinci-003", | |
| prompt=prompt, | |
| max_tokens=60 | |
| ) | |
| translations[key] = response.choices[0].text.strip() | |
| client = openai.OpenAI() | |
| response = client.chat.completions.create( | |
| model="gpt-4o-mini", | |
| messages=[{"role": "user", "content": prompt}] | |
| ) | |
| translations[key] = response.choices[0].message.content.strip() |
| for key in source_locale: | ||
| if key not in target_locale: | ||
| missing_translations.append(key) |
There was a problem hiding this comment.
| with open(target_locale_path, 'r', encoding='utf-8') as f: | ||
| target_locale = json.load(f) | ||
|
|
||
| target_locale.update(translations) |
There was a problem hiding this comment.
| return missing_translations | ||
|
|
||
| # i18n_translator.py | ||
| import openai |
There was a problem hiding this comment.
The script does not load environment variables (e.g., using python-dotenv), which are typically used to manage the OPENAI_API_KEY. Without this or an explicit API key configuration, the OpenAI client will fail to authenticate. Consider adding load_dotenv() if you intend to use environment variables.
| diff = difflib.ndiff(source_locale[key], target_locale[key]) | ||
| qa_results.append((key, '\n'.join(diff))) |
There was a problem hiding this comment.
Using difflib.ndiff to compare source text with its translation is not an effective QA practice for i18n. Since the languages differ, the diff will essentially show that the entire string has changed, which doesn't help in identifying quality issues. Furthermore, if the values are nested dictionaries, ndiff will raise a TypeError.
|
Closing this in favor of PR #3, which provides a complete Tolgee integration with CI workflows. Thanks for taking a look! 💜 |
74 tests covering: nested-key detection (the case competitor PR tari-project#2 missed), placeholder preservation, deep-merge correctness, QA placeholder mismatch reporting, retry logic, and missing-API-key error handling. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fix for #1: Replace with a supported robust solution
Explanation:
These changes replace the hardcoded paths and manual workflow with a more robust solution that can be integrated into CI and supports multiple projects.
Closes #1