Uh oh!
There was an error while loading. Please reload this page.
refactor(app-shell): type RecordDetailView's confirm handler as the published ConfirmationHandler - #6035
Merged
yinlianghui merged 1 commit intoAug 24, 2026
Conversation
…ublished ConfirmationHandler `RecordDetailView` runs its own confirm runtime rather than consuming `useConsoleActionRuntime`, and declared its handler with an inline re-spelling of the published type's shape. Adopt `ConfirmationHandler` instead, and pin that both runtimes hand `ActionConfirmDialog` the same field set. The inline shape and the published type are member-for-member and return-type identical, so the annotation is a structural no-op: the emitted JS is byte-identical. What the swap buys is that the compiler now keeps this declaration in step with the published type instead of leaving a near-duplicate to drift. `options` stays inert on this path and that is not a reason to narrow it: the runner calls a confirm handler with one argument, while `handleDeleteView` in `ObjectView.tsx` calls a `ConfirmationHandler` directly with all three fields localized. The parity pin asserts the state ARRIVING at `ActionConfirmDialog` from each runtime, through one shared double, rather than comparing the two declarations to each other. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CSoz9uGhaaSgiq3hshtN7L
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
|
yinlianghui
marked this pull request as ready for review
August 24, 2026 14:23
Uh oh!
There was an error while loading. Please reload this page.
yinlianghui
deleted the
claude/issue-5835-recorddetailview-confirm-handler-type
branch
August 24, 2026 14:35
This was referenced Aug 24, 2026
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes#5835
RecordDetailViewruns its own confirm runtime rather than consuminguseConsoleActionRuntime, and declared its handler with an inline re-spelling of the published type's shape. This adoptsConfirmationHandler, and pins that both runtimes handActionConfirmDialogthe same field set.Gates below were run on the branch head
004b68ed7.What changed
1. The published type replaces the inline shape —
packages/app-shell/src/views/RecordDetailView.tsx:2. A parity pin —
RecordDetailView.confirmRuntimeParity-5835.test.tsx, 4 tests.3. A changeset with empty frontmatter — this releases nothing, measured rather than asserted (below).
packages/app-shell/src/hooks/useConsoleActionRuntime.tsxis untouched.packages/app-shell/src/index.tsis untouched — this card needed no export change. Nothing underpackages/app-shell/src/views/metadata-admin/was read or written."Structurally identical" — verified, not inherited
Both shapes, side by side on the merge-base
53dc89db8:ConfirmationHandlermessage: stringmessage: stringoptions?: { title?: string; confirmText?: string; cancelText?: string }options?: { title?: string; confirmText?: string; cancelText?: string }Promise< boolean >Promise< boolean >(inferred from thenew Promise< boolean >the body constructs)Member for member, optionality for optionality, return type for return type: identical. The published type is not wider and not narrower, so adopting it is a structural no-op and there is no accept/reject behaviour to describe. Nothing here is a divergence to stop and report on.
The change is enforced at the type boundary only, and that boundary is silent
Reverse verification, direction predicted before running: restore the inline shape, expect
type-checkto stay GREEN, because the two shapes are structurally identical.Observed: GREEN —
REV_TYPECHECK_EXIT=0. Stating that plainly, as the honest reading: the parity pin carries the entire enforceable change. Adopting the published type buys future coupling — the compiler now keeps this declaration in step withConfirmationHandlerinstead of leaving a near-duplicate free to drift — but it moves no gate today.Under erasure a type-only change also cannot move a runtime test, so no runtime ablation was manufactured for it. Instead the claim was measured: transpiling
RecordDetailView.tsxbefore and after with the workspace esbuild gives byte-identical output, 64527 bytes both sides. That is also what makes the empty-frontmatter changeset the correct declaration rather than a convenient one.Mutation proved on disk both ways: the injected text (the inline
message: string, options?:spelling) grepped present at 1, the removed text (theuseCallback< ConfirmationHandler >spelling) grepped absent at 0. Restored undertrap ... EXIT INT TERM; the restored file is byte-identical to the pre-probe copy (diff -qclean).Deliverable 2: the pin asserts what ARRIVES at the dialog
ActionConfirmDialogis doubled once, for both importers — the hook imports../views/ActionConfirmDialog.js, the view imports./ActionConfirmDialog.js, one module, one double. Each runtime is then driven for real and the assertions run against thestateobject the dialog actually received. No test compares the two declarations to each other.Both arities are pinned, because the parameter is live on only one path:
packages/core/src/actions/ActionRunner.ts:await this.confirmHandler(this.evaluator.evaluate(action.confirmText) as string)— a single argument, confirming the card). This is the only wayRecordDetailView's handler is ever reached: it goes toActionProviderasonConfirmand nowhere else.handleDeleteViewinObjectView.tsxuses.Both runtimes supply the same field set on both arities:
message,open,options,resolve.Counter-probe: the pin can go red
Two perturbations of one runtime's field set (the
RecordDetailViewside; the hook was left untouched), each restored undertrap:optionsfrom the state literalexpected [ 'message', 'open', 'resolve' ] to deeply equal [ 'message', 'open', 'options', ...(1) ]expected [ Array(5) ] to deeply equal [ 'message', 'open', 'options', ...(1) ]The single survivor in both probes is the console-runtime test — the untouched side — which is the discriminating shape rather than a blanket failure.
Note that probe A leaves
type-checkgreen:ConfirmDialogState.optionsis optional, so a runtime that silently stops forwarding the bag is invisible to the compiler. That is precisely the gap the pin fills.The vacuous pass is closed explicitly: the expected field set is written out as a literal tied to what
ActionConfirmDialog.tsxgenuinely reads, so "both sides supply nothing" fails rather than passes.Mutation proved on disk for every leg, injected text and removed text grepped separately;
git diff HEAD --statcarries no probe residue afterwards, and theinjectedDrift5835marker greps absent.Gates, by name with exit codes — all at
004b68ed7pnpm --filter @object-ui/app-shell type-check(script name echoed:@object-ui/app-shell@17.6.0 type-check)TYPECHECK_EXIT=0pnpm exec vitest run packages/app-shell/src/views/RecordDetailView.confirmRuntimeParity-5835.test.tsxVITEST_EXIT=0— Test Files 1 passed (1), Tests 4 passed (4)pnpm exec eslint src/views/RecordDetailView.tsx src/views/RecordDetailView.confirmRuntimeParity-5835.test.tsx(plaineslint, no--no-inline-config)ESLINT_EXIT=0, warnings only, all pre-existing in kindnode scripts/check-changeset-presence.mjsMerge-base delta stated:
53dc89db8, two files changed plus the changeset. app-shell was run path-filtered — the whole-package suite was not run.Out of scope, reported not fixed
The typing alignment surfaced no divergence in the handlers. Measuring them did surface one next door, in the dialog's close path, which I filed rather than touched — the two runtimes reset
confirmStatedifferently on close. Filed as #6034 (finding, unassigned): the hook replaces the whole state and blanksmessage, the view spreads and flips one flag. Same drift family, out of this card's fence, and neither side is obviously the correct one.Generated by Claude Code