Skip to content

fix(i18n): guard messages.pot against source drift and regenerate it - #44467

Merged
rusackas merged 3 commits into
masterfrom
ultraset-fix-44403
Sep 21, 2026
Merged

rusackas merged 3 commits into
masterfrom
ultraset-fix-44403

Conversation

@sadpandajoe

Copy link
Copy Markdown
Member

SUMMARY

superset/translations/messages.pot is the extraction template every language
catalog is generated from, but nothing in CI ever compared the committed
template against a fresh extraction. The babel-extract job regenerates
messages.pot from scratch in throwaway worktrees before comparing
translated/fuzzy counts — a check scoped to catching a reworded source string
invalidating an existing translation, not template/source parity. The template
had drifted 432 strings behind source and carried 109 stale entries, so those
432 strings could not reach any of the 30 .po catalogs and were untranslatable
in every language.

This PR does three things.

1. Adds a drift guard. scripts/translations/check_pot_drift.py re-extracts
from source with the same pybabel extract invocation as babel_update.sh and
fails when the committed template's msgid set differs from a fresh extraction.
It is wired into the babel-extract job as a hard gate, placed before
babel_update.sh overwrites the workspace template. Comparison is on msgid sets
only, so it is insensitive to POT-Creation-Date churn and to the
do-not-translate / python-format post-processing steps. Line wrapping is
deliberately not normalized away: gettext continuation lines concatenate back
to the exact original, so any whitespace difference that survives parsing is a
real source reword that gettext's exact-match lookup would miss.

2. Unbreaks extraction of the explore additional-actions menu.
useExploreAdditionalActionsMenu/index.tsx was almost entirely invisible to
pybabel extract: return /[",\r\n]/.test(s) ? ... : s; puts a regex literal
directly after return, and babel's JavaScript lexer reads that / as
division, so the " inside the character class opens a phantom string literal
that swallows the next ~290 lines. Only 3 of the module's 34 translatable
strings survived extraction. The stale template still carried the other 31 only
because it predated the file's current shape — so regenerating it would have
deleted 10 still-live, already-translated strings ("Export All Data", "Export
Current View", "New chart", "Superset Chart", …) from all 30 catalogs. Hoisting
the regex into a module constant fixes extraction with no behavior change.

3. Regenerates the template and all 30 catalogs via babel_update.sh's own
sequence, so the new guard starts green. Adding the guard without regenerating
would fail CI immediately against the already-stale template.

Regeneration changes zero existing translation values (verified by parsing
both revisions with babel.messages.pofile.read_po for all 30 catalogs, and
independently re-verified in review): the diff is additions plus the removal of
100 msgids that no longer exist in source, none of which still appear inside a
t()/_()/__()/tct() call. The it/messages.po catalog's diff looks large
because babel_update.sh also normalizes it from CRLF to LF line endings
(master's copy was the only CRLF-encoded catalog) — that's a whitespace-only
normalization, not a retranslation; the zero-changed-values check above covers
it.

Note: babel_update.sh's msgcat --sort-by-msgid --no-wrap step is a no-op in
CI today — --sort-by-msgid is not an msgcat option, the script has no
set -e, so msgcat exits without writing. This PR reproduces current CI output
rather than introducing a new convention; the flag itself is addressed
separately in #44395. Its other two effects are redundant, since --sort-output
and --no-location are already on the pybabel extract call.

Out of scope, left untouched (separate, pre-existing issues): flask_babel.lazy_gettext call sites (e.g. superset/views/filters.py:330) are not reachable by pybabel extract at all, since neither babel's default keywords nor this project's -k list include lazy_gettext — worth its own follow-up issue.

Fixes #44403

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

Guard passes on this branch:

python scripts/translations/check_pot_drift.py
# superset/translations/messages.pot matches a fresh extraction.

Guard catches the bug it exists for — restore the pre-fix template and re-run:

git show master:superset/translations/messages.pot > superset/translations/messages.pot
python scripts/translations/check_pot_drift.py
# exits 1: strings missing/stale
git checkout -- superset/translations/messages.pot

Unit tests (mocked cases plus a real end-to-end extraction against this
checkout):

pytest tests/unit_tests/scripts/translations/

Frontend behavior of the hoisted regex is unchanged:

cd superset-frontend
npm run test -- src/explore/components/useExploreAdditionalActionsMenu

Regeneration is idempotent — re-running ./scripts/translations/babel_update.sh
produces no diff beyond POT-Creation-Date.

ADDITIONAL INFORMATION

sadpandajoe and others added 2 commits September 21, 2026 00:44
superset/translations/messages.pot is the extraction template every
language catalog is generated from, but nothing in CI ever compared the
committed template against a fresh extraction — the babel-extract job
regenerates messages.pot from scratch in throwaway worktrees before
comparing translated/fuzzy counts (.github/workflows/superset-translations.yml),
a check scoped to catching a reworded source string invalidating an
existing translation, not to template/source parity. The template had
drifted 432 strings behind source and carried 109 stale entries, so those
432 strings could not reach any of the 30 .po catalogs and were untranslatable
in every language.

Add scripts/translations/check_pot_drift.py, which re-extracts from source
with the same pybabel invocation as babel_update.sh and fails when the
committed template's msgid set differs from a fresh extraction (ignoring
whitespace-only wrapping differences). Wire it into the babel-extract CI
job as a hard gate. Regenerate messages.pot and propagate to all 30 .po
catalogs via babel_update.sh's extract/apply-do-not-translate/update
sequence so the new guard starts green; adding the guard alone, without
regenerating, would immediately fail CI against the already-stale template.

Fixes #44403

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…e drift guard

Two follow-ups to the messages.pot drift guard.

1. `useExploreAdditionalActionsMenu/index.tsx` was almost entirely invisible
   to `pybabel extract`: its `return /[",\r\n]/.test(s) ? ... : s;` puts a
   regex literal directly after `return`, and babel's JavaScript lexer reads
   that `/` as division, so the `"` inside the character class opens a phantom
   string literal that swallows the next ~290 lines of the file. Only 3 of the
   module's 34 translatable strings survived extraction. The committed
   template still carried the missing 31 only because it predated the file's
   current shape — so regenerating it (as the guard requires) deleted 10
   still-live, already-translated strings from all 30 catalogs, among them
   "Export All Data", "Export Current View" and "New chart". Hoist the regex
   into a module constant so the lexer never sees it in `return` position, and
   regenerate from the base catalogs so those translations are preserved
   rather than dropped. No behavior change: the hoisted literal is identical.

2. `check_pot_drift.py` collapsed whitespace in every msgid before comparing.
   That was unnecessary — gettext continuation lines concatenate back to the
   exact original, so `read_po` already yields identical msgids for a wrapped
   and an unwrapped copy of the same string — and harmful: it silently
   accepted a whitespace-only source reword ("Save  chart" -> "Save chart"),
   which gettext's exact-match lookup treats as a completely different message
   and which therefore leaves the template and every catalog stale. Drop the
   normalization and cover the case with a test.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@bito-code-review

bito-code-review Bot commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Bito Automatic Review Skipped - Large PR

Bito didn't auto-review this change because the pull request exceeded the line limit. No action is needed if you didn't intend for the agent to review it. Otherwise, to manually trigger a review, type /review in a comment and save.

@github-actions github-actions Bot added i18n Namespace | Anything related to localization i18n:spanish Translation related to Spanish language i18n:italian Translation related to Italian language i18n:french Translation related to French language i18n:chinese Translation related to Chinese language i18n:japanese Translation related to Japanese language i18n:russian Translation related to Russian language i18n:korean Translation related to Korean language github_actions Pull requests that update GitHub Actions code i18n:dutch i18n:slovak i18n:ukrainian i18n:portuguese i18n:brazilian i18n:traditional-chinese labels Sep 21, 2026
@github-actions github-actions Bot added i18n:persian risk:ci-script PR modifies scripts that execute in CI (supply chain risk) i18n:czech i18n:latvian labels Sep 21, 2026
@codecov

codecov Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.65%. Comparing base (c9fd9bf) to head (a26a3a2).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #44467      +/-   ##
==========================================
- Coverage   80.65%   80.65%   -0.01%     
==========================================
  Files        2942     2942              
  Lines      175691   175692       +1     
  Branches    40788    40788              
==========================================
  Hits       141703   141703              
- Misses      31326    31327       +1     
  Partials     2662     2662              
Flag Coverage Δ
hive 37.15% <ø> (ø)
javascript 76.11% <100.00%> (-0.01%) ⬇️
mysql 56.37% <ø> (ø)
postgres 56.39% <ø> (-0.01%) ⬇️
presto 39.03% <ø> (ø)
python 84.96% <ø> (ø)
sqlite 56.10% <ø> (ø)
unit 76.77% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

The prior regeneration (549dc5b) ran pybabel extract under Python 3.12
and Babel 2.18.0 instead of this project's pinned Python 3.11 / Babel
2.17.0. Python 3.12's PEP 701 f-string tokenization changes how Babel's
extractor reads the nested-quote f-string at
superset/common/query_context_processor.py:888, producing a spurious
msgid "valuename" that does not correspond to any real translatable
string. CI runs on Python 3.11 and correctly does not extract it, so the
new drift guard (check_pot_drift.py) flagged the committed template as
stale relative to a genuine extraction.

Regenerate messages.pot and all 30 .po catalogs under Python 3.11 with
Babel 2.17.0 pinned, matching CI and the committed Generated-By header.
The diff is exactly the removal of the one spurious entry from every
catalog; no other msgid changed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@rusackas rusackas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Nicely researched fix, and I appreciate the real extraction test on top of the mocked edge cases.

@rusackas
rusackas merged commit 3756b7a into master Sep 21, 2026
84 checks passed
@rusackas
rusackas deleted the ultraset-fix-44403 branch September 21, 2026 23:20
villebro pushed a commit that referenced this pull request Sep 22, 2026
…44467)

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
(cherry picked from commit 3756b7a)
villebro added a commit that referenced this pull request Sep 22, 2026
Native run of scripts/translations/babel_update.sh, superseding the
cherry-pick of #44509 (pure master-source regeneration, skipped) and
reconciling the auto-merged .po diffs from #44467/#44144/#44513/
#44286/#44508 with 7.0's actual translatable strings.
rusackas added a commit that referenced this pull request Oct 1, 2026
babel-extract runs check_pot_drift.py, added in #44467 to fail a PR that
adds or renames a translatable string without regenerating
messages.pot. It is not currently in .asf.yaml's required contexts, so
it can be red (or, on a PR whose branch predates the script's own
existence, error outright) without blocking merge.

That gap let #44465 land 18 minutes after #44467 regenerated the
template, reintroducing drift immediately: #44465's branch was cut
before #44467 merged, so check_pot_drift.py did not exist on it yet,
and nothing required babel-extract to be green (or even present) on
the up-to-date branch before allowing the merge. #44509 cleaned up the
resulting drift.

babel-extract is also the only translation-drift check gated on both
Python and frontend changes (unit-tests-required, which independently
covers check_pot_drift.py via check_pot_drift_test.py, only runs when
a PR touches Python), so it is the one that has to be required for a
frontend-only string addition to be caught at all.

This does not by itself close the race that let #44465 through -- that
needs required_status_checks.strict: true, a separate, repo-wide
decision left for a follow-up PR stacked on this one. This PR only
makes the existing check_pot_drift.py gate binding on the PR's own
branch.

Co-Authored-By: Evan Rusackas <evan@preset.io>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

github_actions Pull requests that update GitHub Actions code i18n:brazilian i18n:chinese Translation related to Chinese language i18n:czech i18n:dutch i18n:french Translation related to French language i18n:italian Translation related to Italian language i18n:japanese Translation related to Japanese language i18n:korean Translation related to Korean language i18n:latvian i18n:persian i18n:portuguese i18n:russian Translation related to Russian language i18n:slovak i18n:spanish Translation related to Spanish language i18n:traditional-chinese i18n:ukrainian i18n Namespace | Anything related to localization risk:ci-script PR modifies scripts that execute in CI (supply chain risk) size/XXL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

i18n: messages.pot is 428 strings behind source, so those strings cannot be translated in any language

2 participants