-
Notifications
You must be signed in to change notification settings - Fork 5
feat: Enable auto translation on learner facing content #679
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
2c27956 to
fc7818b
Compare
for more information, see https://pre-commit.ci
bb23480 to
744c94c
Compare
for more information, see https://pre-commit.ci
src/ol_openedx_course_translations/ol_openedx_course_translations/settings/common.py
Outdated
Show resolved
Hide resolved
src/ol_openedx_course_translations/ol_openedx_course_translations/settings/common.py
Outdated
Show resolved
Hide resolved
src/ol_openedx_course_translations/ol_openedx_course_translations/settings/common.py
Outdated
Show resolved
Hide resolved
src/ol_openedx_course_translations/ol_openedx_course_translations/settings/common.py
Outdated
Show resolved
Hide resolved
src/ol_openedx_course_translations/ol_openedx_course_translations/settings/common.py
Outdated
Show resolved
Hide resolved
src/ol_openedx_course_translations/ol_openedx_course_translations/settings/common.py
Outdated
Show resolved
Hide resolved
src/ol_openedx_course_translations/ol_openedx_course_translations/settings/common.py
Outdated
Show resolved
Hide resolved
src/ol_openedx_course_translations/ol_openedx_course_translations/settings/common.py
Outdated
Show resolved
Hide resolved
...anslations/ol_openedx_course_translations/management/commands/sync_and_translate_language.py
Outdated
Show resolved
Hide resolved
|
As discussed over the call, Mistral needs some changes on how default model is set. It is not working when we remove mistral/ from the model name. Also, gemini is not working. |
asadali145
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
src/ol_openedx_course_translations/ol_openedx_course_translations/utils/command_utils.py
Outdated
Show resolved
Hide resolved
| if provider == "gemini" and not model.startswith(("gemini/", "vertex_ai/")): | ||
| completion_kwargs["model"] = f"gemini/{model}" | ||
| if provider == "gemini": | ||
| if not model.startswith(("gemini/", "vertex_ai/")): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we including vertex_ai here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LiteLLM allows access to Gemini in two ways:
Gemini API (prefix: gemini/)
Vertex AI (prefix: vertex_ai/)
For example, the model vertex_ai/gemini-3-pro-preview uses Vertex AI. If only gemini-3-pro-preview is provided, LiteLLM may default to using Vertex AI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The author's explanation is correct. LiteLLM, the library used here, supports accessing Gemini models through both the native Gemini API (prefixed with gemini/) and Google Cloud's Vertex AI (prefixed with vertex_ai/). The code ensures that if a Gemini model is specified without either of these prefixes, it defaults to the gemini/ prefix for consistency with LiteLLM's expected format, while still allowing explicitly specified vertex_ai/ models to pass through unchanged.
| if provider == "gemini": | ||
| if not model.startswith(("gemini/", "vertex_ai/")): | ||
| completion_kwargs["model"] = f"gemini/{model}" | ||
| # Gemini 3 models require temperature = 1.0 to avoid issues |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we explain the issues here?
What are the relevant tickets?
https://github.com/mitodl/hq/issues/9667
Description (What does it do?)
This PR adds automated translation management functionality to the
ol_openedx_course_translationsplugin. The system syncs translation keys from English to target language files, translates empty keys using LLM APIs, and automatically creates pull requests in themitxonline-translationsrepository.Relevant Test Pull Requests:
https://github.com/zamanafzal/mitxonline-translations/pulls
How can this be tested?
Ensure you're in the Open edX environment (Tutor container)
Checkout this branch for open-edx-plugins.
tutor dev exec lms bashexport the LLM key you want to use. e.g MISTRAL
export MISTRAL_API_KEY="KEY"Generate a github token which will be used to authenticate with github and pushing changes.
export GITHUB_TOKEN ="token"./manage.py sync_and_translate_language {lan_name} --model {model_name}
For example:
It will translate the missing keys and generate files if don't exist and create a PR.