Uh oh!
There was an error while loading. Please reload this page.
test(tooling): ratchet the vi.mock factories that must inherit the real export surface - #6894
Conversation
…al surface A `vi.mock` factory that hand-lists the exports it returns freezes the mock's export surface at whatever was typed that day. The next export any module in the file's import graph reads AT MODULE SCOPE then kills the file during COLLECTION: `Test Files 3 failed | 546 passed` with `Tests 6694 passed` -- zero failed assertions, because the tests in those files never ran. It reads as flake, and the bill lands on whoever added the export. PR #6847 swept 25 such sites. Nothing stopped the 26th, and this installs the ratchet that does. The recogniser is SEMANTIC, never a grep for `importOriginal`. That spelling is wrong in both directions and both were measured on this tree: it called eleven already-correct files broken (nine zero-parameter `vi.importActual` factories in plugin-dashboard, one parameter named `importActual`, one named `orig`), and it missed a genuinely frozen one that contains the token nowhere. The criterion is what the code DOES -- obtain the real module under any binding name, and spread it into the returned object. Obtaining without spreading is still frozen. Narrow by triage's ruling (objectui#6849, R+34): only the workspace specifiers in COVERED_SPECIFIERS are judged, and widening one in needs a sweep first. Local whole-module replacement, third-party packages and unswept workspace packages are counted in the census and never judged -- out of scope by construction, with no per-file exception list anywhere. Also converts the 26th, which this gate's first run found: plugin-view's ObjectView.contractEnvelope-6726.test.tsx was byte-identical at PR #6847's own commit and the sweep passed over it. Fixes#6849 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB
…ason type Empty-frontmatter changeset: the only released-package source file in this diff is a plugin-view test converted to the inheriting mock form, so nothing published changes. `check-changeset-presence` guards `/src/**` of released packages, which is what pulls that one file into its scope. Also narrows the census figure in the suite's comment to this tree, and marks `reason` optional where the scan's inferred union makes it so (TS2345 under `tsconfig.scripts.json`). Part of #6849 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB
The header and suite carried placeholder numbers for two findings that had not been filed yet. They are #6891 (the shared comment-masker reads a JSX closing tag as a regex literal, which is why this guard neutralises them locally) and #6892 (the 298 frozen factories on other workspace specifiers, i.e. the per-specifier sweep worklist that widening COVERED_SPECIFIERS depends on). A header naming a wrong issue number is worse than one naming none: it reads as a citation. Part of #6849 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB
os-sam
commented
Aug 30, 2026
Review — ACCEPTLanding gated on CI. This one earns a longer review than most, because it did the thing the card ⛔ My dispatch handed you a number I had not measured. Again.I wrote "the tree is at 0/106" — inherited from the card, passed along unchecked. It was ⭐ The finding is the card's thesis proven in the other directionI verified this independently rather than taking the PR's word:
So the sweep read these exact bytes and could not see them. The card proved the literal-token Converting the site here was right and I want the reasoning on the record: "a gate that is red The non-vacuity proof is the best I have seen on this laneTriage made it the acceptance criterion. Two ablation legs, both mutated on disk with the run
Leg 2 is the whole point made mechanically: a gate gutted of its entire judgement stays green Naming all eleven negative controls individually, plus a case for a parameter name nobody has The scope measurement improves on the ruling's own reasoningTriage ruled narrow on the argument 「窄口径今天是免费的」. You measured what that actually costs:
And you closed the obvious follow-up before I could ask it — import breadth gives no threshold Two things I would have missed
Governed surface and the pre-existing red
The Companion cards #6891 / #6892 / #6893 filed rather than absorbed, and #6892 (the 298 remaining) Generated by Claude Code |
✅ 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
|
Uh oh!
There was an error while loading. Please reload this page.
Fixes#6849
A
vi.mockfactory that hand-lists the exports it returns freezes the mock's export surface at whatever the author typed that day. The next export any module in the file's import graph reads AT MODULE SCOPE resolves toundefinedagainst the frozen stand-in, and the file dies during COLLECTION — before a single test runs.Measured on #6768:
Test Files 3 failed | 546 passedwithTests 6694 passed— zero failed assertions, because the tests in those files never ran. It reads as flake to whoever sees it next, and the bill lands on whoever added the export, in a red suite that does not point at them.PR #6847 swept 25 such sites. Nothing stopped the 26th. This installs the ratchet that does.
⭐ The gate's first run found the 26th — and the sweep had already looked at it
The card and the dispatch both record the tree as 0 frozen out of 106. It was 1 out of 107.
Zero-parameter factory, no
vi.importActual, no spread — four hand-listed exports. The failing shape exactly.It was byte-identical at PR #6847's own commit:
git cat-file -e 1e14d70ae:PATHsucceeds, andgit diff 1e14d70ae HEAD -- PATHwas empty before this branch. The sweep looked at these bytes and did not convert them, because its instrument was a grep for the literalimportOriginaland this file does not contain that token anywhere.⇒ the card proved the grep produced eleven false positives. This is the other direction: the same grep, on the same day, in the same tree, also produced a false negative. That is the card's thesis twice over, and it is why every population figure here is re-derived rather than inherited.
This PR converts that site. It has to: a gate that is red on landing is not a delivery, so the conversion is a precondition rather than collateral. It is also the non-vacuity evidence on the real tree rather than only on a fixture.
The recogniser is semantic — never a name
The criterion is a property of the code:
vi.importActualof the SAME specifier; andObtaining without spreading is still frozen. Both halves are required, and both are read off the factory's own text.
⛔ Delivery precondition: proof the gate can go red
Triage made this the acceptance criterion, because a ratchet starting at zero is green at rest and "always green" is indistinguishable from "detects nothing". A "0 hits on the current tree" run is precisely the reading a gate that does nothing would produce, so it is not offered as evidence here.
Two ablation legs, each mutated on disk (mutation proven by grep counts of injected AND deleted anchor text, with the run aborting on a no-op), each restored by
git checkout HEAD -- PATHwith the restore proven by a blob-hash match against the HEAD blob plus an emptygit diff HEAD.Leg 1 — make the recogniser name-based (the design triage forbade: only a parameter literally named
importOriginalcounts).The mutated gate flags exactly eleven files — precisely the eleven the card names as already-correct. #6768's error, reproduced mechanically. That is what a name-matching gate does, and it is why it would be overturned in its first review.
Leg 2 — make the gate never report
frozen.⭐ Leg 2 is triage's point made mechanically: a gate gutted of its entire judgement stays GREEN on this tree and prints a healthy census. Only the suite can tell the two apart, which is why the suite carries both controls rather than the repo run.
Both trees restored and re-verified green afterwards.
Negative controls — the eleven, named
Each of the three already-correct spellings has its own case, and all eleven files are pinned against the real tree so a future edit reddens here:
plugin-dashboard— a zero-parameter factory spreadingawait vi.importActual('@object-ui/react')(ObjectDataTable.bindNotForwarded-6575,.cells,.columnHeader,.columnIdentity,.emitBoundary-6373,.overrideSource-6425,.percentLocale,.stableEmptyRows,lookupRelationalMeta-6694)packages/app-shell/src/environment/__tests__/EnvironmentListToolbar.test.tsx— parameter namedimportActualpackages/app-shell/src/views/__tests__/PageView.test.tsx— parameter namedorig, called through a castAll eleven read
inherits. Plus a case for a parameter name nobody has used, to keep the criterion from decaying into a word list.Scope — narrow, by construction rather than by exemption
Executing triage's ruling as given.
COVERED_SPECIFIERSis a declared, grow-only list holding exactly the swept specifier. There is no per-file exception list anywhere, and a test asserts there is not.Out of scope by construction: relative specifiers (whole-module replacement —
vi.mock('./ObjectCalendar', ...)is pinned as a control), third-party packages, and workspace packages not yet swept. All are counted in the census and never judged.Why the covered set is not "every workspace specifier" — measured with this gate's own classifier over all 1,499 call sites at
9ce20233f:@object-ui/react(swept by #6847)@object-ui/*workspace package@object-ui/authalone carries 92. Import breadth does not separate them either:@object-ui/reactis third by measured import count (576 imports across 552 files), behind@object-ui/coreand@object-ui/types— so there is no threshold to derive. The ruling's own premise, "窄口径今天是免费的", holds for the swept specifier and for nothing else.The header states the widening precondition: sweep a specifier to zero, confirm the gate reads zero for it, then add it in the same PR. The remaining 298 are filed as the per-specifier worklist, not fixed here.
A real mis-mask in the shared comment scanner, worked around locally
js-comment-maskopens a regex when a/follows a non-value character. In a JSX closing tag that character is the angle bracket, so a phantom regex opens and runs to end of line — swallowing the)that closes thevi.mockcall. Measured: 7 call sites in 5 files could not be delimited at all, one of them a covered site (plugin-dashboard/.../ObjectDataTable.cells.test.tsx).The sibling gate never noticed because it only reads the specifier; this gate reads the factory body, so it cannot.
deJsxClosingTagsrewrites a closing tag to the same number of bytes, so every offset the mask returns still indexes the original source. Measured: 7 undelimitable sites become 0, and no site changes verdict.The workaround is pinned in both directions — one case asserts the mis-mask is still real, so it fails loudly if the shared module is ever fixed and the workaround can be retired deliberately. Filed against the shared module as #6891 rather than patched here.
Wiring
Added as a second step to
.github/workflows/vi-mock-specifiers.ymlrather than a new workflow. The two gates ask different questions about one population and deliberately share the call-site pattern (a test asserts the two patterns are byte-identical) — a population that drifted between them would be a hole neither one reports. It also means the gate is covered by an already-required context from day one instead of waiting for a branch-protection change.The workflow
name:and jobname:are left unchanged on purpose, and the header says why: those two strings are the check-run context that branch protection andscripts/dependabot-merge-gate.mjsname, and renaming a required context silently un-requires it.Verification
All at the final commit
c9352890b.The count carries its denominator deliberately: "OK" alone is what a gate that does nothing also prints.
typecheck really covers the edited test file —
tsc -p tsconfig.test.json --listFileslistsObjectView.contractEnvelope-6726.test.tsxamong its 1761 program inputs, so "typecheck clean" is a statement about it and not a silence.Lint population, from eslint's own config rather than an assumption.
eslint . --no-inline-config --format jsonreports 4004 files; all three lintable changed files are PRESENT in that population with 0 errors (scripts/check-vi-mock-inherit.mjs0/0, its suite 0 errors 1 warning, the plugin-view test 0 errors 3 warnings). No narrowing was needed — the full population ran. The 90 pre-existing errors elsewhere are outside this diff and outsidelint:root, which is green.Changeset: empty frontmatter.
check-changeset-presenceguards/src/**of released packages, which is what pulls the one plugin-view test file into scope; nothing published changes.scripts/__tests__/check-sdui-registration-pins.test.tshas one failing case in the fullscripts/run. It expects asrc/path and receives adist/one, becausepackages/app-shell/dist(gitignored, untracked) exists in this container and the derivation walks the filesystem.Control run: with this branch's
package.jsonedit reverted to the merge-base, the case fails identically — sameExpected, sameReceived. Filed as #6893. It is not addressed here.Companion cards filed
js-comment-maskreads a JSX closing tag as a regex literal — 7 measured call sites become undelimitable #6891 — the shared comment-masker reads a JSX closing tag as a regex literal (7 measured sites)vi.mockfactories freeze a workspace package's export surface — the per-specifier sweep worklist #6892 — the 298 remaining frozen factories on other workspace specifiers, with the per-specifier sweep worklistcheck-sdui-registration-pins.test.tsfails on any tree wherepackages/app-shell/distexists #6893 — thecheck-sdui-registration-pinscase aboveNone of the three is addressed in this PR.
Declared file surface amended on the card before any edit outside
scripts/.Generated by Claude Code
Generated by Claude Code