Skip to content

Commit 777cc6e

Browse files
committed
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.
1 parent cd9b85f commit 777cc6e

2 files changed

Lines changed: 33 additions & 0 deletions

File tree

fastlane/lanes/catalog_strings_helper_test.rb

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,21 @@ def test_human_translation_is_used_and_marked_translated
7373
assert_equal({ 'state' => 'translated', 'value' => 'Enregistrer' }, cell(cat, 'a', 'fr'))
7474
end
7575

76+
# A human translation that reorders a non-positional source (%@ … %@) via positional specifiers (%2$@ … %1$@)
77+
# is valid at runtime — String(format:) honors positional specifiers regardless of the source's shape — and is
78+
# the standard iOS reordering pattern for RTL / word-order-differing locales. It must ship as `translated`, NOT
79+
# be rejected by the placeholder gate and downgraded to machine/English. This is the user-facing half of the
80+
# TranslationValidator regression (see translation_validator_test.rb): 23 key-as-source strings across 34
81+
# locales (513 currently-shipping cells) do exactly this, e.g. ar "%@ of %@ used on your site" folds to
82+
# "%1$@ من %2$@ على موقعك". With no AI tier here, the rejected human currently degrades to English (needs_review).
83+
def test_reordered_human_translation_of_a_non_positional_source_is_kept_as_translated
84+
cat = catalog('a' => entry('%@ of %@ used'))
85+
capture_stderr { fold(cat, translations: { 'fr' => { 'a' => '%2$@ sur %1$@ utilisés' } }) }
86+
87+
assert_equal({ 'state' => 'translated', 'value' => '%2$@ sur %1$@ utilisés' }, cell(cat, 'a', 'fr'),
88+
'a reordered human translation must ship as translated, not be downgraded')
89+
end
90+
7691
# A whitespace-only GlotPress value is not a real translation: it must not ship as `translated` — the key
7792
# should still reach the model and land as `needs_review`, same as if GlotPress had no value at all.
7893
def test_whitespace_only_human_value_is_treated_as_untranslated

fastlane/lanes/translation_validator_test.rb

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,24 @@ def test_positional_type_change_is_rejected
2323
refute V.placeholders_match?('%1$@ posts', '%1$d posts')
2424
end
2525

26+
# A NON-positional source (%@ … %@) whose translation reorders via positional specifiers (%2$@ … %1$@) is the
27+
# standard, Apple-documented iOS way to reorder arguments for target grammar: String(format:) honors positional
28+
# specifiers regardless of the source's shape, and each specifier still reads an argument of the same type. So
29+
# this must be ACCEPTED, exactly like the already-positional reorder above. The gate currently rejects it,
30+
# because it compares the source's positional/sequential views independently and a bare-%@ source has an empty
31+
# positional map. This is not academic: real GlotPress data has 23 key-as-source strings across 34 locales
32+
# (513 shipping cells) that positionalize a bare-%@ English source this way — e.g. the Arabic
33+
# "%@ of %@ used on your site" => "%1$@ من %2$@ على موقعك". The human-translation gate this catalog work adds
34+
# (CatalogStrings.trusted_human, PluralStrings.human_forms_for) rejects every one of them and downgrades a
35+
# valid, currently-shipping human translation to machine/English.
36+
#
37+
# (Genuine breakage stays rejected by the existing tests: a sequential flip `%@: %d` => `%d : %@` and a type
38+
# change are still caught, so a correct fix cannot simply ignore the positional/sequential distinction.)
39+
def test_positionalizing_a_non_positional_source_to_reorder_is_allowed
40+
assert V.placeholders_match?('%@ - %@', '%2$@ - %1$@') # two objects, reordered
41+
assert V.placeholders_match?('%@. %d posts.', '%1$@. %2$d posts.') # object + int, positionalized in place
42+
end
43+
2644
def test_sequential_order_must_be_preserved
2745
refute V.placeholders_match?('%@: %d', '%d : %@') # flipped non-positional args
2846
assert V.placeholders_match?('%@: %d', 'Total %@: %d') # same order, prose changed

0 commit comments

Comments
 (0)