fix(types): declare onNavigate and onAddComment on the detail arm (objectui#7804, plugin-detail slice) - #9343
Draft
os-tesla wants to merge 4 commits into
Draft
fix(types): declare onNavigate and onAddComment on the detail arm (objectui#7804, plugin-detail slice)#9343os-tesla wants to merge 4 commits into
detail arm (objectui#7804, plugin-detail slice)#9343os-tesla wants to merge 4 commits into
Conversation
`BaseSchemaCore` ends `.passthrough()`, so a key the arm for `type: 'detail'`
does not declare is not refused - it stops being judged and the value is KEPT,
then reaches the renderer. `DetailView` is registered RAW for `'detail'`, so
both keys arrived by identity and RAN: `onNavigate` is called in the component's
own body (`handleBack` / `handleEdit` / the post-delete redirect), `onAddComment`
is forwarded as a prop into the comment composer that awaits it.
Measured on the unmodified arm before the code: an authored
`{ "action": "toast" }` parsed GREEN on both keys with the object surviving into
the parsed output, and the Back click then reported
`TypeError: schema.onNavigate is not a function`. `onBack`, already a named
refusal on the same arm, was refused on the same document and is the lit control.
Both are therefore RUNTIME SLOTS, measured per key rather than per prefix: a
named refusal on the JSON face, a callable twin on the TypeScript face. The two
`KNOWN_UNDECLARED_READS` rows they held are drained (39 -> 37).
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ
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
|
… uniformly `it.each([...DECLARED, ['onBack']])` widened the row type to `string[] | readonly [key: string]`, which vitest cannot narrow into a callback signature (TS2345 under `tsc -p tsconfig.test.json`). Both tables are now `as const` tuples, so the key parameter types as the literal union and the `.shape` index needs no cast. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ
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 was referenced Sep 13, 2026
…ed figures The sibling slice of objectui#7804 landed (the `plugin-kanban` arm, PR #9338), so both numbers this branch carried were derived against a base that has moved. Neither is re-stated by hand here: each is read back off the merged tree. `KnownDrift` header bullet — the one textual conflict. Both sides rewrote the head of the same bullet, so both one-sided resolutions were reproduced against the merged tree first and both are RED, which excludes picking a side: ours (40 entries / 63 keys) -> 2 failed: entry count 40, want 41 key total 63, want 65 theirs(41 entries / 63 keys) -> 1 failed: key total 63, want 65 The merged tree derives 41 entries / 65 keys (`ledgerEntryKeys` and `ledgerEntryMembers` over this file's own AST, via the two pins objectui#7733 and objectui#8222). The bullet now leads with those and narrates both slices in order, newest first: the `plugin-detail` keys onto an existing entry, then the `plugin-kanban` entry, then the objectui#8802 retirement below it. The restatement `N of the registered pairs carry TYPE drift TODAY` tracks the ENTRY count, not the key total. This slice adds two keys to an entry that already existed, so it does not move: main's 41 auto-merged and is correct. `KNOWN_UNDECLARED_READS` merged cleanly — the two slices drained disjoint rows. The gate of record now prints `35 exempted by ledger`, not the 37 this branch was written against. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ
… detail slice drained it A semantic collision the merge could not see: PR #9338's `suite 4` CONTROL names one surviving `KNOWN_UNDECLARED_READS` row to prove the leg reads a SHRINKING map rather than an empty one, and the row it named was `detail::DetailSchema.onNavigate` — exactly the row this branch drains. Both PRs were green alone; the control only reddens once both are in one tree. Re-derived against the merged ledger rather than weakened: the witness moves to `button::ButtonSchema.onSuccess`, one of the 35 rows that survive both slices and belongs to neither. The assertion keeps both halves it had — a non-empty map AND a named row — because the length check alone passes on a map holding a single stale row, which is the reading the control exists to refuse. The comment now records that the witness is re-derived on each landing, so the next slice of objectui#7804 to drain it knows to move it rather than drop it. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ
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
|
Collaborator
Author
Contract reviewHead judged: ⛔ First: this PR's seat falsified an instruction I put in its dispatch, and it was rightMy dispatch told it to lead the changeset with
⇒ The instruction is withdrawn, retracted to the two live seats carrying it, and removed from my dispatch template. What survives: if something breaking ships, say so in the body, because a reader needs it. ⛔ Not because a scanner does.
① Derived judgments
1 & 2 — same mechanism the sibling slice landed on: 3 — the gate of record's own printed line on the merged tree: 4 — both one-sided resolutions reproduced RED first, so picking a side was mechanically excluded before a number was written: 5 — this is the judgement I would most easily have got wrong. That restatement tracks the entry count, not the key total, and this slice adds two keys to an entry that already existed — so it should stay at ⭐ The third moved thing, which neither the dispatch nor I predictedPR objectui#9338's suite-4 control asserted The witness was re-derived to ② Semver
③ Boundary flags
Independence⛔ For this lane a clause-② review is default-tier self-review plus the gates, not an independence-qualified ruling. VerdictPASS on the work. ⛔ But this PR cannot be armed yet, and the reason is mine, not its seat's. Landing pre-check ③ is Both fixes now need the maintainer: objectui#9352 (skills guard, governed surface, needs an APPROVED review from Generated by Claude Code |
This was referenced Sep 13, 2026
Collaborator
Carrier provenance — the SPLIT carrier is healed; card objectui#7804 now carries the gate too⛔ Not a new gate and not a re-grading.
What is owed
⛔ Until then this PR is not armable, and ⛔ nothing here is a verdict on the code: no diff was reviewed to write this comment. Posted by the Generated by Claude Code |
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
3 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.
Part of objectui#7804 — the
plugin-detailslice. The parent card stays OPEN and stays the parent; PRs land per package (director seat, decision batch #69, 2026-09-07, maintainer verbatim 「其他同意」). ⛔ No closing keyword anywhere in this body.What lands
DetailSchema— the zod arm thattype: 'detail'selects — now declares the two handler keys its registered renderer reads, as objectui#6124 RUNTIME SLOTS on both faces: a named refusal on the JSON face, a callable twin on the TypeScript face.onNavigateruntime-slotDetailView's own body and CALLED there —handleBack,handleEdit, the post-delete redirectonAddCommentruntime-slotDetailViewnever calls it: it is FORWARDED as a React prop into the comment composer, whose submit handler awaits it, behind aschema.commentsgate the same passthrough keeps alive⭐ Per key, not per prefix. The two share a registration and a document and still reach the renderer by two different routes; the sibling slice on
object-kanbangot three different dispositions out of three keys sharing one prefix, so the per-key measurement is the deliverable, not a formality.The mechanism
BaseSchemaCoreends.passthrough()⇒ a key an arm does not declare is not refused, it stops being judged and the value is KEPT, then reaches the renderer.ComponentRegistry.register('detail', DetailView)registers the component RAW (unlike'detail-view', which goes through a data-source wrapper), so nothing is interposed and the authored value arrives by identity.Both keys were declared on NEITHER face:
BaseSchema'sany-valued index signature typed them on the TypeScript side and.passthrough()kept them on the zod side, while the renderer read and ran them.Base reading, written BEFORE the code and run on the unmodified tree (
a686403b3)Parse face, all three keys on the same arm:
Driven face, through the real
SchemaRenderer(happy-dom, React 19):Red-first run on that tree: 13 tests, 8 passed / 5 failed. The 5 failures are exactly the landing assertions (both members absent from the shape, both authored objects accepted, the derivation reporting both undeclared). The 8 that passed are the reachability legs, the hazard leg and the
onBackcontrol — i.e. the instrument was already working before the change moved anything.expect(...).toThrow()around the click. React 19 does not rethrow a handler error out of the dispatch — it REPORTS it and the click returns normally — so that spelling would have been a green assertion about a hazard that never fired. The leg now reads what React actually reports, and carries a silent-document control beside it. The miss is written into the test's own docblock.MERGED WITH
main— the sibling slice landed, and both derived figures movedobjectui#9338 (the
plugin-kanbanslice of the same card) merged, takingmainto5a41ce733.mainwas merged into this branch as a merge commit (8421c1f39) — no rebase, no amend, no force-push. Every figure below is re-read off the merged tree; none is stepped by hand.One textual conflict, in
zod-mirror-parity.test.ts: both slices rewrote the head of the sameKnownDriftbullet. Both one-sided resolutions were reproduced against the merged tree FIRST, so that picking a side is mechanically excluded rather than merely unattractive — both are RED:The derivation is the file's own AST instruments (
ledgerEntryKeys,ledgerEntryMembers) under the two pins objectui#7733 and objectui#8222 — not arithmetic on the figures in either branch. The bullet now leads with41 entries / 65 keysand narrates both slices newest-first: theplugin-detailkeys onto an existing entry, then theplugin-kanbannew entry, then the objectui#8802 retirement already below it.⭐ Three figures in that header are machine-read, not one. Besides the entry count, the KEY total is read by objectui#8222's block and the restatement
N of the registered pairs carry TYPE drift TODAYis read by objectui#7733's. The restatement tracks the entry count, not the key total — this slice adds two keys to an entry that already existed, so it does not move:main's41auto-merged and is correct untouched. That was verified, not assumed: in theoursrun above the restatement read 41 and PASSED while the entry count failed.suite 4CONTROL names one surviving ledger row to prove it reads a SHRINKING map rather than an empty one — and the row it named wasdetail::DetailSchema.onNavigate, exactly the row this branch drains. Both PRs were green alone; the control only reddens once both are in one tree. Re-derived, never weakened: the witness moves tobutton::ButtonSchema.onSuccess, one of the 35 rows surviving both slices and belonging to neither. The assertion keeps both halves it had — a non-empty map AND a named row — because the length check alone passes on a map holding a single stale row, which is the reading the control exists to refuse.After
Gate of record, its own printed line, on the merged tree:
The two
KNOWN_UNDECLARED_READSrows this slice owns are drained, enumerated not counted. The ledger walked 39 rows at this branch's merge-base (a686403b3) → 37 onmainnow (objectui#9338 drainedonCardClick+onQuickAdd) → 35 on the merged tree (this slice drainsonNavigate+onAddComment). ⛔ The other 35 are untouched. Earlier revisions of this body quoted39 → 37and37 exempted by ledger; those were correct against the old base and are superseded by the line above.Contract carrier — it applies, and this is the direction
The accept set moves on a published mirror: a document authoring either key was ACCEPTED and KEPT before and is now REFUSED BY NAME (issue
code: 'custom'at the key's own path). That is a move in the narrowing direction, which is still a move ⇒needs:contract-reviewon both limbs; the parent card already carries it and this PR carries it too. ⛔ Neither limb is cleared here — that is the PM's.minor, notmajor: the 40-packagefixedgroup makesmajorunavailable (scripts/check-changeset-no-major.mjs, run insidechangeset:check), so the breaking accept-set move ships asminorwith the reasoning in the changeset.Files
packages/types/src/zod/crud.zod.ts— the twohandlerKeyRefusal(..., 'runtime-slot', ...)arms, with the measurement beside each. Untouched by the merge (blob identical before and after), which is why the ablation below still cites it verbatim.packages/types/src/crud.ts— the callable TypeScript twins, signatures taken from the call sites (onNavigate(url, options),onAddComment(text)), matching whatviews.tsalready declares for the same component under its other registration.packages/types/src/__tests__/handler-keys-string-any-mirrors-7344.test.ts— the ledger that owns thecrud.zod.ts#DetailSchemapair: 2 rows added (8 sites to 10, 4 runtime slots to 6).packages/types/src/__tests__/zod-mirror-parity.test.ts— theKnownDriftentry grows to three keys; the header bullet is the merge's one conflict, resolved to the derived41 entries / 65 keys.packages/plugin-detail/src/__tests__/detail-handler-slots-7804.test.tsx— new; the driven measurement, the hazard, the refusal, and a derivation off the read site.packages/plugin-kanban/src/__tests__/handlerKeyDispositionsMeasured-7804.test.tsx— merge fallout only: thesuite 4control witness re-derived, per the collision described above.scripts/check-handler-key-read-sites.mjs— the two ledger rows removed.KeepsFunctionhelper answerstruefor an absent member, becauseBaseSchema's index signature types itanyand[any] extends [never]is false. The four keys already in that list came fromstring/any, so the helper could fail on them; on these two it could not. ADeclaresExactlyassertion (spelled withEqual, like the file's ownRetiredIsNever) was added beside it, with two synthetic controls proving it fails on an absent member and on a wrong signature.Verification
All readings at final head
980126de0(base5a41ce733), working tree clean at the time of each run.Tests —
pnpm exec vitest run packages/types/ packages/plugin-detail/ packages/plugin-kanban/ scripts/ examples/schema-catalog/, through the shared verify lock:examples/schema-catalog/is in the union deliberately: an authored key's acceptance moved, so a package-scoped run is not the blast radius (objectui#9273).packages/plugin-kanban/joined the union at the merge, because that is where the collision landed.Type-check — the dependent-set membership read was re-done on the MERGED tree rather than carried forward from the pre-merge round, and it reproduces: 37 workspace manifests name
@object-ui/types, and all 37 declare atype-checkscript (all spelledtype-check, hyphenated),@object-ui/siteamong them.pnpm --filter '...@object-ui/types' type-checkunder the lock resolves to 42 packages, 41 of which run atype-checktask — all 41 green, 0 errors,VERDICT command-exit 0.Build —
pnpm build --concurrency=2under the lock:Tasks: 43 successful, 43 total,VERDICT command-exit 0.Gate of record —
check:handler-key-readsexit 0, line quoted above (35 exempted by ledger).Other gates re-run on the merged tree, all exit 0:
check:control-bytes(7532 tracked text files, 0 findings) ·check:new-line-citations(0 new citations added by this branch) ·check:changeset-claims·check:spec-symbols·check:doc-types·check:test-path-roots·check:readme-exports·check:dist-completeness·check:unreferenced-sources·check:governed-queue-guard(NOT GOVERNED — 8 path(s) checked against 5 governed surface(s); none matched) · changeset presence (6 source file(s) of 3 released package(s) changed, and this change declares 1 changeset(s)) ·check:changeset-no-major.check:*families (56 exist in this repo) — CI owns the farm, and this seat runs the targeted set.check:sdui-registration-pins— exit 2, its own printed line isNo console build to weigh at apps/console/dist/assets ... This is exit 2, not a pass; it pins that registrations survive BUNDLING and this diff moves no registration. Remote CI convergence and the ready/queue flip — the PM's, per the dispatch.pnpm testin full — the repo-level scan, CI's run.pnpm --filter '...@object-ui/types' buildselects the DEPENDENTS of@object-ui/types, which excludes packages those dependents themselves depend on:@object-ui/react-runtime(depssucraseonly) and@object-ui/authname@object-ui/typesnowhere, so they were never built and@object-ui/components/@object-ui/app-shellcould not resolve their declarations. A fullpnpm buildis green at 43/43, which is the control proving the two failures were filter-closure gaps rather than findings.Ablation — the declaration is load-bearing, proved on disk
No
disthop is involved: the root vitest config aliases@object-ui/types/zodtopackages/types/src/zod/index.zod.ts, so the mutated file IS the module under test. The merge did not touchcrud.zod.ts— its blob is557c86b6ba292441f4f91e3170dac6964da3924dat the pre-merge head and at980126de0alike — so this leg stands as run.Leg run from the COMMITTED state, under
trap ... EXIT INT TERMwith absolute paths:All three mutated failures are NAMED on
onNavigate; everyonAddCommentleg stayed green — per key, not a blanket. And the gate of record turned red with its own printed line:Restored by
git checkout HEAD -- THE_PATH(spelled as a word: GitHub's body sanitizer eats angle-bracket-shaped fragments even inside code spans), and the restoration is proved by the blob hash returning and an EMPTYgit diff HEAD, never by an exit code.Acceptance notes
as anycast, so objectui#7804's "39" is a floor rather than a total. Measured with a firing control: 2 live cast-hidden handler reads in production sources,(schema as any).onTabChangeinDetailView.tsx(detail/detail-view) and(schema as any)?.onTabChangeincontainers.tsx(tabs); the control — the same anchor without theon-prefix — returns 33 files. ⛔ Not repaired here: it is a different card's scope, and this branch only RECORDS the DetailView one as a reading in its own test. Dedup ran over all 5 pages of the repo-scoped open-issue list (453 open issues) with objectui#7804 as the known-hit control; the three near neighbours (objectui#8327, objectui#8649, objectui#6152) are each shown non-overlapping in that card's body.scripts/or.github/workflows/scans for a leading**BREAKING**(the only two files mentioning the word use it in unrelated prose), and.changeset/config.jsonuses the stock@changesets/cli/changelog, which publishes bodies verbatim. Across 1489 pending changesets the all-caps**BREAKINGspelling appears in 31 and the title-case**Breakingspelling — this changeset's — in roughly 78; first-occurrence line numbers run from 5 to 51,main's own7804-object-kanban-handler-keys-judged.mdcarrying it at line 21. This changeset was therefore left as authored. Successor: whoever wants a real carrier convention has to build the gate first.check:changeset-claims(report-only) flags 16 pending changesets whose bodies namecrud.ts,crud.zod.tsorzod-mirror-parity.test.ts— files any@object-ui/typeschange touches. All 16 predate this branch and none is falsified by it; they describe their own landings, not current totals. Successor: the seat that next edits each of those changesets' own packages.commentsis not declared by this change.DetailViewgates theonAddCommentforward onschema.comments, kept by the same passthrough — but it is not a handler key, declaring it is an accept-set decision of its own, and objectui#7804's rows are the handler keys. Successor: whoever rules the'detail'arm's non-handler drift below.'detail'arm declares far fewer keys thanDetailViewreads —sections,fields,objectName,data,backUrl,editUrl,comments,activities,summaryFields,autoTabs, all kept by the passthrough. Not a handler-key question and outside every row of objectui#7804; the carrier for it is objectui#7804's own'detail'arm, which stays open as the parent.KeepsFunctiontype helper cannot fail on a member that was ABSENT rather than wrongly typed (an absent member readsany, and[any] extends [never]is false). Fixed in place for this pair with aDeclaresExactlyassertion and two synthetic controls; the class successor is whoever next adds a previously-UNDECLARED key to that ledger.Session that produced this change, written as prose so it survives a body edit:
https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZGenerated by Claude Code