test(i18n,app-shell): discover the createSafeTranslation defaults tables instead of hand-listing three (objectui#7884) - #7902
Merged
Conversation
Contributor
ℹ️ Console Performance Budget — not measuredThis run did not produce a console bundle to measure, so there is no pass/fail verdict for the performance budget. This is not a budget violation. Nothing was measured — the numbers a real violation would carry are simply absent.
See the workflow run for details. 📦 Bundle Size Report
Size Limits
|
…les instead of hand-listing three (objectui#7884)
The rule "every row names a key the en pack actually defines" is the one rule in
this repo that rejects a defaults row whose key no locale pack defines. It did
not miss objectui#7874's five dead `timeline.relative.*` rows because it was
wrong -- it never saw them: its `MAPS` was a hand-written list of three IMPORTED
maps, and `TIMELINE_DEFAULT_TRANSLATIONS` was not on it.
Measured: the three-map list judged 400 of 1056 rows (37.9%). It was also
structurally incompletable by its own mechanism -- 11 of the 32 factory call
sites pass an anonymous inline object literal exported under no name at all, so
no list of imports could ever reach them.
The AST walk that discovers these tables already existed, module-private inside
`fallback-placeholder-spelling-3512.test.ts`. It is MOVED, not copied, into
`@object-ui/test-support`, so both gates ask their different questions of one
definition of the population instead of two that drift apart.
Behaviour of the objectui#3512 gate is preserved, verified on one tree either
side of the move: 35 discovery entries, 1072 rows, 0 unreadable, the same 4
needle files, and an identical sha256 over every row
(f7910279e114cd6e1bf1d2b97bf7164661283cd9fbb5eef88286f2f75bea8065).
The scanner is reached as a DECLARED subpath, `@object-ui/test-support/
defaults-table-scan`, and NOT from the package index. It reads the workspace
from disk, so it only type-checks where Node's ambient types are present, and a
barrel re-export puts a module into the program of every consumer that imports
the index for anything at all. Measured with it on the index: `data-objectstack`
imports `{ enumOptions }` from that index in one test, so `tsc -p
packages/data-objectstack/tsconfig.json --listFiles` pulled this file into that
package's program and the repo-wide type-check failed with three TS2591s in a
file that package never asked for. `./zod-wrapper-keys` is the same escape hatch
for the mirror case, and objectui#4325's lesson is untouched: the hazard there
was an UNDECLARED deep path, TS2882 for `tsc`; this one is in `exports`.
One spelling also had to change. The walk skips `__tests__` directories, which is
why the needle could be a single literal while it lived in one; in a normal
`src/` module a verbatim spelling makes the scanner match its own source and
report itself as a fifth interpolator. It is assembled from fragments now, with
the runtime value unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KbJQ1y1J12nZxYzFWhP8Q3
os-sam
force-pushed
the
claude/issue-7884-defaults-maps-mirror-discovery
branch
from
September 6, 2026 02:58
52c8b37 to
c76fa97
Compare
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
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
2 participants
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.
Fixes #7884
Blocked-by: #7887— satisfied: #7887 merged at 2026-09-06T01:55:20Z asd9580f464, and this branch is rebased onto it. The line is kept for the record because it explains the ordering: while those five rows were still onmain, the gate below could only land red.What this changes, and what it deliberately does not
This is not a new rule.
defaults-maps-mirror-en-pack.test.tsx(#4401) already had the one rule in this repo that rejects acreateSafeTranslationdefaults row whose key no locale pack defines: every row names a key the en pack actually defines. It did not miss #7874's five deadtimeline.relative.*rows because it was wrong — it never saw them. ItsMAPSwas a hand-written list of three IMPORTED maps, andTIMELINE_DEFAULT_TRANSLATIONSwas not on it.A correct rule held outside the door by a hand-written list — the same family as #7448 / #7528 / #7548 / #7825 / #7853.
The census, measured before deciding anything
Run over the AST-discovered population on the merged tree (
d9580f464+ this commit):The result, stated positively
With #7874's five rows retired, this defect class is empty repo-wide: 0 offending rows across 34 unique tables and 1056 rows. That is a measured zero, not an absence of looking — and the number that makes it a real zero is the one beside it: blind spot 0. Every discovered table resolved to an object literal and every row to a static string, so there is no bucket of "rows I could not read" hiding behind the 0. That is also what ruled out the "the instrument is not trustworthy" branch by measurement rather than by opinion.
Why the old list said 3 and the walk says 32 — the card's real justification
The existing gate's 3 were hand-listed exported, importable maps, because that file reaches its tables by
import { X_DEFAULT_TRANSLATIONS }. The walk instead resolves every factory call site's first argument, and 11 of today's 32 factory call sites pass an anonymous inline object literal that is exported under no name at all.So the hand list was not merely incomplete — it was structurally incompletable by its own mechanism. No amount of diligence in maintaining a list of imports could ever have reached those 11. Coverage before this change: 400 of 1056 rows, 37.9%; the rule was held outside 62% of its own population.
Dedupe, on
file:line+ keyTIMELINE_DEFAULT_TRANSLATIONSis discovered twice — once as the factory call inuseTimelineTranslation.tsand once through #3512's hand-rolled registry, which keeps it so the registry mirrors its needle-file set. Its rows are therefore scanned twice, which is why five real rows read as ten raw hits during the census. The landed gate de-duplicates on the row's own location plus its key (not the table label — the two discoveries carry different labels), so it can never report inflated numbers.The discovery mechanism was MOVED, not copied
The AST walk already existed, module-private inside
packages/i18n/src/__tests__/fallback-placeholder-spelling-3512.test.ts(#3512). Writing a second one would have been two definitions of the population that drift apart — the very disease this card treats. So it moved, whole, into@object-ui/test-support(private: true, never published), and both gates now discover the same tables from the same code.Home chosen per the PM ruling: the two callers live in different packages, and a deep relative import into another package's
src/__tests__/is the shape #4325 ruled out.@object-ui/test-supportexists precisely for modules two suites share.It is reached by a DECLARED subpath, not from the package index
@object-ui/test-support/defaults-table-scan. The index is otherwise that package's whole surface, and this is the one shape the index physically cannot carry: the module reads the workspace from disk (node:fs,node:path,node:module), so it only type-checks in a program that has Node's ambient types — and a barrel re-export puts a module into the program of every consumer that imports the index for anything at all.That is measured, not predicted. The first revision of this PR did re-export it from the index, and the repo-wide type-check went red:
The chain, read off
tsc -p packages/data-objectstack/tsconfig.json --listFilesrather than inferred from the message: that package'sincludeissrc/**/*, which takes in its tests;src/metadata-client.overlayScope.test.ts:41imports{ enumOptions }from the index; the index re-exported the scanner; so all nine of test-support's modules landed indata-objectstack's program, which carries nonodetypes. After the change to a subpath the same command reports 0 occurrences ofdefaults-table-scanin that program, and the other eight modules stay — the package's normal surface is untouched../zod-wrapper-keysis the same escape hatch for the mirror case (bare-node consumers the TypeScript index cannot serve), so the precedent is the package's own. #4325's lesson is untouched too: the hazard there was an undeclared deep path that resolved only through the vitest alias and was TS2882 fortsc; this one is declared inexports, and withmoduleResolution: bundlerit resolves exactly as.does. The reasoning is written at the index, where the next person will look.A package's surface is its index — including this one). It is narrowed rather than overturned, with the measurement recorded beside it — say the word if you would rather have the module relocated instead.The move is proven behaviour-preserving, not merely called mechanical
The #3512 gate measured on ONE tree, its own private walk versus the shared module:
f7910279e114cd6e1bf1d2b9...f7910279e114cd6e1bf1d2b9...Full digest both sides:
f7910279e114cd6e1bf1d2b97bf7164661283cd9fbb5eef88286f2f75bea8065. Its floors (at least 34 tables/at least 700 rows) are not the evidence here — the real counts are, because a floor still passing proves only that nothing fell below it.One spelling had to change, and it is a finding in itself
The walk skips
__tests__directories. That is why #3512 could hold the literal needle as a single string while living in one. A normalsrc/module cannot: with the needle spelled verbatim, the scanner matched its own source and reported itself as a fifth interpolator, turning #3512's completeness case red. Measured exactly that way during this move. The constant is assembled from fragments now, with the runtime value byte-identical, and the reason is written at the constant so the next person does not "tidy" it back.Positive control — the real defect, both directions, on this tree
Not a fabricated fixture: #7874's five rows, put back exactly as
d9580f464deleted them.c02661e28d...moved to67ff9746...,relative_rows=5,1 file changed, 5 insertions(+). The widened gate then fails, namingTIMELINE_DEFAULT_TRANSLATIONSand all five rows — and exactly five, not ten, so the dedupe holds:git diff HEADis 0 lines, the blob is back to the HEAD blobc02661e28d...,relative_rows=0. Gate green, 15/15.Not a duplicate of #7567 — please do not read it as one
#7567 / #7870 (
factory-default-drift, landed as6ea524f32) compares factory-table rows' VALUES against theenpack, and deliberately abstains whenendefines no value, because with noenvalue there is nothing to compare against. That abstention is correct and must not be turned into a comparison.This gate asks the other question: does the key exist at all. Different question, different rule — and the two are complements, since #7567's abstention bucket is exactly this gate's finding bucket.
Surface
packages/test-support/src/defaults-table-scan.ts— new, the moved walkpackages/test-support/src/index.ts— re-exportpackages/i18n/src/__tests__/fallback-placeholder-spelling-3512.test.ts— delete-plus-import (−300 lines)packages/app-shell/src/__tests__/defaults-maps-mirror-en-pack.test.tsx— the widened rulepackages/i18n/package.json,packages/test-support/package.json— devDep linesOne adjunct beyond the authorised list, flagged rather than slipped in:
packages/test-support/tsconfig.jsonneeded"types": ["node", "vitest/globals"]plus an@types/nodedevDep, because this new module is the first in that package to touchnode:fs/node:path/node:module; without itpnpm --filter @object-ui/test-support type-checkis TS2591. Naming the types replaces the auto-included set, hencevitest/globalsalongside.Verification
Repo-wide
pnpm turbo run type-check(no--filter) on the pushed SHAc76fa9718, withgit diff HEADempty:Tasks: 81 successful, 81 total, exit 0, zero TS diagnostics — including all five packages the earlier failure touched (data-objectstack,types,core,i18n,collaboration). The same command reproduced the failure before the fix (Failed: @object-ui/data-objectstack#type-check), so it is verified by the instrument that caught it, not by a narrowed one.The three pre-landing inputs were re-run after the fix and are unchanged: census still 0 hits / blind spot 0 across 34 unique tables and 1056 rows; the #3512 gate still byte-identical either side of the move (35 / 1072 / 0 / 4 needle files, digest
f7910279e114…); the positive control still fires namingTIMELINE_DEFAULT_TRANSLATIONSwith 5 rows, not 10.Gate union, with
git diff HEADempty — all exit 0:vitest(both suites, 33 passed) ·type-checkfor test-support / i18n / app-shell ·lintfor test-support / i18n / app-shell (0 errors) ·check:readme-exports(afterturbo run build, which it requires) ·check:unreferenced-sources·check:phantom-deps·check:self-import·check:control-bytes·check:type-check-coverage·check:vi-mock-specifiers·check:vi-mock-inherit·check:pre-install-import-graph·check:esm-specifiers·check:lint-coverage·check:i18n-keys·check:i18n-dead-keys·check-changeset-no-major.Type-check greens proven non-vacuous by
--listFiles:defaults-table-scan.tsis in the test-support program (1 hit, and 82@types/nodefiles),defaults-maps-mirror-en-pack.test.tsxis in app-shell'stsconfig.test.jsonprogram (1 hit).Changeset verdict, verbatim from
node scripts/check-changeset-presence.mjs:Authored by the
os-devseat, sessionhttps://claude.ai/code/session_01KbJQ1y1J12nZxYzFWhP8Q3.Generated by Claude Code