From 8cf5c72713bd3beefd316eb636771c0ed9768df3 Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Tue, 25 Aug 2026 18:39:05 +1200 Subject: [PATCH] i18n audit: count plural entries as translated in the coverage summary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The human-readable coverage summary read localizations[lang].stringUnit.state directly. A pluralised entry keeps its units under variations.plural..stringUnit, so that flat lookup returned None and scored a fully-translated plural as a gap. It reported de/es/fr/pt-PT missing=4 and pl missing=5 on the Strand catalog when every one of those entries was translated in every plural form - "%lld days" is "%lld Tage", "%lld días", "%lld jours", "%lld dias", "%lld dnia". Worse than a wrong number: it sent a reader off to re-translate strings that were already done, and it made Polish look like the worst-covered shipped language precisely BECAUSE it correctly carries one/few/many/other where the others need only a flat unit. `_string_units` and `_is_translated` were written for exactly this - the former's docstring says "Reading only the FIRST shape makes every pluralised entry look untranslated to this gate". The GATE was fixed then (#844); this summary was the one caller still doing the naive read. It now goes through `_is_translated`, which also keeps a partially-translated plural counting as a gap. Separately, and NOT caused by the above: the ratchet in i18n_extra_locale_baseline.txt was stale by two on four targets (it 181->179, ru 158->156, zh-Hans 131->129, zh-Hant 181->179). The gate already computed the lower numbers on unmodified main and asked for the file to be lowered on every run, because real translations had landed since it was last written. Lowered to match, down only, as the file requires. The genuine drift is unchanged in substance: it/ru/zh remain the untranslated locales. Five tests pin the summary itself rather than only the shared helper, since a revert would restore the flat read and leave the helper's own tests green. --- Tools/i18n_audit.py | 12 ++++- Tools/i18n_extra_locale_baseline.txt | 8 ++-- Tools/test_i18n_audit.py | 66 ++++++++++++++++++++++++++++ 3 files changed, 80 insertions(+), 6 deletions(-) diff --git a/Tools/i18n_audit.py b/Tools/i18n_audit.py index 88e6d3ac6c..85a2fe94e9 100644 --- a/Tools/i18n_audit.py +++ b/Tools/i18n_audit.py @@ -1234,8 +1234,16 @@ def catalog_summary() -> None: for v in strings.values(): if v.get("shouldTranslate") is False: continue - state = (v.get("localizations", {}).get(lang) or {}).get("stringUnit", {}).get("state") - if state != "translated": + # Via `_is_translated`, NOT a bare `localizations[lang].stringUnit.state` read: a + # pluralised entry keeps its units under `variations.plural..stringUnit`, so the + # flat lookup returns None and scores a fully-translated plural as a gap. That is the exact + # trap `_string_units` was written for, and this summary was the one caller still falling + # into it — reporting de/es/fr/pt-PT missing=4 and pl missing=5 on the Strand catalog when + # every one of those entries was translated in every form. Worse than a wrong number: it + # sent a reader to re-translate strings that were already done, and it made Polish look + # like the worst-covered language precisely BECAUSE it correctly carries one/few/many/other + # where the others need only a flat unit. + if not _is_translated(v, lang): missing += 1 line += f" {lang} missing={missing}" print(" ", line) diff --git a/Tools/i18n_extra_locale_baseline.txt b/Tools/i18n_extra_locale_baseline.txt index c987dbea30..d854b49885 100644 --- a/Tools/i18n_extra_locale_baseline.txt +++ b/Tools/i18n_extra_locale_baseline.txt @@ -14,8 +14,8 @@ NOOPWatch/Localizable.xcstrings:zh-Hant 0 NOOPWatchComplications/Localizable.xcstrings:it 25 NOOPWatchComplications/Localizable.xcstrings:zh-Hans 25 NOOPWatchComplications/Localizable.xcstrings:zh-Hant 25 -Strand/Resources/Localizable.xcstrings:it 181 -Strand/Resources/Localizable.xcstrings:ru 158 -Strand/Resources/Localizable.xcstrings:zh-Hans 131 -Strand/Resources/Localizable.xcstrings:zh-Hant 181 +Strand/Resources/Localizable.xcstrings:it 179 +Strand/Resources/Localizable.xcstrings:ru 156 +Strand/Resources/Localizable.xcstrings:zh-Hans 129 +Strand/Resources/Localizable.xcstrings:zh-Hant 179 values-zh/strings.xml 49 diff --git a/Tools/test_i18n_audit.py b/Tools/test_i18n_audit.py index 42cb919b74..2cbf8ef24a 100644 --- a/Tools/test_i18n_audit.py +++ b/Tools/test_i18n_audit.py @@ -434,3 +434,69 @@ def test_two_word_brand_is_flagged(self): if __name__ == "__main__": unittest.main() + + +class CatalogSummaryPluralCoverage(unittest.TestCase): + """The human-readable coverage summary must count plural entries the same way the gate does. + + `catalog_summary` used to read `localizations[lang].stringUnit.state` directly. A pluralised entry + keeps its units under `variations.plural..stringUnit`, so that flat lookup returned None + and scored a fully-translated plural as a gap — it reported de/es/fr/pt-PT missing=4 and pl missing=5 + on the Strand catalog when every one of those was translated in every form, and it made Polish look + worst-covered precisely because it correctly carries one/few/many/other. + """ + + def _summary(self, catalog: dict) -> str: + import contextlib + import io + saved_catalogs, saved_load = ia.CATALOGS, ia.load_catalog + try: + ia.CATALOGS = [((), ia.ROOT / "fixture" / "Localizable.xcstrings")] + ia.load_catalog = lambda _path: catalog + buf = io.StringIO() + with contextlib.redirect_stdout(buf): + ia.catalog_summary() + return buf.getvalue() + finally: + ia.CATALOGS, ia.load_catalog = saved_catalogs, saved_load + + @staticmethod + def _plural(states: dict[str, str]) -> dict: + return {"variations": {"plural": { + cat: {"stringUnit": {"state": st, "value": f"{cat} form"}} for cat, st in states.items() + }}} + + def test_fully_translated_plural_is_not_a_gap(self): + cat = {"strings": {"%lld days": {"localizations": { + lang: self._plural({"one": "translated", "other": "translated"}) + for lang in ("de", "es", "fr", "pt-PT") + }}}} + out = self._summary(cat) + for lang in ("de", "es", "fr", "pt-PT"): + self.assertIn(f"{lang} missing=0", out) + + def test_polish_extra_plural_categories_are_not_penalised(self): + """one/few/many/other is Polish being handled correctly, not five gaps.""" + cat = {"strings": {"%lld days": {"localizations": { + "pl": self._plural({c: "translated" for c in ("one", "few", "many", "other")}), + }}}} + self.assertIn("pl missing=0", self._summary(cat)) + + def test_partially_translated_plural_is_still_a_gap(self): + """The fix must not over-correct: one untranslated category still counts.""" + cat = {"strings": {"%lld days": {"localizations": { + "de": self._plural({"one": "translated", "other": "new"}), + }}}} + self.assertIn("de missing=1", self._summary(cat)) + + def test_flat_string_unit_still_counted(self): + """Non-plural entries keep working exactly as before.""" + cat = {"strings": { + "Hello": {"localizations": {"de": {"stringUnit": {"state": "translated", "value": "Hallo"}}}}, + "Bye": {"localizations": {"de": {"stringUnit": {"state": "new", "value": ""}}}}, + }} + self.assertIn("de missing=1", self._summary(cat)) + + def test_should_translate_false_is_skipped(self): + cat = {"strings": {"NOOP": {"shouldTranslate": False, "localizations": {}}}} + self.assertIn("de missing=0", self._summary(cat))