fix(types): the import boundary keeps the descriptions it imports (objectui#9034) - #9086
Conversation
…jectui#9034)
`stripImportedDefaults` dropped the `.describe()` of every schema it derived.
A zod 4 description is registry state keyed by the node, not `def` state, so
neither derivation carried it: `.removeDefault()` returns the inner node, and
the protocol spells its guidance `.default(v).describe(d)` — `d` sat on the
outer node that was discarded; and `cloneWithDef` builds `new Ctor({...def})`,
which copies `def.checks` faithfully and the description not at all.
Measured across the published surface of @objectstack/spec 17.4.0: 2024 of 2024
described ZodDefault nodes and 1364 described container nodes arrived on this
side with no description. Both now go through one rule, `withDescriptionOf`,
and 0 are lost.
The identity property is untouched: reference-equal node count across the same
surface is 9665 before and 9665 after. The carry uses `.describe()` rather than
a write to `_zod.parent` because it must CLONE — on the `.optional().default()`
branch, taken 267 times, the replacement node is one of the spec's own objects.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jmxdo7bmeqCQHLSfmLVX9w
…iled as The carve-out in the identity-property pin referenced a placeholder number while the finding was still unfiled. It is objectui#9088. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jmxdo7bmeqCQHLSfmLVX9w
✅ 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
|
|
Parked for the night — ⛔ nothing is wrong with this PR. The seat is winding down at the maintainer's instruction (「当前任务处理完就下班」, under a token constraint that also caused objectui#9091 and objectui#9092 to be withdrawn). This PR is State, so the next seat spends nothing re-deriving it:
⭐ One thing the reviewer should be given as a fact about the tree, ⛔ not as this seat's conclusion: the dev refuted the route this seat prescribed — Generated by Claude Code |
|
UN-PARKED — the maintainer asked for existing work to be followed through to merge (「现有的任务跟进到合并」), so the ceiling-tier contract review is running now on head ⛔ The parking note above ( Next: adopt the verdict verbatim or void it entirely — ⛔ never rewrite one — then, on a PASS, strip both carriers together and land by squash. Generated by Claude Code |
|
Contract review record — ADOPTED VERBATIM. Ceiling-tier ( Contract reviewPR: #9086 — "fix(types): the import boundary keeps the descriptions it imports (objectui#9034)" ① What the card asks, and whether the diff delivers exactly thatThe card asks for one thing: The true three-dot diff (
Nothing outside the ask is in the diff. The card's own framing — that the file already had a working "clone rule" against description loss — does not hold, and the PR is right to say so. I verified it from zod 4.4.3 source rather than from the PR (② below): ② What I re-derived myselfEnvironment. Fresh clone in my scratchpad ( The zod 4.4.3 facts, read from source (not from the PR).
So the stated fact holds: a The PR's test and the surrounding suite, at head.
The census, re-derived with my own instrument (scratch vitest file, deleted afterwards; base-version walker copied beside it so both walkers run in one process; (a) A verbatim replica of the PR's pairing (test.ts:234–271) reproduces the PR's table exactly: base 16,120 visited / 9,665 reference-equal / 2,024 described (b) But that instrument misaligns. At test.ts:254, (c) Corrected pairing (branch decided by (d) The 25 "kept" before the fix. All 25 paths are (e) The spec's graph is not mutated. Snapshot of 33,556 reachable nodes (type, description, full registry meta, child identities) before any walk vs after both walkers ran over every root: 0 mutated, 0 new nodes. Probe P2 on the already-optional branch: the output is a clone whose (f) Identity carve-outs, as the PR's test computes them: 1,635 roots; 676 with a default; 959 clean; 15 behind (g) Card-scope and "invented" counts. Top-level object-shape members, deduplicated by object identity: 1,237 object exports / 8,132 members / 1,181 (h) Protocol-side spelling. Over the 22 ESM files in the spec dist: (i) Inner-described spelling. 50 Ablations (each:
Docblock precision probe. Of the 267 described already-optional defaults, the replacement is literally the spec's own inner object 257 times under base; under head 0 are the spec's object and 257 are describe-clones whose parent is that object; 10 have a rebuilt inner. Sibling by construction. Not measured (an unmeasured thing is not a clean thing):
③ Findings
Also for the card's record, not the PR's: the card's statement "none is spelled No blocking finding. The fix is one rule applied at both loss sites, the pin reddens on full revert (5) and on each half alone (3 / 2), the identity property holds for every plain clean export (941 / 941), the two carve-outs are exact and non-vacuous, the spec's shared graph is unmutated, the changeset is a patch that says what the diff does, and the diff contains nothing the card did not ask for. Independence pairSame-session test, applied literally to what the lines carry: the Implemented-by line carries a branch name; the Reviewed-by line carries a session id. A branch is not a session; they do not resolve to the same session. Result: not SELF-REVIEW; the ruling stands. Noted for the record and disregarded as evidence per instruction: both commits on the branch carry a VERDICT: PASS Generated by Claude Code |
Landing provenance — PR #9086 (card objectui#9034)Reviewed and landed head ① contract review — PASS ON RECORD. ⭐ The review refuted the card's own framing, and upheld the dev for refuting the seat's. The card (and my dispatch brief) said the file already had a working "clone rule" against description loss. It did not: ② gates — ③ checks. 35 check-runs, all charter provenance. Seven governing files byte-identical between
|
2 participants
Fixes #9034
What was wrong
stripImportedDefaultsdropped the.describe()of every schema it derived from@objectstack/spec.A zod 4 description is not
defstate..describe(d)stores{ description: d }inz.globalRegistry— a WeakMap keyed by the node — and thedescriptiongetter reads back through_zod.parent, a link only zod's ownclone()sets. So neither of the boundary's two derivations carried it, for the same reason:.removeDefault()returns the node's inner type. The protocol spells its guidance.default(v).describe(d), sodsat on the outer node that this arm discards.cloneWithDefbuildsnew Ctor({ ...def, ...patch }). It copiesdeffaithfully —def.checksabove all, which is what it exists for — and copies the description not at all, because the description was never indef.The second one is the finding the card did not have. The walker's
lazyarm already named description loss as the defect it guards against and namedcloneWithDefas the guard; that sentence was half wrong — the clone rule saveddef.checksand never saved the description. The comment is corrected in this diff rather than left standing.Measurement, re-derived rather than trusted
Instrument: walk every schema-shaped export of every module subpath the spec publishes (read out of its own
exportsmap), run the realstripImportedDefaultsover each, and pair the two graphs node-by-node. Not a replay of the two operations — the function itself.Command (both readings produced by the same instrument, before and after the diff):
Against
@objectstack/spec17.4.0 — 17 of 17 module subpaths loaded, 0 load failures, 1635 schema-shaped exports, 16120 distinct nodes visited:ZodDefaultnodesRestricted to the card's own scope — top-level object-shape members only — 1031 of 1031 described
ZodDefaultmembers lost it before, 0 after. The card filed 110 of 110; the direction is confirmed and the magnitude is larger, because that instrument walked a smaller set of exports. Nothing here contradicts the card's reading, it supersedes its denominator.The strip invents no description either: the count of members that gained one they did not have is 0 before and 0 after.
The fix is one rule, not two
Both derivations now go through
withDescriptionOf. It uses.describe()and not a write to_zod.parent, for a measured reason:.describe()clones, and on the.optional().default()branch — taken 267 times across this surface —.removeDefault()hands back a node that is already omissible, so the replacement is one of the spec's own objects. Carrying metadata by mutation there would relabel@objectstack/specfor every other consumer in the workspace, and every value assertion in the new test would still be green. It also deliberately carries the description and nothing else:z.globalRegistry.get(...)would also hand backid, and re-registering anidrewrites the registry's id map to point at this package's derivation.The identity property is pinned, not assumed
A carry implemented by rebuilding nodes that did not need rebuilding would pass every value assertion above and break the property batch #90's reversibility argument rests on. So it is asserted directly: every export with nothing to strip still comes back reference-equal, and the reference-equal node count across the whole surface is unchanged at 9665.
Two exceptions are pinned by shape, so a new break cannot hide inside a matching count:
lazyexception — it cannot answer "was anything stripped below me?" without forcing the getter, so it always rebuilds.Controls
Every count carries a control that fires; the ablation below is the control for the fix itself.
packages/types/src/zod/imported-defaults.tsreverted to itsorigin/maincontent and nothing else changed, the new test file goes 5 failed / 13 passed, and the five are exactly the description assertions. Restored withgit checkout HEAD -- THEPATH; restoration proven by an emptygit diff HEADand an on-disk blob hash equal to the HEAD blob (15a63179), not by an exit code.ArtifactPackageEntrySchema.manifest.defaultDatasourcecarries"Default datasource for all objects in this package"; before the diff the stripped member readsundefined, after it reads the same string..removeDefault()dropping the description, and a rawnew Ctor({...def})rebuild dropping it, are each pinned. If a later zod propagates either, those go red and tell the next reader the carry is redundant instead of leaving them to re-derive it.Out of scope, filed not ridden
The identity-property pin surfaced a second, unrelated defect in the same file:
stripImportedDefaultsrebuilds every rest-less tuple, whether or not anything beneath it changed. Zod spells "no rest element" asdef.rest === null, and thetuplearm compares that against theundefinedits owndef.rest ? ... : undefinedproduces, sonull === undefinedis false and the arm always takes the rebuild branch.Reproduced in two lines, and confirmed pre-existing by running it against the
origin/mainwalker:stripImportedDefaults(z.tuple([z.number(), z.number()]))is not identity, while a tuple with a rest element and a plain object both are. It is a different defect class from description loss, and repairing it moves the reference identity of published mirror bindings — its own contract-surface change. Filed as #9088; carved out of the pin here by shape, with the carve-out itself asserted non-empty so it cannot pass vacuously once that issue lands.Verification
pnpm exec vitest run packages/types/pnpm --filter @object-ui/types run type-checkpnpm exec eslinton both changed filesnode scripts/check-changeset-presence.mjsnode scripts/check-changeset-no-major.mjscheck:control-bytes,check:spec-symbols,check:new-line-citations,check:esm-specifiers,check:self-import,check:unreferenced-sources,check:comment-mask-corpusnode scripts/check-governed-queue-guard.mjs --testExit codes captured before any pipe.
Landing
⛔ Draft, carrying
needs:contract-reviewto match the card's carrier. Landing is the seat's — not flipped ready, not enqueued, no auto-merge.🤖 Generated with Claude Code
https://claude.ai/code/session_01Jmxdo7bmeqCQHLSfmLVX9w
Generated by Claude Code
Generated by Claude Code