wordpress-mobile
diff --git a/‎docs/localization-pipeline.md‎
Lines changed: 13 additions & 6 deletions b/‎docs/localization-pipeline.md‎
Lines changed: 13 additions & 6 deletions
diff --git a/‎fastlane/lanes/ai_translator.rb‎
Lines changed: 20 additions & 2 deletions b/‎fastlane/lanes/ai_translator.rb‎
Lines changed: 20 additions & 2 deletions
diff --git a/‎fastlane/lanes/ai_translator_test.rb‎
Lines changed: 42 additions & 0 deletions b/‎fastlane/lanes/ai_translator_test.rb‎
Lines changed: 42 additions & 0 deletions
diff --git a/‎fastlane/lanes/catalog_helper.rb‎
Lines changed: 24 additions & 61 deletions b/‎fastlane/lanes/catalog_helper.rb‎
Lines changed: 24 additions & 61 deletions
diff --git a/‎fastlane/lanes/catalog_helper_test.rb‎
Lines changed: 56 additions & 0 deletions b/‎fastlane/lanes/catalog_helper_test.rb‎
Lines changed: 56 additions & 0 deletions
@@ -2,7 +2,7 @@
 
 How user-facing strings get from English source into every shipped locale. This is the **release/tooling** view (the fastlane lanes under `fastlane/lanes/`); for how to *write* localizable strings in app code, see [localization.md](./localization.md).
 
-> The contract for every shipped string is **`human ?? AI ?? English`**: a human (GlotPress) translation if one exists, otherwise a machine translation, otherwise the English source. Nothing ships a broken placeholder — machine output that fails the format-specifier gate falls back to English.
+> The contract for every shipped string is **`human ?? AI ?? English`**: a human (GlotPress) translation if one exists, otherwise a machine translation, otherwise the English source. Nothing ships a broken placeholder — **any** translation, human or machine, that fails the format-specifier gate is rejected and falls through to the next rung.
 
 ## The round trip
 
@@ -45,13 +45,20 @@ The plural reverse-fold (`PluralStrings.fold_translations!`) fills each `(key, l
 
 > **This does not ship machine translations yet.** `Plurals.xcstrings` is built into the app but **not consumed at runtime** — nothing reads the catalog, and nothing references its keys yet; the app still renders plurals the legacy way. The fold *pre-populates* the catalog so it's ready when plurals cut over to it. Until that cutover, the AI plural translations sit in the catalog unused.
 
-## What's deferred: regular strings
+## What's staged, not shipped: regular strings
 
-Regular (non-plural) strings are **not** machine-translated, by design. The app still ships the legacy `WordPress/Resources/<locale>.lproj/Localizable.strings` for them — `Localizable.xcstrings` (`generate_strings_catalog`) is the designated future backing store, but today it's only generated transiently in CI as a coverage check — the file is gitignored, not committed, and nothing ships from it. A machine translation written into the legacy `.strings` would be **live immediately**, and we don't want machine-translated regular strings shipping before the catalog cutover.
+Regular (non-plural) strings still ship the legacy way — from `WordPress/Resources/<locale>.lproj/Localizable.strings`, with no machine translation. A machine translation written there would be **live immediately**, and we don't want machine-translated regular strings shipping before the catalog cutover. `Localizable.xcstrings` (`generate_strings_catalog`) is the designated future backing store; it's gitignored and not a build member, so nothing ships from it.
 
-So regular-string MT waits for the same shape as plurals: once `Localizable.xcstrings` becomes the runtime store, a regular-string **catalog reverse-fold** folds the human translations in and AI-fills the `needs_review` gaps, staged in the catalog (not shipped) until cutover — exactly as the plural fold does today.
+The tooling to **stage** regular-string translations into that catalog now exists, the same shape as the plural fold. `CatalogStrings.fold_translations!` fills each `(key, locale)` cell of `Localizable.xcstrings` as `human ?? AI ?? English` — human ⇒ `translated`, machine / English ⇒ `needs_review` — and is reuse-aware in the same way: a kept machine cell isn't re-translated, and a human translation supersedes it on the next fold. It runs as two manual lanes: `generate_strings_catalog` (extract the English source) and `localize_catalog` (download GlotPress into a throwaway temp dir, fold the humans in, AI-fill the rest, commit the catalog). The download is a fresh temp dir every run, so no stale/partial translation state is ever carried between runs.
 
-When that's built, two facts established here will carry over:
+Two things keep it from shipping anything today, and both set it apart from the plural fold:
+
+- **Manual, not in the release path.** These lanes aren't wired into `download_localized_strings` or any beta/release step — a run extracts strings, calls the API (cost), and commits a large catalog, so it's run on demand. Only the unit tests run in CI.
+- **Staged, not shipped.** `Localizable.xcstrings` still isn't the runtime store, so the folded translations sit in the catalog unused until the cutover — exactly like the plural catalog.
+
+**Keys are immutable.** `generate_strings_catalog` hard-fails if an explicit-key string's English is reworded in place — `xcstringstool sync` would silently keep that key's now-stale translations, and the fold can't tell a translation of the old English from a current one. Rewording requires a **new key**. Key-as-source strings are exempt: rewording one changes the key, which sync handles as new/stale. (Enforcement today fires where the catalog persists; extending it to the transient-catalog CI path is a follow-up — a lint on the committed English `.strings`.)
+
+Two facts the fold relies on, both established by the reverse download:
 
 - **"Undefined by GlotPress" = absent**, not empty. The export omits untranslated strings (`status: current`; verified no empty-valued entries), so absence is the untranslated signal.
 - **Humans always supersede MT**, and machine output never returns to GlotPress — so there's no translation-memory pollution and no manual reconciliation, as long as MT lives in a state-bearing store (the catalog's `needs_review`).
@@ -77,5 +84,5 @@ When that's built, two facts established here will carry over:
 | Brand do-not-translate + per-locale terms | `fastlane/lanes/translation_glossary.rb` |
 | Anthropic SDK glue + Message Batches | `fastlane/lanes/anthropic_batch.rb` |
 | Plural fold (`Localizable.strings` ⇄ `Plurals.xcstrings`) + AI wiring | `fastlane/lanes/plural_strings_helper.rb`, `fastlane/lanes/localization_plurals.rb` |
-| Catalog generation (future regular-string backing store) | `fastlane/lanes/localization_catalog.rb` |
+| Catalog generation + regular-string fold (staged, manual) | `fastlane/lanes/localization_catalog.rb`, `fastlane/lanes/catalog_strings_helper.rb` |
 | Download/upload orchestration | `fastlane/lanes/localization.rb` |
@@ -160,16 +160,26 @@ def translate_plural(english_forms:, categories:, locale:, note: nil, anchors: {
   # strings already sorted by key so each batch naturally groups one feature (reader.*, editor.*) — better
   # terminology consistency within a batch.
   #
+  # A batch spanning many keys is many sequential requests. If one FAILS mid-run (network / rate limit / SDK
+  # error), the batches that already succeeded are KEPT and the run stops — the remaining strings fall back to
+  # English and are retried on the next run, rather than the whole locale's completed (and billed) work being
+  # discarded. Stopping (vs. continuing) avoids hammering the API when the failure is systemic.
+  #
   # @param strings [Array<Hash>] each { key:, source:, comment: } (string or symbol keys both accepted).
   # @param locale [String] target lproj code.
   # @param batch_size [Integer] strings per request.
   def translate_all(strings, locale:, batch_size: DEFAULT_BATCH_SIZE)
     items = batchable_items(strings)
     return {} if items.empty?
 
-    items.each_slice(batch_size).with_object({}) do |chunk, out|
-      out.merge!(translate_batch(chunk, locale))
+    translated = {}
+    items.each_slice(batch_size).with_index do |chunk, index|
+      translated.merge!(translate_batch(chunk, locale))
+    rescue StandardError => e
+      warn_batch_failure(locale: locale, index: index, kept: translated.size, error: e)
+      break
     end
+    translated
   end
 
   # Builds Message Batch jobs for many strings across many locales (the async / cheaper bulk path). Returns
@@ -297,6 +307,14 @@ def to_string_keys(hash)
     (hash || {}).each_with_object({}) { |(key, value), acc| acc[key.to_s] = value }
   end
 
+  # A batch request failed mid-run. Keep the batches that already succeeded and STOP rather than hammer the rest:
+  # the untranslated strings fall back to English and are retried next run (the fold's reuse-awareness means the
+  # kept ones aren't re-billed). Surfaced on stderr — this class is deliberately fastlane-free, so no UI here.
+  def warn_batch_failure(locale:, index:, kept:, error:)
+    warn "AITranslator: batch #{index + 1} for '#{locale}' failed (#{error.message}); " \
+         "kept #{kept} translation(s), the rest fall back to English (retry next run)."
+  end
+
   # One batched request: number the chunk, ask for a JSON {number => translation}, keep the validated ones.
   def translate_batch(chunk, locale)
     numbered = number_chunk(chunk)
 
@@ -4,6 +4,7 @@
 # Uses a canned-reply lambda for `complete:`, so it exercises all of the prompt-building / validation logic
 # without the `anthropic` gem or the network.
 require 'minitest/autorun'
+require 'stringio'
 require_relative 'ai_translator'
 
 # Exercises prompt-building and the validator gate via a canned-reply `complete:` lambda (no gem / network).
@@ -17,6 +18,16 @@ def translator(reply:, prompts: nil)
     AITranslator.new(complete: complete)
   end
 
+  # Runs the block with $stderr captured, returning what it wrote (the class surfaces batch failures via warn).
+  def capture_stderr
+    original = $stderr
+    $stderr = StringIO.new
+    yield
+    $stderr.string
+  ensure
+    $stderr = original
+  end
+
   def test_returns_cleaned_translation
     t = translator(reply: %("Réglages"\n)) # wrapped in quotes + trailing newline
     assert_equal 'Réglages', t.translate(source: 'Settings', locale: 'fr')
@@ -182,6 +193,37 @@ def test_translate_all_bad_json_batch_falls_back
     assert_empty out
   end
 
+  def test_translate_all_keeps_completed_batches_when_a_later_batch_fails
+    calls = 0
+    complete = lambda do |**|
+      calls += 1
+      raise 'rate limited' if calls == 2 # the second batch fails mid-run
+
+      '{"1":"x","2":"y"}'
+    end
+    out = nil
+    warnings = capture_stderr do
+      out = AITranslator.new(complete: complete).translate_all(
+        [{ key: 'a', source: 'One' }, { key: 'b', source: 'Two' },
+         { key: 'c', source: 'Three' }, { key: 'd', source: 'Four' },
+         { key: 'e', source: 'Five' }, { key: 'f', source: 'Six' }],
+        locale: 'fr', batch_size: 2
+      )
+    end
+
+    assert_equal 2, calls, 'must stop after the failing batch, not hammer the remaining ones'
+    assert_equal({ 'a' => 'x', 'b' => 'y' }, out, 'the first completed batch is kept, not discarded')
+    assert_match(/batch 2 for 'fr' failed \(rate limited\)/, warnings)
+  end
+
+  def test_translate_all_returns_empty_without_raising_when_the_first_batch_fails
+    complete = ->(**) { raise 'network down' }
+    out = nil
+    capture_stderr { out = AITranslator.new(complete: complete).translate_all([{ key: 'a', source: 'One' }], locale: 'fr') }
+
+    assert_empty out, 'a total failure degrades to English (empty), and must not propagate the exception'
+  end
+
   def test_translate_all_empty_input_makes_no_call
     called = false
     complete = lambda do |**|
 
@@ -24,75 +24,38 @@ def coverage_gap(reference, catalog_keys)
     reference.reject { |key| catalog_canonical.include?(canonical(key)) }
   end
 
-  # Collapse format specifiers to a single token so source-form (%li) and normalized (%1$li) compare equal.
+  # Strip the positional index from each format specifier so a source-form specifier (%li) and its normalized form
+  # (%1$li) compare equal, while specifiers of a DIFFERENT argument type stay distinct. The positional `N$` prefix
+  # is the only thing that differs between the two extraction paths (genstrings vs xcstringstool) for the same
+  # source literal; collapsing every specifier to one token — as this did before — conflated `%d days` with
+  # `%@ days` and masked a genuinely-dropped key behind a same-prose sibling of a different type.
   def canonical(key)
-    key.gsub(FORMAT_SPECIFIER, "\u0001")
+    key.gsub(FORMAT_SPECIFIER) { |specifier| specifier.sub(/\A%\d+\$/, '%') }
   end
 
-  # --- needs_review reconciliation ----------------------------------------------------------------------
+  # --- immutable-key enforcement -------------------------------------------------------------------------
 
-
-  # `xcstringstool sync` does NOT reconcile an existing key whose English source VALUE changed: it leaves
-  # both the stored English value and the affected translations untouched (verified — source "Settings" →
-  # "Preferences" left en="Settings" and fr="translated"). The in-Xcode build does this reconciliation; the
-  # standalone CLI does not. This closes that gap: where the freshly-extracted English differs from what the
-  # catalog stores, it updates the English value and flips that key's translations from `translated` to
-  # `needs_review` (so the AI/human pipeline re-checks them).
+  # `xcstringstool sync` does NOT touch an existing key whose English source VALUE changed in place: it leaves
+  # both the stored English and the affected translations as-is (verified — source "Settings" → "Preferences"
+  # left en="Settings" and fr="translated"). The in-Xcode build reconciles this; the standalone CLI doesn't.
+  #
+  # Localization keys are IMMUTABLE, so an in-place reword is a rule violation, not something to reconcile:
+  # rewording must mint a NEW key, otherwise the old key silently keeps translations of the OLD English — a
+  # stale translation the fold can't distinguish from a current one. This reports the offending keys so the
+  # lane can hard-fail (rename the key, or revert the English change).
   #
-  # Out of scope here (handled elsewhere): English-as-key strings — editing their text changes the KEY, which
-  # sync already handles as new/stale; and plural entries, whose English is itself a plural variation, so
-  # `reconcile_entry!` bails (no flat English `stringUnit`) — those live in the separate plurals catalog.
-  # Translation-side device/width variations of a regular string ARE reconciled (see `string_units`).
+  # Naturally scoped to explicit-key strings: a key-as-source string has no stored `en` value (its key IS its
+  # English), and rewording one changes the KEY — sync handles that as new/stale, not a value change — so it
+  # never appears here. Plurals don't either (their English is a plural variation, not a flat `stringUnit`).
   #
-  # @param catalog [Hash] parsed `.xcstrings`, mutated in place
+  # @param catalog [Hash] parsed `.xcstrings`
   # @param current_en [Hash{String=>String}] key => freshly-extracted English value
-  # @return [Array<String>] keys that were reconciled (English updated + translations re-flagged)
-  def reconcile_source_changes!(catalog, current_en)
+  # @return [Array<String>] keys whose stored English no longer matches the source (empty ⇒ nothing reworded)
+  def reworded_keys(catalog, current_en)
     (catalog['strings'] || {}).filter_map do |key, entry|
-      key if reconcile_entry!(entry, current_en[key])
-    end
-  end
-
-  # Reconcile one entry against its freshly-extracted English value. Returns the entry (truthy) if it
-  # changed, nil otherwise — matching the Ruby bang-method convention (cf. String#gsub!).
-  def reconcile_entry!(entry, new_value)
-    return if new_value.nil?
-
-    english = entry.dig('localizations', 'en', 'stringUnit')
-    return if english.nil? || english['value'] == new_value
-
-    english['value'] = new_value
-    flag_translations_for_review!(entry['localizations'])
-    entry
-  end
-
-  def flag_translations_for_review!(localizations)
-    localizations.each do |locale, body|
-      next if locale == 'en' || body.nil?
-
-      string_units(body).each do |unit|
-        unit['state'] = 'needs_review' if unit['state'] == 'translated'
-      end
-    end
-  end
-
-  # All stringUnits in a localization body, whether stored flat (`stringUnit`) or nested under one or more
-  # `variations` (a regular string's translation can be varied by device/width, and variations can nest).
-  # Returns the unit hashes themselves so a caller can flip their `state` in place — a single top-level
-  # `body['stringUnit']` lookup would miss the varied leaves entirely.
-  def string_units(node)
-    return [] unless node.is_a?(Hash)
-
-    units = []
-    units << node['stringUnit'] if node['stringUnit'].is_a?(Hash)
-    variations = node['variations']
-    if variations.is_a?(Hash)
-      variations.each_value do |cases|
-        next unless cases.is_a?(Hash)
-
-        cases.each_value { |child| units.concat(string_units(child)) }
-      end
+      stored = entry.dig('localizations', 'en', 'stringUnit', 'value')
+      fresh = current_en[key]
+      key if stored && fresh && stored != fresh
     end
-    units
   end
 end
@@ -0,0 +1,56 @@
+# frozen_string_literal: true
+
+# Pure-Ruby unit suite for CatalogHelper.reworded_keys — the immutable-key guard that detects an explicit-key
+# string whose English source was reworded in place (which `xcstringstool sync` silently ignores, leaving stale
+# translations). Run directly: `ruby fastlane/lanes/catalog_helper_test.rb`. No bundle / network.
+require 'minitest/autorun'
+require_relative 'catalog_helper'
+
+# Exercises the reword detector (an explicit-key string whose stored English differs from a fresh extraction is
+# flagged; key-as-source, variations/plural, unchanged, and removed keys are not) and the coverage compare.
+class CatalogHelperTest < Minitest::Test
+  def en(value)
+    { 'localizations' => { 'en' => { 'stringUnit' => { 'state' => 'translated', 'value' => value } } } }
+  end
+
+  def reworded(strings, current_en)
+    CatalogHelper.reworded_keys({ 'sourceLanguage' => 'en', 'version' => '1.0', 'strings' => strings }, current_en)
+  end
+
+  def test_flags_an_explicit_key_whose_english_changed_in_place
+    assert_equal ['common.save'], reworded({ 'common.save' => en('Save') }, { 'common.save' => 'Save all' })
+  end
+
+  def test_does_not_flag_an_unchanged_key
+    assert_empty reworded({ 'common.save' => en('Save') }, { 'common.save' => 'Save' })
+  end
+
+  def test_does_not_flag_a_key_absent_from_the_fresh_extraction
+    # The key was removed/renamed in code — sync handles that as stale; it is not an in-place reword.
+    assert_empty reworded({ 'common.save' => en('Save') }, {})
+  end
+
+  def test_does_not_flag_a_key_as_source_entry
+    # No stored `en` value (the key IS the English); rewording it changes the key, so it can't appear here.
+    assert_empty reworded({ '%1$@ on %2$@' => {} }, { '%1$@ on %2$@' => '%1$@ at %2$@' })
+  end
+
+  def test_does_not_flag_a_variations_shaped_english
+    # English stored under `variations` has no flat `en` stringUnit value, so it can never read as a reword.
+    varied = { 'localizations' => { 'en' => { 'variations' => { 'device' => { 'iphone' => { 'stringUnit' => { 'state' => 'translated', 'value' => 'Tap' } } } } } } }
+    assert_empty reworded({ 'app.banner' => varied }, { 'app.banner' => 'Click' })
+  end
+
+  def test_reports_every_reworded_key
+    strings = { 'a' => en('One'), 'b' => en('Two'), 'c' => en('Three') }
+    assert_equal %w[b c], reworded(strings, { 'a' => 'One', 'b' => 'TWO', 'c' => 'THREE' }).sort
+  end
+
+  # coverage_gap must not conflate keys that differ only in argument TYPE. `canonical` used to replace every format
+  # specifier with one sentinel regardless of type/index, so if the build-free extraction loses "%d days" (e.g. a
+  # same-basename .stringsdata overwrite — the exact regression this gate exists to catch), the surviving "%@ days"
+  # masked it. "%d days" (int) and "%@ days" (object) are real, distinct en.lproj keys; dropping either is a hole.
+  def test_coverage_gap_does_not_conflate_distinct_type_sibling_keys
+    assert_equal ['%d days'], CatalogHelper.coverage_gap(['%d days', '%@ days'], ['%@ days'])
+  end
+end