fix(types): collapse the redundant UserActionsSchema extension, correct its docblock (objectui#8992) - #9017
Conversation
…rect its docblock (objectui#8992)
`objectql.zod.ts`'s `UserActionsSchema` carried
`stripImportedDefaults(Spec).extend({ group, hideFields, rowColor })`, an
extension that existed only because `@objectstack/spec` did not declare those
three keys while `normalizeListViewSchema` folded objectui's legacy `showGroup`
/ `showHideFields` / `showColor` onto them. The protocol adopted all three in
17.3.0, so the extension is now a second local copy of a protocol declaration
and it collapses into the plain by-reference re-export its own note said it
would become.
The urgent half is the docblock, which told its reader that
`UserActionsConfigSchema` "is NOT `.strict()`, so ... an author writing
`userActions: { group: false }` had it silently stripped — valid on parse, no
effect at render". Measured against the published artifacts of 17.0.0, 17.2.0,
17.3.0 and the resolved 17.4.0, every one of them refuses an undeclared key and
names it (`unrecognized_keys`, one issue). A comment promising silent tolerance
in front of a loud-rejection runtime points an author — human or AI — at a
config that will fail the save gate while telling them that outcome is
impossible.
The accept set does not move: extended and collapsed were parsed side by side
over a 33-document corpus with an identical result — same success, same parsed
output, same refusal codes, keys and messages — with a sentinel proving the
comparison can see a difference when one exists. The one published byte that
moves is the loss of three local `.describe()` strings the extension carried,
which the protocol declares those keys without.
`__tests__/user-actions-mirror-8992.test.ts` pins the refusal and the key-set
identity, with firing controls in both directions, so the corrected paragraph
cannot rot back. The `zod-mirror-parity.test.ts` exclusion survives — the TS
name is re-exported from the spec, not restated here — but its reason no longer
names an `.extend(…)` that is gone.
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
|
Contract reviewHead reviewed: Tier fuse, read not asserted. Instruments. Own blobless clone of the PR branch under my scratchpad, ① Derived judgments1. The false docblock — CONFIRMED; the method is sound. Re-run myself: 2. The equivalence proof — CONFIRMED; all three legs hold. Rebuilt side by side in one vitest process from the resolved spec: 3. The two moves on the published surface — the ruling covers the destination shape; two factual claims in the changeset are wrong; the
4. The EXCLUSIONS row — CONFIRMED. 5. CI — terminal and green on this head; the disposition is right, its wording is not.
② semver — MISMATCHThe changeset declares ③ Boundary flags
Out-of-scope finding of my own (a card, not a rider): IndependenceSELF-REVIEW. The implementer (transcript Required rework — confined to the changeset file and the PR body
No change is required to VERDICT: REWORK What I did not measure
Attribution in prose, per this repo's AGENTS.md mitigation: generated by Claude Code in session Generated by Claude Code |
… in it (objectui#8992) Contract review returned REWORK on the changeset prose and the declared bump. The code — the docblock, the `.extend()` collapse, the new pin and the parity row — was re-measured by the reviewer and found correct, and is untouched here. Only `.changeset/8992-user-actions-collapse-and-docblock.md` moves. 1. `patch` -> `minor`. Re-derived rather than taken on the reviewer's word, and every anchor found by content: `packages/types/CHANGELOG.md` records the IDENTICAL operation on this same file — `ListColumnSchema`'s local `.extend()` collapsing into a plain by-reference re-export — under `## 17.1.0` / `### Minor Changes`; `.changeset/8317-strip-imported-defaults.md`, the boundary ruling this change enrolls three more keys into, is `minor`; the sibling PR of this batch (objectui#8990) declares `minor`; and AGENTS.md states objectui's own breaking changes ship as `minor` with the break spelled out in the body. My own `tsc` probe, built from the two declarations side by side, confirms the break is real and producer-side: a value typed as the OLD output is not assignable to the NEW one (the three keys read as missing), while the reverse direction compiles. The probe is not vacuous — injecting a `@ts-expect-error` where nothing is wrong made the same run fail with TS2578, so the file was genuinely in the program. 2. ⛔ CORRECTING THIS COMMIT'S PARENT. The previous commit message, the changeset and the PR body all said the three keys lose their descriptions "which the protocol declares those keys without". THAT IS FALSE, and it is corrected here rather than by rewriting the parent (a force-push is not available to this branch). The protocol describes all three — `group` reads "Allow users to change record grouping from the toolbar. …" in the 17.3.0 and 17.4.0 tarballs. The real mechanism is objectui's own import boundary: `stripImportedDefaults` unwraps each `ZodDefault` with `.removeDefault()` and re-optionalises the inner node, and the description sits on the outer node it discards. Measured on this object: all ten defaulted keys read `description = undefined` after the strip, on both sides of the change, while `buttons` — the only member that never carried a default — keeps its description through it, which is the positive control. The OUTCOME sentence was right; the cause was invented. 3. "the OTHER EIGHT keys have declared that all along" -> SEVEN of the other eight. `buttons` is `z.ZodOptional[z.ZodArray[z.ZodString]]` and never declared a default. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jmxdo7bmeqCQHLSfmLVX9w
|
Reviewer's out-of-scope finding is filed as objectui#9034 — do not ride it into this PR. The contract review's closing line was
The repair round dispatched for this PR is confined to the changeset bump and the prose corrections (items 1–3 of the verdict). The code — docblock,
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
|
Contract reviewHead reviewed: Charter read at objectstack Tier fuse, read not asserted. Instruments. Own blobless clone of the PR branch at the head sha under my scratchpad, Round-1 rework, item by item
The repair commit touches exactly one file ( ① Derived judgments — every accept-set and public-surface change the diff produces1. Accept set of 2. Key set — UNCHANGED, and now derived from the protocol by reference; judged RIGHT. Eleven keys on both sides, equal to 3. Strictness — UNCHANGED ( 4. Published 5. 6. 7. New pin ② semver — CONSISTENTThe changeset declares ③ Boundary flags — every dev flag and
|
Provenance — carriers cleared, landingClause-② contract review: Reviewer ran at Independence — NOT self-review: Applied literally to the identities as Landing pre-checks — all measured just now
|
2 participants
Fixes #8992
Three items from the card, in its order of urgency, plus the neighbouring floor question it handed the taker.
1. The false docblock — measured, corrected, pinned
packages/types/src/zod/objectql.zod.tstold its reader:Measured false at every published 17.x. Not read from source — parsed, against each version's own published tarball (
npm pack, thenimport('./dist/ui/index.mjs'), no workspace resolution involved):{ oneUndeclaredKey: true }unrecognized_keys: ['group']Independent corroboration from a second instrument, in objectui's OWN published artifact: the emitted
packages/types/dist/zod/objectql.zod.d.tsdeclares this objectz.core.$strict— on both sides of this change. The compiler was already contradicting the comment.The comment is now replaced by what the runtime does, and
__tests__/user-actions-mirror-8992.test.tspins it so it cannot rot back.2. The collapse
stripImportedDefaults(Spec).extend({ group, hideFields, rowColor })→stripImportedDefaults(Spec).Premise re-derived rather than inherited: resolved
@objectstack/specis 17.4.0 (read off disk —node_modules/.pnpm/@objectstack+spec@17.4.0_.../package.json, exactly one copy installed — not off a declared range), and itsUserActionsConfigSchemadeclares 11 keys including all three.The accept set does not move. Extended and collapsed were built side by side in one process and parsed over a 33-document corpus — every declared key in both polarities, the full 11-key block, undeclared keys, objectui's legacy
showGroupspelling, wrong types,null/[]/'str':Identical on every document: same success, same parsed output, same refusal
code/keys/message. With a sentinel proving the comparison can see a difference when one exists (a.omit({group:true})variant diverges immediately), and a firing control proving the refusal is about the key and not a constant.3. The parity exemption
zod-mirror-parity.test.ts'sEXCLUSIONSentry survives — the exclusion criterion still holds, because the TS nameUserActionsConfigis re-exported FROM@objectstack/spec/uiby../index.tsrather than restated in this package, so there is still no second definition to drift from. What changed is its REASON, which named an.extend(…)that no longer exists.Both confined to those three keys, both measured by rebuilding
packages/types/diston each side:.describe()metadata. ⛔ Not because the protocol leaves them undescribed — an earlier revision of this body said that and it is measured FALSE: the 17.3.0 and 17.4.0 tarballs describe all three (groupreads "Allow users to change record grouping from the toolbar. …"). The cause is objectui's own import boundary:stripImportedDefaultsunwraps eachZodDefaultwith.removeDefault()and re-optionalises the inner node, and the description sits on the OUTER node it discards. Measured on this object, on both sides of this change: all ten defaulted keys readdescription = undefinedafter the strip, whilebuttons— the one member that never carried a default — keeps its description through it (positive control). So the three stop being an exception: the local extension had been supplying descriptions the other ten defaulted keys never had. Nothing in this repository reads them..d.tsthey movez.ZodOptional[z.ZodBoolean]→z.ZodDefault[z.ZodBoolean].z.inputis unchanged;z.outputfor these three goesboolean | undefined→boolean, i.e. the keys become REQUIRED on the output type. This is breaking in the producer direction, measured withtscover the two declarations built side by side: a value typed as the old output is NOT assignable to the new one (the three read as missing); the reverse compiles. The probe is not vacuous — injecting a@ts-expect-errorwhere nothing was wrong made the same run fail withTS2578, proving the file was in the program. Readers of parsed output are unaffected; code that CONSTRUCTS an output-typeduserActionsmust add the three keys or widen its annotation. That break is why the changeset isminor(this repo ships its own breaking changes asminorwith the break spelled out), matching the 17.1.0 precedent for the identicalListColumnSchemacollapse.⭐ (2) is the one a contract reviewer should look at. It is a normalisation of the DECLARATION — and, separately, a real producer-side break, which is what the
minorgrade is for.stripImportedDefaultsis deliberately typedTin /Tout — stripping is "a property of the PARSE, not of the declaration" (decision batch #90, objectui#8317) — so every key that arrives from the spec declaresZodDefaultwhile the runtime has the default removed. SEVEN of the other eight keys on this same object have declared exactly that all along —sort,search,filter,refresh,rowHeight,addRecordForm,editInline;buttonsisz.ZodOptional[z.ZodArray[z.ZodString]]and never declared a default. The extension was making three keys the odd ones out of an object whose eleven members behave identically at runtime. ⛔ Nothing about what parses changes: an omitted key is still absent from the parsed output, measured, on all eleven.Verification
Every command's exit code captured to a file before any pipe; each verdict quoted from the tool's own output. Heavy runs serialised through the container's shared verify lock (slot
os-dev-8992) — the wall-clock seconds below are shared-box readings, not idle-box ones.pnpm exec vitest run packages/types/Test Files 169 passed (169)·Tests 3350 passed (3350)pnpm --filter @object-ui/types type-checknode scripts/check-changeset-presence.mjs✅ 3 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)node scripts/check-control-bytes.mjs✅ OK (scanned 7210 tracked text file(s))node scripts/check-spec-symbol-derivation.mjs✅on all three of its checksnode scripts/check-new-cross-file-line-citations.mjsVERDICT … 0 new citation(s) … exit 0node scripts/check-unreferenced-sources.mjsOK Every shipped source file in every covered package is reachableNon-vacuity of the suite run: 168
.test.tsfiles are tracked underpackages/typesat the merge base and 169 are on disk, and vitest reported exactly 169 — so the new pin is inside the package run, not merely inside its own targeted one.Non-vacuity of the type-check:
tsc -p tsconfig.test.json --listFilesenumerates 635 files and contains all three changed/added files by name, sotype-checkgenuinely covered them.Ablation — the new pin is shown to FAIL, twice, in both of its directions
Each leg: mutate → prove the mutation reached disk by counting its marker in the source → run → restore → prove the restore by comparing
git hash-objectagainst theHEADblob and bygit diff HEADbeing empty. Restore points atHEAD(never a baregit checkout --), and the whole script carriestrap … EXIT INT TERMwith absolute paths.Tests 10 passed (10).loose()— recreate exactly the world the false comment described4 failed | 6 passed— all four refusal assertions redden.extend({ zzLocalWidening })— a local key the protocol does not declare1 failed | 9 passed— the key-set identity assertion reddensHEADblob67f26263…restored byte-identical after each leg.The dist marker, read both ways.
packages/types/distwas built fromHEADand deliberately NOT rebuilt during the two ablation legs: the mutation markers count 0 indistwhile the pin reddens, which is positive proof the run reads SOURCE and that a skipped rebuild cannot have measured a stale artifact here. (The rootvitest.config.mtsaliases@object-ui/types→packages/types/srcexplicitly, and the whole 169-file suite ran green before anydistexisted at all.) The other direction was measured separately, WITH rebuilds, in the type-surface run above: the marker moves0 → 2 → 0across build-at-HEAD → build-at-pre-fix → build-at-restored.CI on the first head (
b4e86e41), stated as measuredTerminal and green: 34 check runs — 31 success, 3 skipped (
Test (coverage),Test (coverage shard …),dependabot), 0 failure, 0 pending. All nine merge-queue required checks green,Type Checkamong them, which is the downstream type answer this PR declared to CI.It was not green on the first attempt, and the honest account of that is narrower than an earlier revision of this body's report claimed:
Test (shard 2/4)started 18:07:31Z, itsRun testsstep wentin_progressat 18:08:03Z and never ended, and the job was cancelled at 18:32:31Z — exactly 25m00s against a declaredtimeout-minutes: 20. ⛔ So "cancelled at the job bound" is not what the numbers say: the two do not match, this session issued no cancel, and only one CI run exists on the branch (so it was notcancel-in-progress). The cause is NOT MEASURED — the job log sits behind a blob host this container's proxy refuses.BaseSequencer.shard: sha1 of the root-relative path, sorted, thencalculateShardRange) over the 2924 collected specs on this head putszod-mirror-parity.test.ts— a file this PR changes — in shard 2, the shard that stalled; the new pin lands in shard 1. Control: the computation yields 731 specs per shard, matching the 731 files CI reported for the shard-2 re-run, and a non-test path resolves to no shard at all.@object-ui/corefloor — decided: NOT bumped here, and filed insteadThe card asked the taker to decide whether
@object-ui/core's declared'@objectstack/spec': '^17.2.0'should move. It should — but not in this PR. Filed with the measurement as objectui#9012.The finding is real and now measured rather than inferred: the range sits in
dependencies(consumer-facing), core's fold emitsuserActions.group/.hideFields/.rowColor, and against the real 17.2.0 artifact that exact output comes backREFUSED refused-keys=["group","hideFields","rowColor"]while 17.3.0 accepts it — same firing control refused by both, so the difference is about the three keys and not the harness.Why it is not a rider here:
@object-ui/types), and raising a declared floor narrows what consumers may resolve.check-spec-range-floors.mjsjudges symbol presence in the published artifact, andUserActionsConfigSchemais exported by both 17.2.0 and 17.3.0 (verified in each tarball'sdist/ui/index.d.ts); its workflow also only triggers on changes to the gate script itself. A bump landed quietly here would be prose-backed and could drift straight back.⛔ objectui#5435 stays open and is not addressed by this PR; objectui#2231 (the legacy-vocabulary migration) is out of scope, as the card directs.
验收备注
Observations found on the way, none filed — recorded here so the reviewing seat sees them rather than the next dev rediscovering them:
EXCLUSIONSrows carry the same stale reason this PR fixed.HttpMethodSchema,HttpRequestSchema,ViewDataSchema,ListColumnSchema,SelectionConfigSchemaandPaginationConfigSchemaare all plainstripImportedDefaults(Spec…)re-exports today, yet each is excluded "because the local.extend(…)adds renderer props".ListColumnSchemadid once carry an.extend(two commits touch that spelling); the other five never did — the reason is a template that was true of the family it was written for and is now false of six of its seven members. ⛔ Not fixed here, and the reason an earlier revision of this body gave was wrong: PR fix(types): ObjectKanbanSchema.groupBy is optional, as the protocol declares it (objectui#8990) #9021 (objectui#8990) does not touchzod-mirror-parity.test.ts— measured off its file list, it touchespackages/types/src/objectql.tsandpackages/types/src/zod/objectql.zod.ts, so the only file the two PRs share isobjectql.zod.ts, which is not where these rows live. The reason that does hold is scope: a seventh row's repair does not license rewriting six more under a different card's claim. The taker is the next collapse card inobjectql.zod.ts— it will read the same template.:607, not:604, and the declaration at:611, not:608. Re-derived rather than trusted, as the dispatch instructed. Everything else in the card held.UserActionsSchematurns out to have exactly one consumer,ListViewSchema.userActionsin the same file; it is not re-exported from the@object-ui/types/zodbarrel.Not measured
type-check(plugin-view,plugin-detail,app-shellreadListViewSchema). Deliberately declared to CI'sType Check, which is green on the farm. ⛔ An earlier revision of this body justified that with "assignment-safe in the direction consumers care about" — that over-claims: the movement is safe for READERS of parsed output and breaking for PRODUCERS of output-typed values (measured above). In-repo there is noz.output/z.inferover these schemas to break, which is why CI is green; external producers are not covered by any run here.@object-ui/core's — 26 declare^17.0.0. Named as a separate job in objectui#9012.🤖 Generated with Claude Code
https://claude.ai/code/session_01Jmxdo7bmeqCQHLSfmLVX9w
Generated by Claude Code
Generated by Claude Code