Skip to content

Localization: stage translations into the String Catalog (manual)#25713

Open
jkmassel wants to merge 2 commits into
trunkfrom
jkmassel/catalog-strings-translation
Open

Localization: stage translations into the String Catalog (manual)#25713
jkmassel wants to merge 2 commits into
trunkfrom
jkmassel/catalog-strings-translation

Conversation

@jkmassel

@jkmassel jkmassel commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Fills the app's String Catalog (Localizable.xcstrings) with translations — the human translations we already have from GlotPress, plus machine translations for whatever is still untranslated. This gets the catalog ready ahead of moving the app over to String Catalogs. Nothing users see changes.

Two things to know

Staged, not shipped. The catalog file is generated, but the app doesn't read it at runtime yet — regular strings still come from the existing Localizable.strings. This only pre-fills the catalog for the eventual switch-over. Machine translations are marked needs_review, so they stay clearly separate from reviewed human translations and can sit in the catalog without shipping to anyone.

Manual, not automated. These lanes are run by hand, not wired into any release or CI step: a run extracts strings, calls a paid translation API, and commits a large file, so it shouldn't happen on its own. The unit tests still run in CI.

How it works

Three steps, each runnable on its own:

  1. generate_strings_catalog — extracts the English strings from the code into the catalog.
  2. download_catalog_strings — downloads the current translations from GlotPress.
  3. localize_catalog — adds the human translations, machine-translates whatever's left, and commits.

Each string gets the best translation available in each language: a human translation if there is one, otherwise a machine translation, otherwise the English text. Human translations are marked translated; machine translations and English fallbacks are marked needs_review.

Re-running is cheap. A machine translation already in the catalog is left alone instead of being requested again, and a human translation always replaces a machine one on the next run. Machine translation only happens when an API key is set — without one, the catalog fills from human translations and English. A machine translation is kept only if its format placeholders (%1$@, %2$d, …) match the English, so a mangled one falls back to English rather than shipping a broken string.

Test plan

  • Unit tests pass (fastlane/lanes/*_test.rb) — these run in CI
  • rubocop clean
  • Manual run against live GlotPress data (steps below)

Manual run

First bundle install. A machine-translation run also needs an ANTHROPIC_API_KEY — the Fastfile picks it up from ~/.wpios-env.default. Without a key the run still works: it fills in the human translations and English only, skips machine translation, and costs nothing — a good way to check the plumbing before spending anything.

Run the three lanes, scoped to one well-translated locale to keep it cheap:

bundle exec fastlane ios generate_strings_catalog
bundle exec fastlane ios download_catalog_strings locales:fr
bundle exec fastlane ios localize_catalog locales:fr
  1. generate_strings_catalog extracts the English strings into Localizable.xcstrings (~3,900 of them). No network, no cost.
  2. download_catalog_strings downloads the French translations from GlotPress into a scratch folder. No cost.
  3. localize_catalog fills the French translations into the catalog, machine-translates the gaps, and commits.

French is well-translated, so most entries come from GlotPress and only a small number fall to machine translation, which keeps the cost for one locale low. A few entries may stay English where a machine translation was rejected for not matching the English placeholders.

localize_catalog prints a per-locale summary at the end, so you can check the result straight from the log — no need to open the catalog:

fr: 3923 entries — 3805 human, 112 machine, 6 still English
    editor.post.delete: "Delete" → "Supprimer"
    settings.title: "Settings" → "Réglages"

Expect mostly human, a smaller machine count, and only a few still English. The sample lines let you spot-check the machine translations for obvious nonsense.

localize_catalog commits the catalog, which is a large file. This run is only to check the output and cost, so discard the commit rather than pushing it (git reset --hard HEAD~1). The catalog gets committed for real during the migration, not in this PR.

Plurals are handled the same way in a separate PR (#25710), since they live in their own catalog file.

@dangermattic

dangermattic commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator
2 Warnings
⚠️ This PR is larger than 500 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.
⚠️ This PR is assigned to the milestone 27.1. This milestone is due in less than 4 days.
Please make sure to get it merged by then or assign it to a milestone with a later deadline.

Generated by 🚫 Danger

Base automatically changed from jkmassel/claude-string-translation to trunk June 30, 2026 23:07
@jkmassel
jkmassel force-pushed the jkmassel/catalog-strings-translation branch from d5697f8 to a45aaf1 Compare July 1, 2026 01:27
@jkmassel jkmassel changed the title Localization: stage regular-string translations into Localizable.xcstrings (manual) Localization: stage translations into the String Catalog (manual) Jul 6, 2026
@jkmassel
jkmassel force-pushed the jkmassel/catalog-strings-translation branch 3 times, most recently from b558570 to c5a43d9 Compare July 14, 2026 23:15
Adds the localize half of the String Catalog pipeline: a manual (non-CI) lane that folds current GlotPress translations for regular, non-plural strings into the existing Localizable.xcstrings as `human ?? AI ?? English`. The catalog isn't the runtime store yet (the app still ships Localizable.strings), so this only pre-populates it for the eventual cutover — nothing users see changes.

- `localize_catalog` lane: download current GlotPress translations into a throwaway dir, fold them in (human => `translated`), then AI-fill the rest (=> `needs_review`). Manual only — it calls the translation API (cost) and commits a large catalog; never wired into `download_localized_strings` or CI.
- `CatalogStrings.fold_translations!`: pure, reuse-aware fold. An existing valid cell is kept, so re-runs only translate genuinely-new gaps — the catalog's `needs_review` state IS the persistence (no side-store). A human translation from GlotPress supersedes a kept cell on the next fold.
- Immutable-key enforcement replaces `reconcile_changed_sources`: reworded English in place is now a hard error, since `xcstringstool sync` silently keeps the old key's translations and would ship stale text. Rewording requires a new key.
- `translate_all` keeps completed batches when one fails mid-run, so a partial API failure degrades to English for the remainder instead of dropping the lot.
- docs/localization-pipeline.md documents the regular-string fold and the `human ?? AI ?? English` floor — any translation, human or machine, that fails the placeholder gate falls through to the next rung.
@jkmassel
jkmassel force-pushed the jkmassel/catalog-strings-translation branch from c5a43d9 to cd9b85f Compare July 16, 2026 18:22
@jkmassel jkmassel self-assigned this Jul 16, 2026
@jkmassel jkmassel added this to the 27.1 milestone Jul 16, 2026
@jkmassel jkmassel added Tooling Build, Release, and Validation Tools [Type] Enhancement labels Jul 16, 2026
@jkmassel
jkmassel marked this pull request as ready for review July 16, 2026 18:48
@jkmassel
jkmassel requested a review from a team as a code owner July 16, 2026 18:48

@oguzkocer oguzkocer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I've completed my first pass on this and found one issue reported by Claude and opened a PR to add a couple failing tests for it: #25802

I didn't see anything in my manual review, but I also haven't spent a significant time on it yet. If it's not reviewed by anyone else, I'll pick this up again once #25802 is addressed.

…tions (#25802)

* Add failing tests: placeholder gate rejects valid reordered human translations

TranslationValidator compares a source's positional and sequential specifier
views independently, so it only permits reordering when the source is ALREADY
positional. A non-positional source (`%@ … %@`) whose translation reorders via
positional specifiers (`%2$@ … %1$@`) is rejected — even though that is the
standard, Apple-documented iOS reordering mechanism and is correct at runtime
(`String(format:)` honors positional specifiers regardless of the source shape).

The human-translation gate added for the String Catalog fold
(`CatalogStrings.trusted_human`, `PluralStrings.human_forms_for`) inherits this,
so it rejects and downgrades valid, currently-shipping human translations to
machine/English. This is not hypothetical: the committed translated `.strings`
contain 23 key-as-source strings across 34 locales (513 cells) that positionalize
a bare-`%@` English source this way, e.g. ar "%@ of %@ used on your site" =>
"%1$@ من %2$@ على موقعك".

Two failing tests pin the correct behavior:
- translation_validator_test.rb — the root cause at the validator.
- catalog_strings_helper_test.rb — the user-facing downgrade in the fold.

The fix belongs in TranslationValidator (shared by AI, plurals, and regular
human strings): treat a non-positional source as implicitly numbered 1..N by
appearance order and accept a fully-positional candidate whose index→type map
matches. Existing tests already keep genuine breakage (sequential flip, type/
count change) rejected, so a correct fix cannot just drop the distinction.

* Fix placeholder gate: accept positional reorder of a non-positional source

TranslationValidator compared a source's positional and sequential specifier
views independently, so it only permitted reordering when the source was ALREADY
positional. A non-positional source (`%@ … %@`) whose translation reorders via
positional specifiers (`%2$@ … %1$@`) was rejected — even though that is the
standard, Apple-documented iOS mechanism and is correct at runtime, because
`String(format:)` honors positional specifiers regardless of the source's shape.

Normalize a purely-sequential signature into the implicit 1..N positional map its
bare specifiers already carry (`%@ %d` => `{1 => object, 2 => int}`) before
comparing, applied to both source and candidate. Once both sides are index→type
maps, `%@ - %@` and `%2$@ - %1$@` compare equal and the reorder is accepted.

Genuine breakage stays rejected: a sequential flip `%@: %d` => `%d : %@` yields
`{1 => object, 2 => int}` vs `{1 => int, 2 => object}` — a disagreement on which
index owns which type — and type/length/count changes are unaffected. A signature
that already uses positional specifiers, or one that mixes bare and positional
specifiers (malformed), is left unchanged, so mixed strings still require an exact
structural match.

Fixes the two failing tests added in the previous commit and propagates to all
three gate consumers (AI, plurals, regular human strings) via the shared
validator. All 7 fastlane/lanes suites pass (113 tests); rubocop clean.

* Add failing tests: gate and coverage compare miss format-argument edge cases

Three currently-red tests documenting real defects found auditing the
placeholder gate and the build-free coverage compare:

- A candidate that binds the same positional index to two different types
  (%1$@ %1$d) is accepted: signature() collapses it last-wins, and this PR's
  implied_positional widened the reach to bare-specifier sources (%d), which
  regressed from reject to accept.
- Dynamic field width/precision (%*d, %.*f) consume an extra int vararg the
  signature never counts, so %d and %*d reduce to the same signature.
- coverage_gap conflates keys that differ only in argument type (%d days vs
  %@ days), masking a genuinely dropped key behind a same-prose sibling.

* Fix placeholder gate: reject a positional index reused with a conflicting type

signature() stored positional specifiers in a Hash keyed by index with plain
assignment, so a second reference to the same index (`%1$@` then `%1$d`)
silently overwrote the first, collapsing an object/int conflict to one token.
A candidate that references argument 1 as both an object and an int passes an
int to a `%@` conversion at runtime — a wrong-vararg read. This PR's
implied_positional widened the reach: a bare-specifier source like `%d` now
normalizes to {1 => int} and matched the collapsed candidate, regressing that
case from reject to accept.

Record a conflicting reuse as a distinct token so it can never equal a
well-formed source's single type, while still allowing a consistent reuse
(`%1$d … %1$d`, which legally reads one argument twice).

* Fix placeholder gate: count dynamic field width/precision arguments

A `*` in a specifier's field width or precision (`%*d`, `%.*f`) consumes its
own int argument before the value, so `%d` and `%*d` are not interchangeable.
The FORMAT_SPECIFIER regex matched the `*` but signature() emitted no token
for it, so the two reduced to the same signature and a translation could add
or drop a dynamic-width/precision argument undetected — reading one vararg
past what the call site supplies. Emit an int argument for each `*`, in
appearance order before the value.

* Fix coverage compare: keep argument type when canonicalizing keys

canonical() replaced every format specifier with one sentinel regardless of
type, so keys differing only in argument type (`%d days` vs `%@ days`)
collapsed to the same coverage cell — a genuinely-dropped key was masked by a
same-prose sibling of a different type, defeating the same-basename overwrite
regression the coverage gate exists to catch. Strip only the positional index
(the sole difference between the genstrings source-form `%li` and the
xcstringstool-normalized `%1$li` for the same source literal), so `%li` and
`%1$li` still compare equal while `%d` and `%@` stay distinct.

* Satisfy RuboCop: extract signature helpers and merge the catalog test class

Danger's RuboCop pass flagged two offenses from the previous commits:

- Metrics/AbcSize: signature() exceeded the ABC limit once it gained the
  positional-conflict and dynamic-width handling. Extract record_specifier
  and add_positional so signature() is a thin loop again; behavior unchanged.
- Style/OneClassPerFile: the coverage_gap test added a second top-level class
  to catalog_helper_test.rb. Fold it into the file's single test class,
  renamed CatalogHelperTest since it now covers both reword detection and the
  coverage compare.

---------

Co-authored-by: Jeremy Massel <1123407+jkmassel@users.noreply.github.com>
@jkmassel

Copy link
Copy Markdown
Contributor Author

@oguzkocer – I fixed that issue (and a couple of related ones) in #25802 then landed it into this PR.

Comment on lines +78 to +80
# --- Failing tests: gaps surfaced by the pipeline-breaker audit (currently RED; each documents a real defect
# where the gate violates its own "same count, same types, same index→type mapping" invariant) -----------

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@mokagio mokagio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I asked Opus 4.8 for help in reviewing this PR. Of the various points it raised, the one that stood out to me regards the consequences of a no-API-key run.

Implicit tradeoff: the fold materializes a cell for every (key, locale) pair — 141,240 cells, ~22 MB committed — and the "still English" count in the run summary is derived from those materialized cells rather than computed. So the storage decision and the reporting decision are silently coupled, and nothing in the PR says so.

Where it bites today: the no-API-key run the PR recommends as the cheap plumbing check writes 141,240 English-fallback cells and commits 21.5 MB of zero-information JSON. In the intended mode (AI on) this is a non-issue — English fallbacks drop to zero.

Suggestion: don't commit when the AI tier is disabled (or warn loudly), and make summarize derive english: as translatable_total − human − machine so a future prune of English-fallback cells doesn't silently zero out that number.

ai_translator = catalog_ai_translator
written = CatalogStrings.fold_translations!(catalog, ..., ai_translator: ai_translator)
...
if ai_translator.nil? && options[:commit].to_s != 'true'
  UI.important('Skipping the commit: no AI tier, so every gap cell is an English placeholder. ' \
               'Re-run with ANTHROPIC_API_KEY set, or pass commit:true to stage the human-only fold.')
  next
end

git_add(path: LOCALIZABLE_CATALOG, shell_escape: false, force: true)
git_commit(...)

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

Labels

Tooling Build, Release, and Validation Tools [Type] Enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants