Skip to content

fix: route server-composed and split-key strings through the i18n catalogs - #312

Open
antosubash wants to merge 1 commit into
mainfrom
fix/i18n-composed-strings
Open

fix: route server-composed and split-key strings through the i18n catalogs#312
antosubash wants to merge 1 commit into
mainfrom
fix/i18n-composed-strings

Conversation

@antosubash

Copy link
Copy Markdown
Owner

Closes #296

Both lint checks parse .tsx. A sentence assembled in Python reaches the page already rendered, and a sentence spliced from two catalog keys around a value is two fragments a translator cannot reorder — neither is visible to either check, so both drifted untranslated with nothing to flag them.

What changed

Server-composed sentences. The users "Recent activity" card built English in Python (f"{verb} {kind} {label}" from an English verb table), so the list read as English in every locale. It now emits a catalog key plus its interpolation arguments and resolves them against the request's Translator — the same shape menus use (label_key + MenuRegistry.get_for_user(translate=…)). The row's kind goes through the owning module's AuditLink.label_key rather than its English label. A unit test translates with a stub that echoes key(args), so an f-string reintroduced here fails loudly instead of shipping English.

Sentence fragments as keys. permissions.edit.granted_summary was "of {total} granted" with the count rendered beside it; the 403 page spliced error.forbidden_permission_prefix + the permission + …_suffix. Both are now single keys with an ordinary {granted} / {permission} placeholder.

A shared way to put markup inside a translated sentence. New InterpolatedText (packages/ui) renders one key and drops the styled value at the placeholder, wherever the translation moved it. feature_flags' ScopeCard had solved this locally by interpolating a Word Joiner and splitting on it; it now uses the shared component. The sentinel never reaches a catalog either way (the translator only ever sees {tenant_id}), but the shared version also handles a translation that dropped the placeholder — it appends the value rather than silently deleting it — and one that repeated it, keeping every word of the copy.

Upload failure copy. A 413 answered "File exceeds the maximum allowed size", which tells someone holding a 40 MB file nothing actionable. The message names the limit (interpolated by the catalog), and the response carries max_bytes alongside for callers composing their own copy. format_bytes mirrors pages/format.ts, including its half-up rounding, so one limit is never spelled two ways on one screen.

host/locales/es.json shipped 25 setup.* keys still in English — translated.

Nothing was checking locale parity. SM013–SM016 only ran when SM_I18N_SUPPORTED_LOCALES was set, and it is set in neither .env nor .env.example, so make doctor was green throughout. Rather than only documenting the variable, the diagnostic now falls back to the locale files each namespace actually ships when no set is declared: SM014/SM015 hold every translation to the default catalog's key set, while SM013 stays quiet, because a locale nothing promised cannot be missing. Declaring the set still turns SM013 back on.

That fallback immediately reported the issue's fourth bullet: four orphan relative_time.{months,years}_ago_{one,other} keys in packages/ui/locales/es.json.

Two deliberate departures from the issue text

  1. The orphan ui keys are declared in en.json, not deleted from es.json. They are not dead: useRelativeTime calls t(key, { count }), and i18next resolves _one/_other before falling back to the stem. Deleting them would regress "hace 1 mes" to "hace 1 meses". Adding the English forms clears SM015 and keeps the Spanish singular.

  2. SM_I18N_SUPPORTED_LOCALES is documented in .env.example but left commented out. Setting it live would make a copied .env override the value edited at /admin/settings — the precedence trap CLAUDE.md calls out for i18n_*. The auto-detect fallback gets make doctor checking parity on a stock checkout with no configuration at all, which is what the bullet was after.

The issue also says the Word Joiner "is an invisible character a translator can delete or duplicate with no feedback". That part is not accurate — it is passed as the value of an ordinary {tenant_id} placeholder and never appears in a catalog. The real fragility was the silent truncation on a mis-translated placeholder, which the shared component now handles.

Verification

All run in the worktree after uv sync --all-packages + npm install, exit codes read directly (nothing piped through tail that would mask them).

Command Result
uv run pytest -q (full suite) 2811 passed, 60 deselected, exit 0
uv run ruff format --check . exit 0
uv run ruff check . All checks passed, exit 0
uv run ty check All checks passed, exit 0
uv run python scripts/check_file_size.py no files exceed 300 lines, exit 0
npx biome check . 334 files checked, exit 0
make ci-js-typecheck (host + every module/package tsconfig) exit 0
npx vitest run (full suite) 398 passed in 48 files, exit 0
node scripts/check_untranslated_strings.mjs no untranslated strings, exit 0
uv run python scripts/check_hardcoded_strings.py exit 0
uv run python scripts/check_metadata.py / check_readmes.py exit 0
make doctor 0 errors, 0 warnings, 0 info

make doctor was additionally probed by injecting a bogus key into packages/ui/locales/es.json; it reported SM015 … has keys not in default: __probe__ and went quiet again when reverted — so the new fallback is genuinely running, not merely silent.

Not done

  • No browser pass. Every touched screen (403 page, role editor, feature-flag scope card, file upload card, users edit page) is covered by unit/integration tests, but I did not render them in a browser.
  • _kind_of still lowercases the translated kind label for mid-sentence use. Correct for English and Spanish, wrong for a language that capitalises common nouns; documented in the docstring, which points such a locale at rephrasing the whole summary.* clause instead.
  • scripts/check_untranslated_strings.mjs still cannot see strings behind a variable or a config object. Widening it is a separate change.

…alogs

The `hardcoded-strings` and `untranslated` checks both parse `.tsx`. A sentence
assembled in Python arrives at the page already rendered, and a sentence spliced
from two catalog keys around a value is two fragments a translator cannot
reorder — neither is visible to either check, so both drifted untranslated.

- Users' "Recent activity" card composed English in Python
  (`f"{verb} {kind} {label}"` from an English verb table). It now emits a
  catalog key plus its interpolation arguments and resolves them against the
  request's `Translator`, the same shape menus use via `label_key` +
  `MenuRegistry.get_for_user(translate=…)`. The row's *kind* goes through the
  owning module's `AuditLink.label_key` rather than its English `label`.
  A unit test translates with a stub that echoes keys, so an f-string here
  fails loudly rather than silently shipping English.

- `permissions.edit.granted_summary` was the fragment "of {total} granted" with
  the count rendered beside it, and the 403 page spliced
  `error.forbidden_permission_prefix` + the permission + `…_suffix`. Both are
  now single keys with an ordinary `{granted}` / `{permission}` placeholder,
  rendered by a new shared `InterpolatedText`, which puts the styled value at
  the placeholder wherever the translator moved it. `feature_flags`' ScopeCard
  had solved this locally by splitting on a Word Joiner; it now uses the shared
  component, which also handles a translation that dropped or repeated the
  placeholder instead of silently truncating.

- A 413 answered "File exceeds the maximum allowed size", which tells someone
  holding a 40 MB file nothing they can act on. The message now names the limit
  (interpolated by the catalog) and the response carries `max_bytes` alongside.
  `format_bytes` mirrors `pages/format.ts` — including its half-up rounding, so
  one limit is not spelled two ways on one screen.

- `host/locales/es.json` shipped 25 `setup.*` keys still in English; translated.

- Nothing was checking locale parity: SM013–SM016 only ran when
  `SM_I18N_SUPPORTED_LOCALES` was set, and it is set in neither `.env` nor
  `.env.example`, so `make doctor` was green while drift accumulated. The
  diagnostic now falls back to the locale files each namespace actually ships
  when no set is declared — SM014/SM015 hold every translation to the default
  catalog, while SM013 stays quiet, since a locale nothing promised cannot be
  missing. That immediately surfaced four orphan `relative_time.*_ago_{one,other}`
  keys in `packages/ui/locales/es.json`; they are declared in `en.json` rather
  than deleted, because i18next resolves them when `count` is passed and
  removing them would regress "hace 1 mes" to "hace 1 meses".

`SM_I18N_SUPPORTED_LOCALES` is documented in `.env.example` but left commented:
like every `SM_` variable it wins over the value edited at /admin/settings, and
the parity check no longer needs it.

Closes #296
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 4, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
🔒 Security Review Completed 2026-09-04T22:15:12.567473Z c5ec235 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying simple-module-python with  Cloudflare Pages  Cloudflare Pages

Latest commit: c5ec235
Status: ✅  Deploy successful!
Preview URL: https://ecfaddef.simple-module-python.pages.dev
Branch Preview URL: https://fix-i18n-composed-strings.simple-module-python.pages.dev

View logs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Server-composed and split-key strings bypass the i18n catalogs

1 participant