Repository navigation
ci(translate): run only the legs a push can change - #575
Merged
Merged
Conversation
Every qualifying push ran 10 languages x 20 phases of lesson legs plus 11 interface-string legs, about 212 jobs, even when the push changed one lesson or only registry metadata. On the #570 merge every leg ended with "no changes". scripts/translate_matrix.py now builds both matrices. For a push it diffs github.event.before..github.sha and selects: - lesson legs for each phase whose docs/en.md changed, for every automatic language; - every phase for a language that newly has ci: true; - interface-string legs for every site language when site/ui-strings.json or scripts/translate_ui_strings.py changed, and for languages newly listed as ci or site. A push that also changes the code the legs run (translate.yml, translate_lessons.py and the modules it imports, or the runtime requirements) runs every leg. So does a range that cannot be read: an all-zeros before, a force-push, or a before commit that cannot be fetched. The prepare checkout stays at depth 1; the script fetches only the before commit at depth 1 (0.1 to 0.3 MiB and about 1 s in local measurements, against 103 MiB for a full-history checkout). workflow_dispatch keeps its scope: the requested languages, or the automatic set, times the requested phase or every phase. An empty matrix now skips its job instead of failing the run, and the "no registered non-source languages selected" error applies only to an invalid manual request. The lesson matrix is now an include list, so one push can combine changed phases with a newly automatic language without a cross product. A change to the selector itself triggers the workflow, which then runs only the prepare job unless the same push changed translation inputs.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: 🔵 Low · up to A failed translation leg may need a manual rerun. The documented recovery path makes this a bounded operational risk, not a merge blocker. Pre-merge checks |
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 63 functions across 2 files. (3 skipped: 3… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
-
Check name Status Explanation Description check The description clearly explains the push-based translation matrix selection, fallback behavior, workflow changes, tests, and known limitations. It is directly related to the changeset. Title check The title concisely and accurately describes the primary change: running only translation workflow legs that a push can change. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 63 functions across 2 files. (3 skipped: 3 unsupported.)
- Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
-
📝 Generate docstrings 💡
-
-
- Commit to this branch
- Create a new PR
-
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Every qualifying push to
mainruns the full translate matrix: 10 automatic languages x 20 phases = 200 lesson legs, 11 interface-string legs, plusprepareandcoverage, so 213 jobs of about 60 s each. On the #570 merge every leg ended with "no changes". A one-lesson edit also fans out to all 200 lesson legs.With this change a push runs only the legs its commit range can change. Manual runs keep their scope.
How a push chooses its legs
scripts/translate_matrix.pynow builds both matrices. On a push it diffsgithub.event.before..github.shaand selects:phases/<phase>/**/docs/en.mdchanged, for every automatic (ci: true) language;ci: true; a metadata-only registry edit (dir,locale, names,_comment) selects nothing;site/ui-strings.jsonorscripts/translate_ui_strings.pychanged, and for languages newly listed asciorsite.It falls back to today's full matrix when:
translate.yml,translate_lessons.pyand the modules it imports (build_catalog.py,_lib.py), or.github/translate-requirements.txt. A test walks the imports of both translator scripts so this list cannot drift silently;before, a force-push (github.event.forced), abeforethat is not a commit id, abeforecommit that cannot be fetched, or alanguages.jsonatbeforethat cannot be parsed.Other details:
preparecheckout stays at depth 1. The script fetches only thebeforecommit withgit fetch --depth=1. Measured locally against GitHub: +284 KiB and 1 s for the feat(site): server-render lessons and their translations #570 range, +84 KiB and 1 s for feat(paths): connect career routes to practical delivery evidence #559. Afetch-depth: 0checkout is 103 MiB and took 44 s.translateandui-stringshave anif:on the prepare outputs, so an empty matrix skips the job instead of failing.coveragekeeps!cancelled() && needs.translate.result != 'skipped'. The "no registered non-source languages selected" error now applies only to an invalid manual request.includelist, so one push can combine changed phases with a newly automatic language without a cross product.workflow_dispatchselects the same legs as before. I ran the old jq step and the new script on 14 inputs (blank, one phase, mixed valid and invalid codes, source-only, whitespace, traversal, unknown phase): outputs and error messages are identical.persist-credentials: false, the same job permissions, and the event values reach the script only throughenv. Abeforevalue that is not a commit id is never passed to git.scripts/translate_matrix.pyis a push trigger, but it is not in the full-matrix list, because no leg runs it. A selector-only change runs onlyprepare.readme-counts-syncincurriculum.ymlis a single job, so it has no fan-out to reduce.Replay on real ranges
I ran the script against real history in a full clone. Each row runs the script at the pushed commit with that push's real
beforeSHA, which I read from the run's check suite. "Today" counts the full matrix at that commit plusprepareandcoverage. The minutes column uses the per-job durations of the actual run, each rounded up to a whole minute. It compares all jobs with only the jobs the new selection keeps.a795cb5..8e8a069languages.jsonmetadata, andtranslate.yml(the newcoveragejob)translate.ymlchangedlanguages.jsonchange alone (synthetic commit)5190afd..d75fb6114-agent-engineering6331da9..ff34db811-llm-engineering(8 automatic languages then)f6dbae7..6f73b43site/ui-strings.jsonb6a7a17..6d4e13aui-strings.jsonandtranslate_ui_strings.py9abf3d7..a0e2361languages.json,ui-strings.json,translate_ui_strings.py,translate.ymltranslate.ymlchangedlanguages.jsonchange alone (synthetic commit)pt-BRandfabecomeci,zh-TWbecomessitept-BR,fax 20 phases), 3 UI legs (zh-TW,pt-BR,fa)beforeforced: truebeforecommitBoth #570 and #556 also edited
translate.yml, so the rule keeps their full matrix. The synthetic rows apply only thelanguages.jsonchange on top ofbefore. They show that the registry logic alone selects nothing for #570 and only the new languages for #556.Estimated savings
translate.ymland 1 editedbuild_catalog.pyand_lib.py. Those keep the full matrix by design. This PR's merge editstranslate.yml, so it runs the full matrix once.Not covered
docs/i18n.mdnow says so. The same gap applies when thetranslateconcurrency group replaces a pending run with a newer one, because the newer run diffs only its own range. A follow-up could also select phases whose published cache on thetranslationsbranch is stale (that needs a 27 MiB depth-1 fetch inprepare).translate_lessons.py,build_catalog.py,_lib.pyandtranslate-requirements.txtare still not push triggers, as before. They force the full matrix only when they arrive with a triggering change.matrix.includefrom JSON, theif:skip of an empty matrix, and the depth-1 fetch ofbefore. Locally, a depth-1 clone fetchedbeforefrom GitHub and selected the expected 10 legs for feat(paths): connect career routes to practical delivery evidence #559.Tests
python3 scripts/test_translate_workflow.py: 34 tests pass on Python 3.14 and 3.12. 17 of them are new push-selection tests that use temporary git repos: one lesson, unrelated phase files, metadata-only and removal registry edits, newlyci,sitepromoted toci, newlysite, the UI sources, each full-run source, a merged lesson and new language, a moved lesson and a removed phase, a multi-commit range, the unreadable ranges (zeros, force-push, unknown commit, empty, option-shaped), an unparseable previous registry, and a shallow clone that fetchesbefore. Other new tests check that the prepare step passes the event only throughenv, that theif:guards exist, and that the full-run list covers the import closure of each leg.translateif:, one at a time. The suite failed each time.npx --yes @action-validator/clipasses ontranslate.ymlandcurriculum.yml.test_translate_ui_strings.py,test_audit_readme_locales.py,build_readme_i18n.py --checkandnode --test site/test_ui_i18n.jspass.