Repository navigation
feat(i18n): extract translator context comments from source - #44397
Conversation
Code Review Agent Run #953e4dActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
github-actions
Bot
added
i18n
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #44397 +/- ##
=======================================
Coverage 82.71% 82.71%
=======================================
Files 3011 3011
Lines 188332 188332
Branches 43716 43716
=======================================
Hits 155775 155775
Misses 29787 29787
Partials 2770 2770
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
34beb41 to
157bf32
Compare
Code Review Agent Run #82a731Actionable Suggestions - 0Additional Suggestions - 1
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
157bf32 to
24c455c
Compare
|
Rebased onto current master ( Re-checked after the rebase, since master's template was regenerated in the meantime by #44467, #44574 and #44584:
CI was red here only because the branch predated #44574, which fixed the template drift on master. That should clear with this rebase. |
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
|
The issue is correct. While To resolve this, update the documentation to explicitly instruct developers to run the Would you like me to implement this documentation fix? I can also check the rest of the PR for other comments if you would like me to address them as well. docs/developer_docs/contributing/howtos.md Updating language files./scripts/translations/babel_update.sh |
Code Review Agent Run #2c58eaActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Code Review Agent Run #29e96cActionable Suggestions - 0Additional Suggestions - 3
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
…w tests One POT header constant and one extract fake instead of two copies of each; return annotations on both fake factories; docstrings on the six comment-drift tests, matching the others this PR adds. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Code Review Agent Run #448ac8Actionable Suggestions - 0Additional Suggestions - 1
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Code Review Agent Run #dd8355Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
…mantic layers in the Backend note An extracted comment now marks an entry do-not-translate only when it carries the exact `do-not-translate` line apply_do_not_translate.py stamps. The loose phrase match still applies to translator comments, where the ru catalog's legacy "# Не переводить" marker lives. Without this, an i18n: note such as "do not translate as the animal" made the backfill skip the very entry it was meant to guide. The Backend column also lists semantic-layer types when SEMANTIC_LAYERS is enabled, so its note now names both. The template and all 30 catalogs carry the new wording, identical to what pybabel update writes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Code Review Agent Run #77e29fActionable Suggestions - 0Additional Suggestions - 1
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
…r-context-comments
CodeAnt PR Risk: Low Risk
Assessed commit: |
|
Re-triggered CI after the runner outage on |
…r-context-comments # Conflicts: # superset/translations/messages.pot
|
Merged latest master to resolve a |
| if backfill_po._developer_note(entry) | ||
| } | ||
| assert expected["Backend"] == _BACKEND_NOTE | ||
| assert len(expected) == 4 |
There was a problem hiding this comment.
These two assertions pin the catalogs to exactly four notes and to the current Backend wording. Following the new howto and adding a fifth i18n: note (or rewording one), with the template and all catalogs regenerated in sync, would still fail CI here until someone edits this test. Should the fixed examples live in a fixture, with this check asserting only that every catalog carries each note the template does?
There was a problem hiding this comment.
Agreed. 96d0540834 removes the pinned count and the pinned Backend wording, and it also covers a second case.
Most PRs commit only messages.pot. On master at 1cf9569807, every catalog lags the template by 18 msgids. A test that reads the committed catalogs fails for a contributor who adds a note this way. For a note on a new string, it failed with AttributeError.
The new test builds a template and a catalog in a temporary directory. It reads the pybabel update call from babel_update.sh, replaces only the -i and -d paths, and runs it. It then reads the note back. It covers an added, a reworded, a removed and a wrapped note. Adding -l fr to the script's update call fails all four cases. The PR description has the scenario matrix and the negative controls.
The committed-catalog test pinned four notes and the Backend wording. A contributor who added a note per the howto failed CI. Most upstream PRs commit only messages.pot, and catalogs lag it. A test on the committed catalogs also failed that workflow. For a note on a new string, it failed with AttributeError. The new test builds a template and a catalog in a temporary directory. It reads the pybabel update call from babel_update.sh and replaces only the -i and -d paths. It then reads the note back with _developer_note. It covers an added, a reworded, a removed and a wrapped note. A change to the script's update flags that stops propagation fails it. The drift check keeps the template in sync with the source. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…r-context-comments
|
Both |
pybabel drops a note after a blank line, inside the call's parentheses, or above a Python _( call whose string starts on the next line. It publishes every comment line between the i18n: line and the call. The howto and the babel_update.sh comment said that only tagged comments are extracted, which is wrong for those lines. The howto lists the placements and a grep to confirm a note landed. It no longer says the backfill note takes precedence: in a live run the note did not change the model's answer against unanimous references. Updating language files points at babel_update.sh, because a bare pybabel update leaves a python-format flag that msgfmt rejects. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Three note cases had no test that a plausible bug would fail. They are a hyphenated do-not-translate in a note, a note on a plural entry, and a new noted string. Each new case fails one mutation: a substring marker match, a singular-only note, and a union of msgids in the comment comparison. The comment-drift tests stub extract_fresh instead of subprocess.run, so they no longer snapshot the repository with git for each case. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@sadpandajoe, |
…r-context-comments
|
|
|
Thank you for persisting across several reviews, @glaterza. It's a LGTM! |
|
Thank you, @hainenber, and thank you @sadpandajoe for the patience and the review rounds that shaped the tests and the docs. |
3 participants
SUMMARY
A catalog shows a translatable string without its code. A term that is clear at the call site can get a wrong translation in the catalog.
Each of these translations is in a catalog on master:
SlugnlSlakSlugdeKopfzeileHostitOspiteBackendruДрайверBackendfrServeurRussian renders
Slugcorrectly asЧитаемый URL("readable URL").The error depends on the language and the string, so one central fix cannot correct it.
The context has to reach the catalog with the string.
This PR passes
--add-comments=i18n:topybabel extract.A comment tagged
i18n:directly above a translatable string becomes a#. i18n: ...extracted comment on that entry:pybabel updatecopies extracted comments frommessages.potinto every language catalog.The existing
#. do-not-translatemarker uses the same mechanism.Human translators see the note in the catalog they edit.
The AI backfill does not read extracted comments, so
backfill_po.pyadds the note to the prompt.pybabel extractreads comments that start withi18n:, plus any comment lines between that line and the call.It drops a note after a blank line, inside the call's parentheses, or above a Python
_(whose string starts on the next line.The howto lists these placements, because the drift check cannot see a dropped note.
The PR adds notes to three terms with wrong translations on master, and to one ambiguous phrase.
It documents the convention under Contributing Translations.
Readers of these comments
Each reader, and the check for it:
babel_update.shmessages.pot, then into every catalogcheck_pot_drift.py(CIbabel-extract)backfill_po.pydo-not-translateline counts as a markerapply_do_not_translate.pymsgid, which is valid next to a notepybabel compile,npm run build-translation.moand JSON files carry msgid and msgstr onlyfrontend-check-translationspassesNote propagation test
test_pybabel_update_carries_the_note_into_catalogsbuilds a template and a catalog in a temporary directory.It reads the
pybabel updatecall frombabel_update.shand replaces only the-iand-dpaths.It runs that call, then reads the note back with
_developer_note.It covers an added, a reworded, a removed and a wrapped note.
The test builds its own input, so a contributor's new note does not change its result.
Most upstream PRs commit
messages.potwithout the catalogs.All of the last 8 master commits that changed
messages.potcommitted no catalog (measured 2026-10-08 atc1908747b1).At that commit, every catalog lacks 21 template msgids and keeps 1 stale one.
Scenario matrix: each row is a contributor change, reverted after the run.
Columns: the drift check, the first test (pinned counts), a loosened test that was never pushed, and the fixture test.
.py, full regeneration5 == 4.tsx, full regeneration3 == 4AttributeErrorS9 fails the drift check, which is the expected result.
Negative controls on
test_pybabel_update_carries_the_note_into_catalogs:-l fradded to the update call inbabel_update.sh, 4 of 4 cases fail.--domain otheradded to that call, 4 of 4 cases fail.pybabel updateskipped, 4 of 4 cases fail._developer_notedropping continuation lines, 1 case fails._developer_notekeeping the do-not-translate marker, 1 case fails.Merging
#44395 changed
msgcatnormalization in the same script and has merged.This branch merges into master without conflicts.
Running
babel_update.shon this head changes noi18n:line and keeps all 25 do-not-translate markers.In
messages.potit changes only thePOT-Creation-Dateline.In the catalogs it adds the 7 msgids they lack at this head: 31 files, +1,038/−108.
msgfmt --check-formatreports errors in thefr,it,ptandslcatalogs.These errors are on master, and none is on a line this PR changes.
@rusackas suggested this approach in discussion #43562.
A docs PR with a Translation Guide follows after this PR merges.
It links a heading that this PR adds to
howtos.md.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before, the defect in the running app: Russian UI, Databases list.
The
Backendcolumn header reads «Драйвер» ("Driver").apache/superset:latest(6.1.0), localeru, 2026-09-17.After, the note a translator sees in every catalog (Russian shown):
Docs: Contributing → Development How-tos → Adding context for translators.
Rendered from
4bbce5afc3with the local docs server:The drift check fails on a stale note, with the source reworded and the template not regenerated.
It passes after
git checkoutrestores the source:TESTING INSTRUCTIONS
babel_update.shre-extracts from source and updates every catalog.It also adds upstream strings that the catalogs lack.
In the catalogs, the four notes are this PR's only change.
In the template, it also updates
POT-Creation-Date.msgcat --no-wrapjoins one msgid that master committed across three lines.The tests, and a negative control for each fix:
pr-verifydeselectedtests/unit_tests/scripts/translations/babel_update_test.py::test_the_trailing_line_strip_keeps_the_last_entry_intactfor this head.On a machine with gettext, that test also fails.
That test comes from master (#44395) and is not part of this PR.
Its restricted
PATHhas nopython, whichbabel_update.shcalls.CI skips it, because the Python-Unit runner has no gettext.
Restore files with
git checkout, as shown, and not by moving a backup file back.check_pot_drift.pycallsgit stash create.That command exits 1 when a rewrite left a tracked file's content identical.
The check then stops with a
CalledProcessError.This behavior is on master, independent of this PR.
Named tests:
test_pybabel_update_carries_the_note_into_catalogs:pybabel updatecarries a note into a catalog.test_extraction_carries_i18n_comments_to_the_template: a real extraction keeps tagged comments and drops untagged ones.test_extract_flags_match_babel_update_shtest_is_do_not_translate_ignores_prose_in_an_i18n_notetest_backfill_skips_do_not_translate_entries_end_to_endLive AI run (2026-10-08).
translate_batchtranslated ruBackend6 times per case with the script's default model,claude-sonnet-4-6.Treiber, frServeur, it and pt_BRDriver, esControlador)Драйвер6 of 6Драйвер5 of 6,База данных1Драйвер6 of 6Бэкенд6 of 6Бэкенд6 of 6The note did not change the model's answer in either case.
An earlier round reported 3 of 6 without the note and 0 of 6 with it; this run does not reproduce that result.
The notes inform human translators in every catalog.
Making the prompt follow a note over unanimous references is a separate change.
ADDITIONAL INFORMATION
🤖 Generated with Claude Code