Repository navigation
i18n CI: make the coverage gate diff-scoped again, not a full-tree invariant - #524
Merged
Merged
Conversation
This was referenced Jul 16, 2026
digitalerdude
force-pushed
the
fix/452-i18n-diff-scoped-gate
branch
2 times, most recently
from
July 17, 2026 10:45
4ec4221 to
e3275f4
Compare
digitalerdude
marked this pull request as ready for review
July 17, 2026 10:46
digitalerdude
force-pushed
the
fix/452-i18n-diff-scoped-gate
branch
2 times, most recently
from
July 25, 2026 14:17
af3ac81 to
77de488
Compare
…variant ci_check() audited the whole merged tree unconditionally once the ryanbr#453 backlog closed, ignoring base_ref entirely. That means a transient regression on main (the 9.0.1 release generating a raw AppChangelog.kt literal, fixed by ryanbr#514) red-flagged every open PR whose diff never touched the offending file — and those PRs stayed red, since a pull_request-triggered workflow doesn't re-run just because main changed. ryanbr#508 and ryanbr#506 are still showing that stale failure right now, over 5 hours after ryanbr#514 fixed main. Restores the base_ref diff (Android side already had the git_show/ literals_at_ref scaffolding for this, just unused) across every check: hardcoded literals, missing-translation keys, and format-string mismatches, on both platforms. A violation already present on base_ref isn't this PR's fault and no longer fails it; existing debt is still printed for visibility. Today that's zero everywhere, so the practical behavior on a healthy main is unchanged — this only changes what happens when main itself regresses.
digitalerdude
force-pushed
the
fix/452-i18n-diff-scoped-gate
branch
from
July 27, 2026 13:59
77de488 to
301a39a
Compare
…coped-gate # Conflicts: # Tools/i18n_audit.py
…coped-gate # Conflicts: # Tools/i18n_audit.py
…coped-gate # Conflicts: # Tools/i18n_audit.py
ryanbr
added a commit
that referenced
this pull request
Sep 19, 2026
#524 made `--ci` exempt any violation already present at the base ref, which is what stops a regression on main red-flagging every open PR. The comment beside the invocation still said the opposite: ... `--ci` is a whole-tree gate, so the value it resolves to does not change the verdict. Comment only, no behaviour change. It now records the two things a future editor of this file needs and cannot see from the YAML. `fetch-depth: 0` is load-bearing: the audit reads the base through `git show`, so a shallow clone misses every base lookup. That direction fails safe, since nothing gets exempted and the gate fails loudly, but it fails for a reason nobody would guess from the error. And on a PUSH to main the fallback resolves to origin/main, which IS the pushed commit, so that run compares the tree against itself and cannot fail. Verified by planting a hardcoded literal and running both ways: base=HEAD passes, base=HEAD~1 catches it. The pull_request run is the one that bites; this job on main is a no-op today rather than the main-side safety net the push trigger was added as. Recorded here rather than changed, because the fallback is a behaviour question for a separate change. Sharpened after reading this file's own header: the push trigger was added for a named, recurring case. A release pushes straight to main with no PR, and appchangelog-gen.py emits the What's New title as a raw Kotlin literal that has to be localized by hand. Auditing the push is how that gets caught the moment it lands. The comment now says so, rather than calling it a safety net in the abstract.
ryanbr
added a commit
that referenced
this pull request
Sep 19, 2026
#524 made `--ci` exempt any violation already present at the base ref, which is what stops a regression on main red-flagging every open PR. It also made the base ref decide the verdict, and the push path was still resolving it the old way. On a push, `github.base_ref` is empty and the fallback resolved to `origin/main`, which IS the pushed commit. That run compared the tree against itself and could never fail. This is not abstract. The push trigger exists, per this file's own header, because a RELEASE pushes straight to main with no PR and `appchangelog-gen.py` emits the What's New title as a raw Kotlin literal that has to be localized by hand. Auditing the push is how that gets caught the moment it lands. Simulated exactly that, using the real 11.8.0 title as the planted literal: base = origin/main (the pushed commit) exit 0 slips through base = github.event.before exit 1 FAIL 1 new hardcoded literal android/.../SleepScreen.kt: 'A gym log book on your wrist, a Coach you can switch off' So a push now diffs against `github.event.before`, the tip before the push, which spans every commit the push carried rather than only the last one. A pull request is unchanged: it still diffs against the branch it targets. `event.before` is all zeros on a branch's first push and absent on a manual run. Neither can be diffed, so both fall back to `HEAD~1`: narrower, but still a real comparison rather than a self-comparison. All three paths exercised locally. The comment now describes the behaviour as fixed rather than as broken, so main never carries a note saying this run is inert.
ryanbr
added a commit
that referenced
this pull request
Sep 19, 2026
…2329) ci(i18n): diff a push against the tip before it, not against itself #524 made `--ci` exempt any violation already present at the base ref, which is what stops a regression on main red-flagging every open PR. It also made the base ref decide the verdict, and the push path was still resolving it the old way. On a push, `github.base_ref` is empty and the fallback resolved to `origin/main`, which IS the pushed commit. That run compared the tree against itself and could never fail. This is not abstract. The push trigger exists, per this file's own header, because a RELEASE pushes straight to main with no PR and `appchangelog-gen.py` emits the What's New title as a raw Kotlin literal that has to be localized by hand. Auditing the push is how that gets caught the moment it lands. Simulated exactly that, using the real 11.8.0 title as the planted literal: base = origin/main (the pushed commit) exit 0 slips through base = github.event.before exit 1 FAIL 1 new hardcoded literal android/.../SleepScreen.kt: 'A gym log book on your wrist, a Coach you can switch off' So a push now diffs against `github.event.before`, the tip before the push, which spans every commit the push carried rather than only the last one. A pull request is unchanged: it still diffs against the branch it targets. `event.before` is all zeros on a branch's first push and absent on a manual run. Neither can be diffed, so both fall back to `HEAD~1`: narrower, but still a real comparison rather than a self-comparison. All three paths exercised locally. The comment now describes the behaviour as fixed rather than as broken, so main never carries a note saying this run is inert.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
ci_check()inTools/i18n_audit.py(#452) originally audited the whole merged tree unconditionally once the #453 backlog closed —base_refwas accepted on the CLI but never actually used. That means a transient regression onmainred-flags every open PR, including ones whose diff never touched the offending file.That's exactly what happened: the 9.0.1 release generated a raw literal into
AppChangelog.kt(fixed by #514), and it broke the gate on #507, #512, #513 — then #514 fixedmain, but #508 and #506 kept showing the stale failure for hours afterward, because apull_request-triggered workflow doesn't re-run just because the base branch changed.(#509 independently re-implemented the same
AppChangelog.ktfix as #514, which had already landed — it didn't touch this underlying issue.)Since this PR was opened, #558/#581 landed a much-improved literal scanner (catches literals hidden behind if/else/when/.let, and contentDescription inside a semantics{} lambda) plus a baseline (
Tools/i18n_audit_baseline.json) to exempt the ~250-entry backlog that scanner surfaced. The baseline solves a different problem than this PR, though: it's a fixed snapshot, so a new regression onmainafter the snapshot (the same class of incident as #514) would still red-flag every unrelated open PR until someone manually updates the baseline.What
ci_checknow exempts a violation on either of two independent grounds:base_ref— so this PR didn't cause it, regardless of whether it's made it into the baseline yet.Locale-gap and format-mismatch checks (missing translations, printf-argument mismatches) aren't covered by the baseline at all — the
base_refdiff is their only exemption. On a healthymainthis is a no-op: the gate behaves identically to before. It only changes what happens whenmainitself regresses — that regression now stays scoped to whoever caused it, instead of spreading to every other open PR, independent of whether the baseline has caught up yet.Verification
python3 Tools/i18n_audit.py --ci HEADon the current tree → exit 0 (192 Android + 64 iOS pre-existing findings correctly reported as exempt, not failures).base_refto the pre-i18n: localize the 9.0.1 What's New title (unblock the coverage gate) #514 commit (still carrying the brokenAppChangelog.ktliteral) → reported as pre-existing, exit 0. Under the gate as it stood before this PR, that same state hard-fails (confirmed against Clarify phone heart-rate broadcast copy #508/Restore recognizable Android dashboard metric icons #506's actual CI logs at the time).--ci HEAD→ still exit 0, proving thebase_refexemption works independently of the baseline (this is the actual value of the PR: a future regression not yet in any baseline still won't spread to unrelated PRs).Text(...)call not present in the baseline or onbase_ref→ correctly caught, exit 1; reverted, exit 0 again.<plurals>(i18n: real <plurals> instead of hand-rolled singular/plural pairs #541) and Applevariations.plural(i18n: teach the Apple audit about xcstrings plural variations #542) regressions (deleted a real translated plural / broke placeholder parity on a synthetic multi-form plural) → both still correctly caught after merging with this PR's diff-scoping.python3 Tools/i18n_audit.py(plain report mode) and--platform ios/android --fulland--update-baselineall still run correctly and match the committed baseline exactly (no drift).Scope
Python-only change to
Tools/i18n_audit.py, no workflow YAML changes needed. No app code touched.