Uh oh!
There was an error while loading. Please reload this page.
fix(metadata-protocol): add the missing typecheck script and repair all 63 test-layer type errors, graduating the package out of the DEBT ledger - #14018
Conversation
📓 Docs Drift CheckNothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs. What this run could not see
Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
zhuangjianguo
commented
Sep 1, 2026
PM review — ACCEPT, and ⭐ the card's headline was falsified in the seat's own PR body
1. ⭐⭐ Two of the card's factual claims are false, and the work is still right(a) The count is 63, not 159. Re-measured on current ⭐ The diagnosis is the valuable part: the 159 was most plausibly measured without the dependency closure built, which inflates counts through unresolved-import cascades. (b) "Nothing runs ⇒ The real gap was narrower than the card claimed, and already declared: the surface was ratcheted at 63-tolerated rather than gated at zero. That is the gap this PR closes, and it is a real one — but it is not "type-checked by nothing". 2. Route (a) taken, on the taxonomy rather than on effort63 errors collapse to 6 root causes, two of which carry 56 of the 63 — so (a) is cheap and strictly better, which is the criterion I asked for. ⭐ The largest (34× TS2322) is worth naming because it is the same shape this lane keeps meeting: 3. ⭐⭐ A2.2 was confirmed by RUNNING it, and the distinction was load-bearingI asked for a firing positive control rather than an inherited claim. The seat found that the naive check would have lied:
4. My hard constraint held: the ratchet counted DOWN, never upA2.4 came back partly falsified — "and this is the finding": adding the script does not leave CI unchanged, because the package was never absent from the type-check gates. ⭐ The entry is DELETED, not raised. ⇒ The MAINTAINER-ONLY ratchet-raising path was never touched and the STOP did not fire. That was the constraint I was least willing to delegate, and it is satisfied by measurement rather than by assurance.
5. The green is not vacuous
⭐ No ablation, and the report says why instead of fabricating one: the single causal claim (the TS7006 pile being downstream of the broken import) was proven by the staged re-measure 63 → 41 → 0, in which the 9 TS2835 and all 13 TS7006 cleared together from a 9-line codemod whose landing was proven on disk by before/after greps in both directions. 6. Scope held⛔ No sibling package was given a script — the other 11 script-less packages (including Status⛔ Not released: CI is still in flight at the time of writing. The bar is every check completed and green — not the required subset — because enqueue is irreversible here. On green: flip ready, arm auto-merge, verify enqueue, then verify the landing by content on
Generated by Claude Code |
Uh oh!
There was an error while loading. Please reload this page.
Fixes#13978
Route (a) FIX — chosen on the taxonomy, not on effort appetite. All of this
package's
tsc --noEmiterrors are repaired, thetypecheckscript is added,and the package graduates out of the DEBT ledger in this same PR.
The count is 63, not 159 — and it is already in a ledger
Two corrections to the card, both measured on current
main:1. The fresh count is 63. Re-measured with the dependency closure built
first:
tsc --noEmit -p tsconfig.jsonreports 63 error lines, all 63 in*.test.ts, zero in non-test source (the card's 159 was measured onaee1fd9ec2). The card's three named offenders reproduce to the unit —protocol.stored-migration.test.ts27,seed-loader-multi-value-reference.test.ts10,
protocol.stored-conversions.test.ts7 — so the difference is not thosefiles. 63 is independently corroborated below.
2. "Nothing runs
tscover this package" is no longer true. The packagecarries a
DEBTentry inscripts/check-type-check-coverage.mjs, and itsrecorded number is 63 with a per-code composition that matches this
measurement exactly, code for code:
CI's
typecheck-debtlane runspnpm check:type-check-debt, whoseMEASUREDinvariant re-runs the package's own
tsc --noEmitand reds if the countexceeds the recorded one. So the test surface was already type-checked in CI —
as a shrink-only ratchet tolerating 63, not as a gate tolerating zero. That
is a materially smaller and declared gap than the card describes, and it is
the gap this PR closes.
Taxonomy: 63 errors, 6 root causes, 2 of them dominant
Grouped by root cause rather than by code:
metadatacollapsing tostringin the seed-row parameter type./seed-loaderimport under NodeNextinsertmock read atcall[2]evthelper narrower than the projector it feedsinsertManynot on the declaredIDataEnginereturn type.at(-1)needs lib ES2022They collapse. Two causes carry 56 of the 63, and the ledger's own note
already said so: "read it as two repairs, not as forty." That is the criterion
— route (a) was chosen because the population is concentrated and mechanical,
which is exactly the condition under which (a) is cheap and strictly better than
declaring the surface unchecked.
Root cause 1 (34 errors) — the repair was already written down in this package
makeStubEnginein the two affected files declared its parameter asPartial[Row] & { type, name, metadata: unknown }. On the rowmetadatais thestored string, and
string & unknownisstring— so the plain intersectionrefuses every body written as an object literal, which is the entire seeding
convenience the harness exists for. Both harnesses already did
typeof r.metadata === 'string' ? r.metadata : JSON.stringify(r.metadata)atruntime: the declared type contradicted the implementation beside it.
Three sibling test files in this same package already use the correct form, and
one of them states the reason verbatim:
So this is drift back to an idiom the package had already adjudicated, not a new
shape. Two of the 34 wore a different face (
Type '{ name, label }' is not assignable to type 'string') — the same cause showingmetadataalreadycollapsed to
string.Root cause 2 (22 errors) — one broken import, and the cascade it caused
Nine test files imported
'./seed-loader'without the extension. UnderNodeNext that does not resolve, so every symbol it names becomes
any— whichis where the 13 TS7006 came from. AGENTS.md predicts this exact shape ("a pile
of TS7006 implicitly-any is usually one broken import upstream ... fix the
extension first and re-measure"), and the re-measure confirms it: 63 to 41,
with the 9 TS2835 and all 13 TS7006 clearing together from one 9-line codemod.
'./seed-loader.js'is not a preference here: 8 other files in this packagealready spell it that way,
index.tsamong them, and these 9 were the onlyextensionless relative imports in the entire package.
Evidence
Measured at
381db66f4e, the final commit of this branch, dependency closurebuilt first.
pnpm --filter @objectstack/metadata-protocol typecheck— exit 0(63 to 41 after cause 2, to 0 after all six).
pnpm --filter @objectstack/metadata-protocol test— exit 0,Test Files 148 passed | 2 skipped (150),Tests 2060 passed | 10 skipped (2070).The 2/10 skips are pre-existing; this PR adds no
.skip,.only,@ts-ignoreor
eslint-disable(verified over the diff).tsc --listFilesputs 150 of 150*.test.tsfiles in the program, including all four of the most-repaired files.
pnpm check:type-check-coverage— exit 0, and its own verdict line movesfrom 12 ledger entries to 11:
67/78 workspace packages type-checked (plus the root), 11 in the DEBT ledger (309 frozen raw errors). 372 minus 309 = 63,the entry this PR retires.
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack— all exit 0. Two returned exit 3(PREREQUISITE NOT MET, nothing measured, not a pass):
check:dual-build-cjs-loadsandcheck-test-completeness.mjs, both needing afull workspace build/test summary this seat did not run. CI runs both with the
prerequisite met.
turbobehaviour, verified rather than inherited (with a firing control)The card's claim that
turbo run typecheckruns nothing for a script-lesspackage is correct, and the dry run alone would not have shown it:
@objectstack/core#typecheck— withcommand: 'NONEXISTENT'.turbo run typecheck --filter=@objectstack/core: exit 0,Tasks: 3 successful— and those three arespec:build,types:build,metadata-core:build, the^builddependencies.core:typecheckappearsnowhere in the run output.
it declares the script:
@objectstack/metadata-protocol:typecheck: cache miss, executingthentsc --noEmit, 14 tasks. The task really does run once thescript exists, which is also the proof that this PR puts the package into the
typecheck-workspacelane.The shrink-only ratchet does not count up
check:type-check-debtreturns exit 3 locally (PREREQUISITE NOT MET — itrefuses to measure without the full built closure, deliberately, since an
unbuilt closure measures a different world). Declared narrowing: rather than
run that repo-level sweep, this seat measured the only coupling that could move
another entry —
packages/restis the one ledgered package whose tsconfigpathsredirects@objectstack/metadata-protocolinto the producer'ssource. Its test program contains 23 metadata-protocol source files and 0
*.test.ts; none of the 15 files this PR edits is among them. So no otherledger entry can move, and this package's own entry is deleted, never raised.
Scope
⛔ Not expanded to the sibling packages. Triage measured 12 of 73 packages
with no
typecheckscript,packages/coreamong them. Those 159 — now 63 —errors are this package's; every other package has its own population needing
its own triage. No script was added anywhere else.
packages/coreis untouchedand stays in the ledger.
One bounded in-place edit is declared here rather than left implicit: two
prose comments in
scripts/check-type-check-coverage.mjsmade present-tenseclaims about the entry this PR deletes ("metadata-protocol quotes the misleading
note ... and is skipped for exactly that reason — correct entry"). Left as-is
they would describe a ledger row that no longer exists, which is the note-rot
failure that file legislates against. Both are now past tense and point at the
synthetic self-test case that carries the shape. Comment-only; the rule and its
self-test fixtures are unchanged.
Non-test source: untouched. The STOP condition for route (a) was that the
compiler would demand a behaviour-adjacent change; it did not. Every repair is a
test file, and the two that looked producer-shaped were not:
bodyis alreadydeclared on the projector's own parameter
(
MetadataMutationEvent & { body?: unknown }) and only the test helper wasnarrower;
insertManyis already on the fixture and only the helper's declaredreturn type hid it.
Changeset
None —
skip-changeset. This PR releases nothing: 13 test files, aCI-internal gate script, and one dev-only
scriptsentry in a manifest. Nosrc/non-test source, no published behaviour, no consumer-observable change.That is the gate's own prescription for the tests-only / CI-internal-script case.
The label is applied on the PR.
Clause-②: no. Path limb — nothing underpackages/spec/src/**. Contentlimb — no accept/reject behaviour moves and no published surface changes; a
typecheckscript is not shipped to consumers, and the diff reached no non-testsource, so the condition the PM flagged as the one thing that would flip this
did not occur.
Generated by Claude Code