feat: Add ol_openedx_ai_static_translations plugin; clean up ol_openedx_course_translations#769
feat: Add ol_openedx_ai_static_translations plugin; clean up ol_openedx_course_translations#769
Conversation
|
@copilot rebase this PR |
Done. The branch has been rebased onto the latest |
There was a problem hiding this comment.
Pull request overview
This PR extracts the static translation sync/LLM translation + PR-creation workflow from ol_openedx_course_translations into a new standalone CMS-only plugin (ol_openedx_ai_static_translations), while slimming down the original course translations plugin to only the remaining course/subtitle translation functionality.
Changes:
- Introduces
ol_openedx_ai_static_translationswith thesync_and_translate_languagemanagement command and supporting sync/translation utilities. - Removes static-sync infrastructure (command + helpers + dependencies + bundled glossaries) from
ol_openedx_course_translations. - Updates plugin configuration/docs and bumps
ol_openedx_course_translationsversion to0.6.0.
Reviewed changes
Copilot reviewed 26 out of 28 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/ol_openedx_course_translations/README.rst | Removes static translation sync command documentation/config from the course translations plugin. |
| src/ol_openedx_course_translations/pyproject.toml | Removes sync-only dependencies and bumps plugin version. |
| src/ol_openedx_course_translations/ol_openedx_course_translations/utils/constants.py | Trims constants to only those needed for remaining course translation features. |
| src/ol_openedx_course_translations/ol_openedx_course_translations/utils/command_utils.py | Deletes shared command utilities that were only used by the extracted sync command. |
| src/ol_openedx_course_translations/ol_openedx_course_translations/settings/common.py | Removes static-sync repo/token settings from the course translations plugin settings. |
| src/ol_openedx_course_translations/ol_openedx_course_translations/glossaries/machine_learning/ar.txt | Removes bundled glossary content from the course translations plugin. |
| src/ol_openedx_course_translations/ol_openedx_course_translations/glossaries/machine_learning/de.txt | Removes bundled glossary content from the course translations plugin. |
| src/ol_openedx_course_translations/ol_openedx_course_translations/glossaries/machine_learning/el.txt | Removes bundled glossary content from the course translations plugin. |
| src/ol_openedx_course_translations/ol_openedx_course_translations/glossaries/machine_learning/es.txt | Removes bundled glossary content from the course translations plugin. |
| src/ol_openedx_course_translations/ol_openedx_course_translations/glossaries/machine_learning/es_419.txt | Removes bundled glossary content from the course translations plugin. |
| src/ol_openedx_course_translations/ol_openedx_course_translations/glossaries/machine_learning/fr.txt | Removes bundled glossary content from the course translations plugin. |
| src/ol_openedx_course_translations/ol_openedx_course_translations/glossaries/machine_learning/ja.txt | Removes bundled glossary content from the course translations plugin. |
| src/ol_openedx_course_translations/ol_openedx_course_translations/glossaries/machine_learning/pt_BR.txt | Removes bundled glossary content from the course translations plugin. |
| src/ol_openedx_course_translations/ol_openedx_course_translations/glossaries/machine_learning/ru.txt | Removes bundled glossary content from the course translations plugin. |
| src/ol_openedx_ai_static_translations/setup.cfg | Adds plugin-local tooling config (isort/pytest) consistent with other plugins in this repo. |
| src/ol_openedx_ai_static_translations/README.rst | Adds documentation for the new static translations plugin and its management command. |
| src/ol_openedx_ai_static_translations/pyproject.toml | Defines packaging metadata and dependencies for the new plugin. |
| src/ol_openedx_ai_static_translations/ol_openedx_ai_static_translations/utils.py | Houses extracted sync/translation helpers (validation/config/glossary/PO+JSON sync logic). |
| src/ol_openedx_ai_static_translations/ol_openedx_ai_static_translations/settings/cms.py | Adds CMS-only plugin settings entry point. |
| src/ol_openedx_ai_static_translations/ol_openedx_ai_static_translations/settings/init.py | Provides default settings with hasattr guards to avoid clobbering. |
| src/ol_openedx_ai_static_translations/ol_openedx_ai_static_translations/management/commands/sync_and_translate_language.py | Implements the extracted sync_and_translate_language management command. |
| src/ol_openedx_ai_static_translations/ol_openedx_ai_static_translations/management/commands/init.py | Adds management command package marker. |
| src/ol_openedx_ai_static_translations/ol_openedx_ai_static_translations/management/init.py | Adds management package marker. |
| src/ol_openedx_ai_static_translations/ol_openedx_ai_static_translations/constants.py | Adds constants required for static translation syncing/formatting. |
| src/ol_openedx_ai_static_translations/ol_openedx_ai_static_translations/apps.py | Registers the plugin as a CMS-only Django app via Open edX plugin settings. |
| src/ol_openedx_ai_static_translations/ol_openedx_ai_static_translations/init.py | Adds package marker and short description. |
| src/ol_openedx_ai_static_translations/LICENSE.txt | Adds plugin-local license file. |
| src/ol_openedx_ai_static_translations/CHANGELOG.rst | Adds plugin-local changelog scaffold. |
Comments suppressed due to low confidence (1)
src/ol_openedx_ai_static_translations/ol_openedx_ai_static_translations/management/commands/sync_and_translate_language.py:53
- The command’s
--providerargument restricts choices to (openai, gemini, mistral) but usesTRANSLATIONS_PROVIDERS['default_provider']as the default. If that setting is configured asdeepl(common for course translations), argparse will fail even when the user doesn’t pass--provider. Consider only setting the argparse default when it’s in the allowed choices (or includedeeplif supported, or validate inhandleinstead of usingchoices).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: asadali145 <52656433+asadali145@users.noreply.github.com> Agent-Logs-Url: https://github.com/mitodl/open-edx-plugins/sessions/eaab10e1-8468-4675-9118-cd9b1fdac54e
Co-authored-by: asadali145 <52656433+asadali145@users.noreply.github.com> Agent-Logs-Url: https://github.com/mitodl/open-edx-plugins/sessions/eaab10e1-8468-4675-9118-cd9b1fdac54e
…ourse_translations Co-authored-by: asadali145 <52656433+asadali145@users.noreply.github.com> Agent-Logs-Url: https://github.com/mitodl/open-edx-plugins/sessions/0ad538c8-6c6c-4b49-852c-62936b165920
c042ea0 to
877eccb
Compare
| completion_kwargs = dict(base_kwargs) | ||
| completion_kwargs["model"] = model | ||
|
|
||
| if api_key: |
There was a problem hiding this comment.
Bug: Provider-specific model prefixing in configure_litellm_for_provider is skipped when api_key is None, which can occur with implicit credential discovery, causing incorrect API routing.
Severity: LOW
Suggested Fix
Move the model prefixing logic for Gemini and Mistral providers outside of the if api_key: block. This ensures the model name is always correctly formatted, regardless of how the API key is provided or discovered.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location:
src/ol_openedx_ai_static_translations/ol_openedx_ai_static_translations/utils.py#L184
Potential issue: The function `configure_litellm_for_provider` only adds the required
`gemini/` or `mistral/` prefix to the model name if an `api_key` is explicitly passed.
In scenarios where authentication relies on implicit credential discovery (e.g., Google
credential files) and `api_key` is `None`, the prefix is not added. This can cause
LiteLLM to route requests to an unintended service, such as Vertex AI instead of the
Gemini API, leading to incorrect API usage.
Extracts the
sync_and_translate_languagemanagement command and all supporting sync infrastructure out ofol_openedx_course_translationsinto a new standaloneol_openedx_ai_static_translationsplugin, incorporating the glossary-as-path change from PR #760.New plugin:
ol_openedx_ai_static_translationsconstants.py+utils.pyat package root (vs nestedutils/in source plugin)TRANSLATIONS_GITHUB_TOKEN,TRANSLATIONS_REPO_URL,TRANSLATIONS_REPO_PATH,TRANSLATIONS_PROVIDERS,LITE_LLM_REQUEST_TIMEOUT) — useshasattrguards so it won't clobber if old plugin is also installed--glossarynow takes an explicit directory path instead of a boolean flag:Cleanup in
ol_openedx_course_translationsmanagement/commands/sync_and_translate_language.py,utils/command_utils.py,utils/translation_sync.pyglossaries/directory (both plugins)utils/constants.pystripped to only what remaining code needs:PROVIDER_*,ENGLISH_LANGUAGE_CODE,TRANSLATABLE_ATTRS_*,NEVER_TRANSLATE_ATTRS,XML_FORMAT_ATTRsettings/common.py— removedTRANSLATIONS_GITHUB_TOKEN/REPO_URL/REPO_PATH; keptTRANSLATIONS_PROVIDERSandLITE_LLM_REQUEST_TIMEOUT(still used bytranslate_courseandllm_providers)pyproject.toml— removedGitPython,requests,polib(only needed by moved sync code); bumped version0.5.1 → 0.6.0README.rst— removedsync_and_translate_languagesection and sync-specific config entries✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.