fix(i18n): fail the coverage gate on a floors file it can't read

load_floors() returned {} for a file that existed but didn't parse, so a
merge-conflicted translation_floors.json switched the gate off, and
--update-floors then rewrote every floor from that day's coverage. A
refuting review reproduced both: zh-rCN went from 76.5 to 70.0 with the gate
green, and a floor stored as a string was dropped the same way. A missing
file still means no floors yet. A present but unreadable one, a missing
"floors" object, or a non-numeric floor (booleans included) raises
FloorsFileError, which main() counts as an error, and write_floors() leaves
the file alone.

--update-floors skipped the floor check and printed success at any
coverage. It now checks against the floors it found first, so a locale below
its floor fails either way. The gate's error text told people to lower a
floor while the file said never to. Both now say a lower floor is a hand
edit explained in the commit.

Tests: test_floors_round_trip_and_a_broken_file_is_not_fatal pinned the
fail-open behaviour. It's replaced by ..._fails_closed, which asserts the
opposite for five broken shapes. New tests drive main() over a throwaway
res tree: below-floor --update-floors fails and keeps the floor, a passed
floor is raised (positive control), and an unreadable file fails the check
and isn't rewritten. Five mutants of the fix are each killed by name.

Author: SysAdminDoc

CHANGELOG.md

@@ -255,6 +255,10 @@ The rest of this section takes effect when the report Worker is next deployed.
   lower coverage instead, and the Chinese floor slid from 76.1% to 75.6% that
   way. The eight strings behind the slide are translated now, and the floor is
   back up at 76.5%.
+- A floors file that can't be read, such as one left mid-merge, now fails the
+  check. It used to load as empty, which switched the gate off and let
+  `--update-floors` write every floor again from scratch. `--update-floors`
+  also fails when a locale is below its floor, where it used to report success.
 
 ## v1.7.38 (2026-08-29)
 

docs/TRANSLATING.md

@@ -81,9 +81,12 @@ If you improve a locale, raise its floor in the same change:
 python scripts/check_translations.py --update-floors
 ```
 
-The floor only ever goes up. Lowering one is a deliberate decision that
-belongs in the commit message. A new locale starts with no floor and only
-warns until someone records one.
+`--update-floors` only ever raises a floor, and it still fails when a locale
+is below the floor it had. Lowering a floor means editing
+`scripts/translation_floors.json` by hand, on purpose, with the reason in the
+commit message. A floors file that can't be read fails the check rather than
+switching it off. A new locale starts with no floor and only warns until
+someone records one.
 
 ## Style notes
 

scripts/check_translations.py

@@ -290,19 +290,34 @@ def check_locale(locale_dir: Path, base_strings, base_untranslatable, base_plura
     return errors, warnings, translated, len(translatable_base)
 
 
+class FloorsFileError(ValueError):
+    """The floors file exists but can't be trusted."""
+
+
 def load_floors() -> dict[str, float]:
-    """Return the recorded per-locale coverage floors."""
+    """Return the recorded per-locale coverage floors.
+
+    A missing file means no floors have been recorded yet. A file that exists
+    but can't be parsed, or holds a floor that isn't a number, raises
+    FloorsFileError. Reading it as empty would switch the gate off (a
+    merge-conflicted file did exactly that) and let --update-floors rewrite
+    every floor from whatever coverage happens to be today.
+    """
+    if not FLOORS_FILE.exists():
+        return {}
     try:
         with FLOORS_FILE.open(encoding="utf-8") as handle:
             recorded = json.load(handle)
-    except (OSError, ValueError):
-        return {}
-    if not isinstance(recorded, dict):
-        return {}
+    except (OSError, ValueError) as error:
+        raise FloorsFileError(f"{FLOORS_FILE.name} can't be read: {error}") from error
+    recorded_floors = recorded.get("floors") if isinstance(recorded, dict) else None
+    if not isinstance(recorded_floors, dict):
+        raise FloorsFileError(f'{FLOORS_FILE.name} has no "floors" object')
     floors = {}
-    for locale, value in recorded.get("floors", {}).items():
-        if isinstance(value, (int, float)):
-            floors[str(locale)] = float(value)
+    for locale, value in recorded_floors.items():
+        if isinstance(value, bool) or not isinstance(value, (int, float)):
+            raise FloorsFileError(f"{FLOORS_FILE.name}: the floor for {locale} is {value!r}, not a number")
+        floors[str(locale)] = float(value)
     return floors
 
 
@@ -331,8 +346,9 @@ def floor_report(
             report.append(
                 (
                     f"  ERROR:   {locale} coverage fell to {percent:.1f}%, below its "
-                    f"{floor:.1f}% floor. Translate the new strings, or lower the floor "
-                    f"deliberately in {FLOORS_FILE.name} and say why.",
+                    f"{floor:.1f}% floor. Translate the new strings. Lowering a floor is "
+                    f"a hand edit to {FLOORS_FILE.name}, made on purpose and explained "
+                    f"in the commit message.",
                     True,
                 )
             )
@@ -345,16 +361,17 @@ def write_floors(coverage: dict[str, float]) -> None:
     A floor only goes up here. Running --update-floors after adding English
     strings nobody has translated yet used to record the lower coverage, which
     quietly loosened the gate it exists to enforce. Floors for locales this
-    run didn't measure are kept.
+    run didn't measure are kept. An unreadable file raises FloorsFileError
+    and is left alone.
     """
     floors = load_floors()
     for locale, percent in coverage.items():
         floors[locale] = max(floors.get(locale, 0.0), round(percent, 1))
     payload = {
         "description": (
             "Minimum translated-string coverage per locale, in percent. check_translations.py "
-            "fails when a shipped locale drops below its floor. Raise a floor with --update-floors; "
-            "never lower one by hand."
+            "fails when a shipped locale drops below its floor. --update-floors only raises a floor. "
+            "Lowering one is a hand edit, made on purpose and explained in the commit message."
         ),
         "floors": dict(sorted(floors.items())),
     }
@@ -417,15 +434,22 @@ def main() -> int:
 
     # A floor is only meaningful for a locale that ships from this repo;
     # --dir points at an arbitrary tree (a fork's, pre-merge) that has none.
+    # --update-floors still checks against the floors it found: it can't lower
+    # one, so a locale below its floor stays an error either way.
     if not args.dir:
-        if args.update_floors:
-            write_floors(coverage)
-            print(f"\nRecorded coverage floors in {FLOORS_FILE.name}.")
+        try:
+            floors = load_floors()
+        except FloorsFileError as error:
+            print(f"  ERROR:   {error}. Fix or restore it before checking coverage.")
+            total_errors += 1
         else:
-            for message, is_error in floor_report(coverage, load_floors()):
+            for message, is_error in floor_report(coverage, floors):
                 print(message)
                 if is_error:
                     total_errors += 1
+            if args.update_floors:
+                write_floors(coverage)
+                print(f"\nRecorded coverage floors in {FLOORS_FILE.name}.")
 
     # Only meaningful for locales that actually live in the repo.
     for error in check_locales_config([d for d in locale_dirs if d.parent == RES_DIR]):

scripts/test_check_translations.py

@@ -1,10 +1,15 @@
 #!/usr/bin/env python3
 """Security regression tests for the community translation validator."""
 
+import contextlib
+import io
+import json
+import sys
 import tempfile
 import unittest
 import xml.etree.ElementTree as ET
 from pathlib import Path
+from unittest import mock
 
 import check_translations
 
@@ -86,7 +91,10 @@ def test_locale_without_a_floor_warns_but_does_not_fail(self):
         self.assertFalse(is_error)
         self.assertIn("--update-floors", message)
 
-    def test_floors_round_trip_and_a_broken_file_is_not_fatal(self):
+    def test_floors_round_trip_and_a_broken_file_fails_closed(self):
+        # A missing file is "no floors yet". A file that is there but unreadable
+        # used to load as empty too, which switched the gate off for a
+        # merge-conflicted file. It is an error now.
         with tempfile.TemporaryDirectory() as directory:
             path = Path(directory) / "translation_floors.json"
             original = check_translations.FLOORS_FILE
@@ -98,10 +106,36 @@ def test_floors_round_trip_and_a_broken_file_is_not_fatal(self):
                     {"values-de": 40.0, "values-zh-rCN": 76.1},
                     check_translations.load_floors(),
                 )
-                path.write_text("not json", encoding="utf-8")
-                self.assertEqual({}, check_translations.load_floors())
-                path.write_text('{"floors": {"values-de": "sixty"}}', encoding="utf-8")
-                self.assertEqual({}, check_translations.load_floors())
+                for broken in (
+                    "not json",
+                    "[]",
+                    '{"description": "no floors object"}',
+                    '{"floors": {"values-de": "60.0"}}',
+                    '{"floors": {"values-de": true}}',
+                ):
+                    path.write_text(broken, encoding="utf-8")
+                    with self.assertRaises(check_translations.FloorsFileError, msg=broken):
+                        check_translations.load_floors()
+            finally:
+                check_translations.FLOORS_FILE = original
+
+    def test_update_floors_leaves_an_unreadable_file_alone(self):
+        conflicted = (
+            "{\n<<<<<<< HEAD\n"
+            '  "floors": {"values-zh-rCN": 76.5}\n'
+            "=======\n"
+            '  "floors": {"values-zh-rCN": 76.1}\n'
+            ">>>>>>> branch\n}\n"
+        )
+        with tempfile.TemporaryDirectory() as directory:
+            path = Path(directory) / "translation_floors.json"
+            path.write_text(conflicted, encoding="utf-8")
+            original = check_translations.FLOORS_FILE
+            check_translations.FLOORS_FILE = path
+            try:
+                with self.assertRaises(check_translations.FloorsFileError):
+                    check_translations.write_floors({"values-zh-rCN": 70.0})
+                self.assertEqual(conflicted, path.read_text(encoding="utf-8"))
             finally:
                 check_translations.FLOORS_FILE = original
 
@@ -127,5 +161,67 @@ def test_recorded_floor_matches_the_shipped_locale(self):
         self.assertIn("values-zh-rCN", floors, floors)
 
 
+class TranslationCheckerMainTest(unittest.TestCase):
+    """Drives main() over a throwaway resource tree with one German locale."""
+
+    NAMES = ("a", "b", "c", "d")
+
+    def make_tree(self, directory: str, translated: int) -> Path:
+        res = Path(directory) / "res"
+        (res / "values").mkdir(parents=True)
+        (res / "values-de").mkdir()
+        english = "".join(f'<string name="{name}">{name}</string>' for name in self.NAMES)
+        german = "".join(f'<string name="{name}">{name}-de</string>' for name in self.NAMES[:translated])
+        (res / "values" / "strings.xml").write_text(f"<resources>{english}</resources>", encoding="utf-8")
+        (res / "values-de" / "strings.xml").write_text(f"<resources>{german}</resources>", encoding="utf-8")
+        return res
+
+    def run_checker(self, res: Path, floors_file: Path, *argv: str) -> int:
+        with (
+            mock.patch.object(check_translations, "RES_DIR", res),
+            mock.patch.object(check_translations, "BASE_DIR", res / "values"),
+            mock.patch.object(check_translations, "FLOORS_FILE", floors_file),
+            mock.patch.object(sys, "argv", ["check_translations.py", *argv]),
+            contextlib.redirect_stdout(io.StringIO()),
+            contextlib.redirect_stderr(io.StringIO()),
+        ):
+            return check_translations.main()
+
+    @staticmethod
+    def recorded(floors_file: Path) -> dict:
+        return json.loads(floors_file.read_text(encoding="utf-8"))["floors"]
+
+    def test_update_floors_still_fails_when_a_locale_is_below_its_floor(self):
+        # It used to skip the gate and report success at 25% against a 50% floor.
+        with tempfile.TemporaryDirectory() as directory:
+            res = self.make_tree(directory, translated=1)
+            floors = Path(directory) / "translation_floors.json"
+            floors.write_text('{"floors": {"values-de": 50.0}}', encoding="utf-8")
+
+            self.assertEqual(1, self.run_checker(res, floors, "--update-floors"))
+            self.assertEqual({"values-de": 50.0}, self.recorded(floors))
+            self.assertEqual(1, self.run_checker(res, floors))
+
+    def test_update_floors_raises_a_floor_the_locale_has_passed(self):
+        with tempfile.TemporaryDirectory() as directory:
+            res = self.make_tree(directory, translated=3)
+            floors = Path(directory) / "translation_floors.json"
+            floors.write_text('{"floors": {"values-de": 50.0}}', encoding="utf-8")
+
+            self.assertEqual(0, self.run_checker(res, floors, "--update-floors"))
+            self.assertEqual({"values-de": 75.0}, self.recorded(floors))
+            self.assertEqual(0, self.run_checker(res, floors))
+
+    def test_an_unreadable_floors_file_fails_the_check_and_is_not_rewritten(self):
+        with tempfile.TemporaryDirectory() as directory:
+            res = self.make_tree(directory, translated=3)
+            floors = Path(directory) / "translation_floors.json"
+            floors.write_text("not json", encoding="utf-8")
+
+            self.assertEqual(1, self.run_checker(res, floors))
+            self.assertEqual(1, self.run_checker(res, floors, "--update-floors"))
+            self.assertEqual("not json", floors.read_text(encoding="utf-8"))
+
+
 if __name__ == "__main__":
     unittest.main()

scripts/translation_floors.json

@@ -1,5 +1,5 @@
 {
-  "description": "Minimum translated-string coverage per locale, in percent. check_translations.py fails when a shipped locale drops below its floor. Raise a floor with --update-floors; never lower one by hand.",
+  "description": "Minimum translated-string coverage per locale, in percent. check_translations.py fails when a shipped locale drops below its floor. --update-floors only raises a floor. Lowering one is a hand edit, made on purpose and explained in the commit message.",
   "floors": {
     "values-zh-rCN": 76.5
   }