test(i18n): gate unused translation keys and prune dead ones - #356
Closed
RonenMars wants to merge 1 commit into
Closed
test(i18n): gate unused translation keys and prune dead ones#356RonenMars wants to merge 1 commit into
RonenMars wants to merge 1 commit into
Conversation
Adds a dead-key test asserting every English key is referenced somewhere in the source, complementing the existing key-parity gate in i18n-completeness.test.ts.
The scanner reads `t('ns:key')` and bare `t('key')` calls, keeps the static prefix of dynamic keys as a prefix match, and picks up keys hoisted into consts before reaching t().
Arrays are treated as leaves since they are fetched whole via `returnObjects: true`.
Removes the 161 keys it flagged across all four locales — 654 removals, since ar and ru carry extra CLDR plural forms.
Every flagged key was verified against a raw grep to rule out false positives.
Repoints the plural and interpolation assertions in i18n.test.ts onto keys the app actually uses, since the previous fixtures were among the dead keys.
This was referenced Jul 22, 2026
RonenMars
added a commit
that referenced
this pull request
Aug 1, 2026
## Why The runbook described itself as a "live procedure — edit it as PRs land", but the chain it sequenced is gone. Verified 2026-08-01 — every PR it orchestrated is `CLOSED` with `mergedAt=null`, i.e. **closed, not merged**: ``` #339 #341 #343 #345 #346 #347 #353 #354 #355 #356 #357 #358 #359 #360 #361 #362 #363 #364 #368 #372 #373 #376 #385 #386 #387 ``` The work reached `main` through the integration branch instead, and the PRs were closed as redundant. The integration branch the runbook treated as proof the set could coexist, `integration-dev/v1.0.0-2026-07-22`, has been deleted. That invalidates the phase order, the per-PR loop, the pre-flight sweep, the decision to close `#355`, the conflict **A–I** resolutions, and the definition of done. The only PR row still accurate is `#291`, which is still open and still excluded by standing request. ## What changed The body becomes a tombstone: what the runbook was for, what actually happened, and a table pointing at where each still-true trap now lives. The file is kept rather than deleted so inbound links resolve and nobody rebuilds the same plan from the merge report. Two of its four traps were already in `docs/troubleshooting.md` (the `.claude/` worktree gotcha and serial `SessionScreen` verification). The other two are lifted there now under a new "CI signals" section, because both are repo-wide facts rather than properties of those branches: - a `[skip-ci]` head commit reports every required context green in seconds having run nothing - a stacked PR (base ≠ `main`) still gets full CI here, since `test.yml`'s `pull_request` trigger has no `branches:` filter Conflict **J** is obsolete for a separate reason: those `ios/Podfile.lock` checksums encode the checkout's absolute path, so resolving it was never meaningful. #469 added `scripts/reset-podfile-lock-path-noise.sh`, which drops that drift. ## Two live findings, one already fixed The runbook's `[skip-ci]` section claimed the required contexts were `Type check`, `Unit tests`, `Integration tests`, `Lint`. The ruleset actually required **only `Lint` and `Type check`** — unit and integration tests could not block a merge at all, which makes the `[skip-ci]` trap worse than documented, not better. Its one outstanding post-merge action (add `i18n` to required checks, never done since #368) is now complete: `Unit tests`, `Integration tests` and `i18n` were added to the `main` ruleset on 2026-08-01. Confirmed safe first — all three report `pass` on a real `chore/bump-**` version-bump PR (#472), so the deploy pipeline's bumps are not deadlocked. Note: `.github/workflows/test.yml:25` carries the same wrong claim about which contexts are required. Left alone here to keep this docs-only.
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
1 participant
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.
What
Adds the missing half of this repo's i18n validation and cleans out the dead keys it immediately surfaced.
We already had
__tests__/i18n-completeness.test.ts, which enforces key parity between every locale and English in both directions (missing + stale).What was absent was the reverse check: whether a key that exists in the JSON is actually used anywhere in the app.
This PR adds
__tests__/i18n-unused-keys.test.tsto close that gap.How the scanner works
It walks
app/,components/,hooks/,lib/,services/,utils/,contexts/,stores/and collects key references three ways, since the codebase reads keys in three different shapes.t('ns:key')and baret('key')under a per-file namespace — matching is on the key path alone, namespace-agnostic, because the same path can be reached either way.t(`scanner.errors.uri.${err.code}`)cannot be resolved statically, so the literal part before the first${is kept as a prefix match that covers every branch.t()—SlowLoadingBannerholdsTITLE_KEYSin an array — are caught by a pass over quoted dotted string literals.Arrays are treated as leaf values rather than being flattened into indices, because they are fetched whole via
t(key, { returnObjects: true })—browse:starting.phrasesis the live example.The pruning
The gate flagged 161 unused English keys, which came to 654 removals across all four locales;
arandruare higher because they carry extra CLDR plural forms thatendoes not.Every one of the 161 was checked against a raw grep over the source before deletion, so this is not a regex-confidence cleanup.
Two keys grep-matched but were confirmed dead anyway:
conversation.titlecollided with the JS propertyconversation.title, andsearch.clearwas a prefix of the livesearch.clearSearch.Three of the flagged keys (
sessions:card.connectedTo,sessions:list.sessionCount_one/_other) were dead in the app but were the fixturesi18n.test.tsasserted on for plurals and interpolation.Rather than keep dead keys alive purely to serve a test, those assertions are repointed onto keys the app actually uses —
sessions:card.promptsfor plurals andservers:error.subtitlefor interpolation — so the coverage is preserved and the locales stay honest.Verification
Full suite green at 108 suites / 1051 tests, ESLint clean on all changed TS files.
tsc --noEmitreports 9 errors both before and after this change; all are pre-existing expo-router path typing, none i18n-related.All locale JSON still parses and no empty parent objects were left behind by the deletions.
Follow-up, not fixed here
components/servers/ServerListCard.tsx:67hardcodes an English string,`Disconnect from ${server.label || server.url}?`, whereservers:dialog.removeMessagewas evidently intended.That hardcoding is exactly why the key read as unused.
It is a genuine pre-existing i18n gap but out of scope for this PR, so the orphaned key was removed and the bug left for a separate change.