fix(react): UseNavigationOverlayOptions.onRowClick declares the modifier payload it is called with - #9360
Conversation
…ier payload it is called with
`useNavigationOverlay` declared the option with one parameter:
onRowClick?: (record: Record<string, unknown>) => void;
and `handleClick`, 126 lines below in the same file, asserted that declaration
away in order to call it with two:
(onRowClick as (r: Record<string, unknown>, e?: HandleClickModifiers) => void)(record, event);
The assertion was the only thing holding the two apart — a declaration that had
lost an argument, not a hook that needed one. The consequence is not a crash: it
is that the modifier payload is invisible on the one line a host reads, so a
host implementing Cmd/Ctrl/middle-click has to discover the second argument from
the implementation and then spell its own parameter optional to stay assignable.
`app-shell`'s `ObjectView` does exactly that at three call sites.
The declaration now names both parameters and the assertion is deleted.
`HandleClickModifiers` is declared and exported in this very file, so naming it
here costs no import and no dependency.
Source-compatible in BOTH directions, measured rather than assumed:
a one-parameter handler is assignable to the widened signature, and a handler
written against the widened signature was already assignable to the narrow one
(its minimum argument count is still one). No consumer changes.
⭐ Which is exactly why the pin is not an assignability assertion: both
spellings satisfy each other, so an `extends` pin is green on the broken tree
and on the repaired one alike. `useNavigationOverlay.onRowClickArity-9357.test.tsx`
uses the two instruments that can separate them — an exact-identity read of
`Parameters<...>` under `tsconfig.test.json`, and a bytes read of the
declaration and the call site off disk — each with controls proving it can fire.
Scope: the hook's own option only. The pass-through props on the view
components that feed it still declare one parameter on their own published
faces; which spelling that family converges on is the open question on
objectui#9357 and is not decided here.
Part of objectui#9357
Claude-Session: https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ
Co-authored-by: Claude <noreply@anthropic.com>
✅ 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 reviewReviewed head: Spelling note: generic parameter lists are written in words below ( ① Derived judgments — the accept set and the published faceThe understatement is real, and it is exactly one call site. At the PR head the hook contains one invocation of the option, An optional second parameter is the right spelling, measured on the feeders, not the docblock. Of the nine consumers that feed the option, five call The cast is gone, not survived. The published face names the real type, and that is correct here. The accept set — measured with a reviewer probe, both trees, under the lock. A probe file assigning nine handler shapes to
⇒ The widening narrows the accept set by exactly one class: a handler passed directly to the hook whose second parameter is typed narrower than ② Semver grading against the changeset
Owed edit, blocking: qualify that paragraph. A sufficient form: state that a handler passed directly to ③ Boundary flags and open questions
open_questions:
The code change is the right repair: the arity understatement is real, the cast is deleted rather than relocated, the option names the real payload type where it is nameable, and it agrees with objectui#9356 about the channel. The one defect is a published compatibility sentence that the compiler contradicts for a measured class, in text that ships to consumers and is not correctable after release. FAIL Generated by Claude Code |
ADOPTED — the FAIL at
|
…and pin it
The contract review of this PR verified the repair on every axis and failed it
on one published sentence: the changeset asserted "No caller has to change",
and changeset text ships verbatim into `packages/react/CHANGELOG.md`, where it
cannot be corrected after release.
Measured, that generalisation is false for exactly one class. A handler passed
DIRECTLY to `useNavigationOverlay` whose second parameter is annotated narrower
than `HandleClickModifiers` — React's `MouseEvent` being the shape a host that
discovered the payload from the implementation would write — compiled against
`origin/main` and is refused at this head:
error TS2322: Type '(_record: Record<string, unknown>, _ev?:
ReactMouseEvent) => void' is not assignable to type '(record:
Record<string, unknown>, event?: HandleClickModifiers | undefined) => void'.
… Type 'HandleClickModifiers' is missing the following properties from type
'MouseEvent<Element, MouseEvent>': altKey, buttons, clientX, clientY, and 26
more.
Reproduced in both directions before this edit, same tsconfig, same probe, the
hook file the only variable: refused against this head's declaration (blob
18ddada), accepted against `origin/main`'s (blob 00bb130), with a
one-parameter handler and an exactly-typed handler as controls accepted on both
trees.
Two changes, and nothing else. The changeset's compatibility passage now names
that class, gives the one-line remedy (annotate the parameter
`HandleClickModifiers`, or drop the annotation) and states — without asserting
a count this text cannot re-derive — that no caller inside this repository is
in it. And the pin file's compile-time half gains one `@ts-expect-error` row
holding the boundary, so it is measured rather than described: a directive
whose error stops occurring is itself TS2578, which reds if the option is ever
widened back or the payload respelled `any`.
The code is untouched — the hook, the deleted assertion and every existing pin
assertion stand exactly as reviewed.
Re card objectui#9357.
Claude-Session: https://claude.ai/code/session_01L5xpA5q533BgTTNADibEFt
Co-authored-by: Claude <noreply@anthropic.com>
Sync only, so the checks below run on the tree the merge queue will judge. No conflicts; nothing in the incoming range touches this branch's three files.
|
| Step | Outcome |
|---|---|
| Build packages | success |
| Check console performance budget | failure |
Which half objected:
| Eager-closure half | Verdict |
|---|---|
| Aggregate closure ceiling | ✅ pass |
| Per-chunk ceilings | ✅ pass |
| Ceiling sensitivity (headroom) | |
| Ceiling freshness (checkout vs. base branch) | ✅ pass |
⚠️ A broken gauge half is a verdict about the ceiling, not about the bundle: that line has drifted out of range of the regression it exists to catch, or the report behind it cannot be trusted. It does not say anything grew. TheCheck console performance budgetstep log carries the ceiling and the number it was compared against.
Reason: The entry chunk measured 144.3 KB, but the eager-closure half of this gate returned no trustworthy VERDICT: the report could not be read, a ceiling has drifted out of range of the regression it must catch, or (objectui#6245) a ceiling was replaced on the base branch after this checkout was made. The step log says which. This is not a passing budget — and it is not a size regression either.
See the workflow run for details.
📦 Bundle Size Report
| Package | Size | Gzipped |
|---|---|---|
| app-shell (consoleActionDispatch.js) | 0.20KB | 0.19KB |
| app-shell (index.js) | 16.69KB | 6.21KB |
| app-shell (runtime-config.js) | 20.68KB | 7.36KB |
| app-shell (types.js) | 0.01KB | 0.04KB |
| app-shell (urlParams.js) | 10.06KB | 3.86KB |
| auth (ActiveOrganizationStorage.js) | 25.05KB | 9.16KB |
| auth (AuthContext.js) | 0.31KB | 0.24KB |
| auth (AuthGuard.js) | 2.07KB | 1.00KB |
| auth (AuthProvider.js) | 40.18KB | 10.59KB |
| auth (AuthShell.js) | 3.49KB | 1.40KB |
| auth (ForgotPasswordForm.js) | 12.21KB | 3.45KB |
| auth (LoginForm.js) | 18.15KB | 5.39KB |
| auth (PreviewBanner.js) | 0.90KB | 0.50KB |
| auth (RegisterForm.js) | 6.65KB | 2.22KB |
| auth (SocialSignInButtons.js) | 9.61KB | 3.89KB |
| auth (UserMenu.js) | 3.41KB | 1.23KB |
| auth (auth-gate-events.js) | 1.29KB | 0.66KB |
| auth (authStyles.js) | 5.04KB | 1.72KB |
| auth (createAuthClient.js) | 40.21KB | 10.80KB |
| auth (createAuthenticatedFetch.js) | 8.46KB | 3.43KB |
| auth (index.js) | 3.19KB | 1.44KB |
| auth (invitation-status.js) | 1.22KB | 0.70KB |
| auth (org-roles.js) | 6.66KB | 2.78KB |
| auth (phone-identifier.js) | 1.11KB | 0.66KB |
| auth (types.js) | 0.59KB | 0.35KB |
| auth (useAuth.js) | 5.30KB | 1.02KB |
| auth (useWorkspaceAdminStatus.js) | 11.08KB | 4.58KB |
| collaboration (CommentThread.js) | 26.08KB | 7.56KB |
| collaboration (LiveCursors.js) | 3.17KB | 1.27KB |
| collaboration (PresenceAvatars.js) | 6.49KB | 2.64KB |
| collaboration (PresenceProvider.js) | 2.79KB | 1.13KB |
| collaboration (index.js) | 1.68KB | 0.73KB |
| collaboration (useCollaborationTranslation.js) | 6.05KB | 2.52KB |
| collaboration (useCommentSearch.js) | 1.98KB | 0.88KB |
| collaboration (useConflictResolution.js) | 7.75KB | 1.86KB |
| collaboration (useMentionNotifications.js) | 1.81KB | 0.68KB |
| collaboration (usePresence.js) | 6.33KB | 1.84KB |
| collaboration (useRealtimeSubscription.js) | 7.91KB | 2.01KB |
| components (index.js) | 502.02KB | 115.16KB |
| core (index.js) | 8.52KB | 3.41KB |
| create-plugin (index.js) | 27.94KB | 9.51KB |
| data-objectstack (index.js) | 211.58KB | 58.68KB |
| fields (index.js) | 247.89KB | 62.50KB |
| i18n (LocalizationContext.js) | 1.76KB | 0.96KB |
| i18n (builtinAggregateLabels.js) | 0.86KB | 0.49KB |
| i18n (currency.js) | 1.22KB | 0.64KB |
| i18n (fallbackInterpolation.js) | 6.25KB | 2.77KB |
| i18n (i18n.js) | 8.87KB | 3.64KB |
| i18n (index.js) | 5.22KB | 2.26KB |
| i18n (pickLocalized.js) | 9.86KB | 3.95KB |
| i18n (provider.js) | 32.15KB | 10.49KB |
| i18n (useDisplayLocale.js) | 2.85KB | 1.45KB |
| i18n (useObjectLabel.js) | 34.34KB | 9.17KB |
| i18n (useSafeTranslation.js) | 5.60KB | 2.33KB |
| layout (index.js) | 38.83KB | 10.95KB |
| mobile (MobileProvider.js) | 0.92KB | 0.49KB |
| mobile (ResponsiveContainer.js) | 0.94KB | 0.38KB |
| mobile (breakpoints.js) | 1.51KB | 0.70KB |
| mobile (createOfflineDataSource.js) | 5.61KB | 1.75KB |
| mobile (index.js) | 1.99KB | 0.87KB |
| mobile (offlineQueue.js) | 3.91KB | 1.35KB |
| mobile (pwa.js) | 0.97KB | 0.49KB |
| mobile (serviceWorker.js) | 1.48KB | 0.62KB |
| mobile (serviceWorkerSource.js) | 3.41KB | 1.48KB |
| mobile (useBreakpoint.js) | 1.54KB | 0.65KB |
| mobile (useGesture.js) | 6.96KB | 1.98KB |
| mobile (useOfflineSync.js) | 1.99KB | 0.72KB |
| mobile (usePullToRefresh.js) | 2.53KB | 0.85KB |
| mobile (useResponsive.js) | 0.72KB | 0.42KB |
| mobile (useSpecGesture.js) | 4.39KB | 1.66KB |
| mobile (useTouchTarget.js) | 1.01KB | 0.54KB |
| permissions (MePermissionsProvider.js) | 13.52KB | 4.88KB |
| permissions (PermissionContext.js) | 0.31KB | 0.25KB |
| permissions (PermissionGuard.js) | 0.89KB | 0.45KB |
| permissions (PermissionProvider.js) | 6.24KB | 2.16KB |
| permissions (discardProofCache.js) | 1.04KB | 0.55KB |
| permissions (evaluator.js) | 8.39KB | 3.10KB |
| permissions (index.js) | 0.93KB | 0.41KB |
| permissions (store.js) | 0.91KB | 0.42KB |
| permissions (useFieldPermissions.js) | 1.28KB | 0.53KB |
| permissions (usePermissions.js) | 4.83KB | 2.27KB |
| plugin-ai (index.js) | 14.81KB | 3.63KB |
| plugin-calendar (index.js) | 49.25KB | 13.99KB |
| plugin-charts (index.js) | 71.34KB | 19.90KB |
| plugin-chatbot (index.js) | 195.34KB | 46.51KB |
| plugin-dashboard (index.js) | 131.22KB | 34.59KB |
| plugin-designer (index.js) | 215.94KB | 44.33KB |
| plugin-detail (index.js) | 253.46KB | 65.85KB |
| plugin-editor (index.js) | 2.23KB | 1.05KB |
| plugin-form (index.js) | 136.77KB | 34.17KB |
| plugin-gantt (index.js) | 166.95KB | 41.04KB |
| plugin-grid (index.js) | 211.66KB | 57.50KB |
| plugin-kanban (index.js) | 46.00KB | 14.30KB |
| plugin-list (index.js) | 112.58KB | 27.65KB |
| plugin-map (index.js) | 20.64KB | 6.86KB |
| plugin-markdown (index.js) | 13.88KB | 4.80KB |
| plugin-report (index.js) | 43.41KB | 11.93KB |
| plugin-timeline (index.js) | 30.07KB | 8.74KB |
| plugin-tree (index.js) | 9.55KB | 3.32KB |
| plugin-view (index.js) | 84.42KB | 20.79KB |
| providers (DataSourceProvider.js) | 0.75KB | 0.39KB |
| providers (MetadataProvider.js) | 1.37KB | 0.59KB |
| providers (ThemeProvider.js) | 1.90KB | 0.85KB |
| providers (UploadProvider.js) | 11.66KB | 3.50KB |
| providers (index.js) | 0.45KB | 0.23KB |
| providers (types.js) | 0.01KB | 0.04KB |
| react-runtime (index.js) | 5.62KB | 2.34KB |
| react (LazyPluginLoader.js) | 4.47KB | 1.63KB |
| react (SchemaRenderer.js) | 96.00KB | 31.71KB |
| react (data-invalidation.js) | 5.05KB | 2.08KB |
| react (index.js) | 4.63KB | 2.18KB |
| react (schema-input.js) | 4.25KB | 2.04KB |
| react (spec-input.js) | 0.20KB | 0.18KB |
| sdui-parser (codegen.js) | 6.58KB | 2.74KB |
| sdui-parser (dashboard-widget-options.js) | 3.08KB | 1.30KB |
| sdui-parser (index.js) | 5.66KB | 2.50KB |
| sdui-parser (input-type.js) | 2.84KB | 1.40KB |
| sdui-parser (kanban-quick-add.js) | 3.89KB | 1.87KB |
| sdui-parser (parse.js) | 25.28KB | 7.80KB |
| sdui-parser (provenance.js) | 3.66KB | 1.82KB |
| sdui-parser (types.js) | 0.28KB | 0.23KB |
| sdui-parser (validate.js) | 14.82KB | 4.99KB |
| types (ai.js) | 0.20KB | 0.17KB |
| types (api-types.js) | 0.20KB | 0.18KB |
| types (app.js) | 2.87KB | 1.00KB |
| types (base.js) | 0.20KB | 0.18KB |
| types (blocks.js) | 0.20KB | 0.18KB |
| types (complex.js) | 2.93KB | 1.49KB |
| types (crud.js) | 0.20KB | 0.18KB |
| types (dashboard-filter-alias.js) | 6.23KB | 2.74KB |
| types (data-display.js) | 3.75KB | 1.85KB |
| types (data-protocol.js) | 0.20KB | 0.19KB |
| types (data.js) | 0.20KB | 0.18KB |
| types (designer.js) | 1.85KB | 0.85KB |
| types (disclosure.js) | 0.20KB | 0.18KB |
| types (error-code.js) | 1.54KB | 0.88KB |
| types (expression.js) | 0.20KB | 0.18KB |
| types (feedback.js) | 0.20KB | 0.18KB |
| types (field-types.js) | 0.20KB | 0.18KB |
| types (form.js) | 0.20KB | 0.18KB |
| types (http-inflight.js) | 8.87KB | 3.73KB |
| types (http-retry.js) | 4.32KB | 2.02KB |
| types (icon-key-migration.js) | 4.26KB | 1.63KB |
| types (index.js) | 4.74KB | 2.25KB |
| types (layout.js) | 0.20KB | 0.18KB |
| types (managed-by.js) | 0.19KB | 0.18KB |
| types (mobile.js) | 4.73KB | 2.28KB |
| types (navigation.js) | 0.20KB | 0.18KB |
| types (objectql.js) | 0.20KB | 0.18KB |
| types (overlay.js) | 0.20KB | 0.18KB |
| types (permissions.js) | 0.20KB | 0.18KB |
| types (plugin-scope.js) | 0.20KB | 0.18KB |
| types (record-components.js) | 0.20KB | 0.19KB |
| types (record-semantics.js) | 1.28KB | 0.67KB |
| types (registry.js) | 0.20KB | 0.18KB |
| types (reports.js) | 0.20KB | 0.18KB |
| types (select-option.js) | 0.20KB | 0.19KB |
| types (spec-report.js) | 5.05KB | 1.93KB |
| types (spec-ui-namespace.js) | 0.20KB | 0.19KB |
| types (strict-authoring-face.js) | 14.04KB | 5.36KB |
| types (system-fields.js) | 3.33KB | 1.54KB |
| types (theme.js) | 6.28KB | 2.87KB |
| types (ui-action.js) | 8.11KB | 3.32KB |
| types (views.js) | 0.20KB | 0.18KB |
| types (widget.js) | 0.20KB | 0.18KB |
Size Limits
- ✅ Core packages should be < 50KB gzipped
- ✅ Component packages should be < 100KB gzipped
⚠️ Plugin packages should be < 150KB gzipped
|
os-dev-report {
"issue": 9357,
"pr": "https://github.com/objectstack-ai/objectui/pull/9360",
"status": "done",
"branch": "claude/issue-9357-onrowclick-arity",
"new_head_sha": "e0c5c14b67f9b3ed4da2cc8362b6e19a94255aee",
"content_commit": "d8325fa79 (the one commit; e0c5c14b6 is the origin/main merge, no content of its own)",
"premise_still_valid": true,
"summary": "Exactly the three things ordered, in one commit, plus the ordered merge of origin/main (b67b53bc0) as a merge commit in the same push. (1) The changeset's unconditional \"Not breaking / No caller has to change\" passage is replaced by one that names the refused class, shows it as a code example, gives the one-line remedy, and states -- as measured and true -- that no caller INSIDE this repository is affected. (2) One @ts-expect-error row was added to the pin file's compile-time half, holding that boundary. (3) Nothing else: the hook, the deleted cast and every pre-existing pin assertion are byte-identical to the reviewed head (blob 18ddadad7fecd2e38341060c0bf1e0251e4ca1d5 for the hook, unchanged from 02b60d8ec). PR left draft; no ready flip, no auto-merge, no label touched, PR body not edited.",
"changeset_wording_landed": {
"file": ".changeset/9357-navigation-overlay-onrowclick-arity.md",
"frontmatter_unchanged": "'@object-ui/react': minor",
"replaced_paragraph": "**Not breaking, in either direction.** A one-parameter handler stays assignable to the widened signature (its extra parameter is optional), and a handler written against the widened signature was already assignable to the old one -- measured on this change, both directions. No caller has to change; what changes is that a caller who wants the modifier payload can now see, from the published type, that it is there.",
"landed_text_verbatim": "**Source-compatible in both directions, with one measured exception.** A\none-parameter handler stays assignable to the widened signature (its extra\nparameter is optional), and a handler written against the widened signature was\nalready assignable to the old one -- measured on this change, both directions.\n\n**The exception, and the one class that has to change.** A handler passed\n*directly* to `useNavigationOverlay` whose second parameter is annotated\n*narrower* than `HandleClickModifiers` no longer type-checks. React's\n`MouseEvent` is the shape this hits in practice, because until now the payload\nwas only discoverable from the implementation, so a host that wanted it wrote\nthe annotation it saw arrive:\n\n(a ts fence follows here in the file: a useNavigationOverlay call whose onRowClick is written `(record, ev?: React.MouseEvent) => { }`, with an inline comment reading \"was accepted; now TS2322 -- HandleClickModifiers is not assignable to React.MouseEvent\")\n\nIt compiled before only because the old declaration had no second parameter to\ncheck the annotation against. The parameter is checked contravariantly, so the\nannotation now has to *admit* `HandleClickModifiers`. **The fix is one line at\nthe call site:** annotate the parameter `HandleClickModifiers` (exported from\n`@object-ui/react`), or drop the annotation and let it be inferred. Either way\nthe handler keeps receiving exactly what it received before -- this is a\ntype-level change only, with no runtime behaviour attached.\n\nNo caller *inside this repository* is in that class, and the repository's own\ntype-check re-derives that on every run rather than this sentence asserting it;\na host that reaches the hook through a view component's `onRowClick` prop is\nunaffected either way, because the prop's own declared type is what gets\nassigned to the option. The boundary is pinned as a `@ts-expect-error` row in\nthis package's `useNavigationOverlay.onRowClickArity-9357` test, so it cannot\nmove without a red check.\n\nWhat the widening buys everyone else: a caller who wants the modifier payload\ncan now see, from the published type, that it is there.",
"note": "The count of in-repo feeders is deliberately NOT written down (AGENTS.md #9): the sentence points at the repository's own type-check as the instrument that re-derives it. The opening and closing paragraphs of the changeset, and the scope note, are unchanged."
},
"pin_row_landed": {
"file": "packages/react/src/hooks/__tests__/useNavigationOverlay.onRowClickArity-9357.test.tsx",
"half": "compile-time (the half tsconfig.test.json executes; vitest erases it)",
"lines_150_to_152_verbatim": [
"type NarrowerSecondParam = (record: RECORD, event?: ReactMouseEvent) => void;",
"// @ts-expect-error a second parameter narrower than `HandleClickModifiers` is refused (TS2322)",
"type _NarrowerSecondParamIsRefused = Expect of (NarrowerSecondParam extends OnRowClick ? true : false)"
],
"spelling_note": "RECORD stands for the record type (Record of string to unknown) and 'Expect of X' for Expect applied to X: GitHub's body sanitiser deletes angle-bracket-shaped spans, code fences included, so generics are written in words here. The file on disk carries the real angle brackets.",
"support_line": "One type-only import added at the top of the same file: import type { MouseEvent as ReactMouseEvent } from 'react'.",
"why_this_form_and_not_the_reviewer_s_assignment_verbatim": "The reviewer's probe was a value assignment (a const of type OnRowClick). Both forms were measured side by side in a throwaway probe before anything was edited, and BOTH fire at the head and BOTH red as TS2578 against origin/main's hook -- they exercise the same assignability relation. The type-level form was landed because the file's own top docblock and its section header both assert that this half is 'erased at runtime' / 'vitest erases every line of it'. A const declaration would have falsified those two published sentences, and repairing them would have been a third change this order does not authorise. The landed form is also the idiom its immediate neighbours (_NarrowIsAssignableToWide, _WideIsAssignableToNarrow) already use. The TS2322 diagnostic the reviewer measured is quoted verbatim in the row's docblock, so the host-visible error is on the record."
},
"reproduction_of_the_refusal_both_directions": {
"method": "One probe file under packages/react/src/hooks/__tests__/ plus a tsconfig extending packages/react/tsconfig.test.json and naming only that file, so the reading is not mixed with the existing pin's output. Same tsconfig on both legs; the hook file was the only variable, swapped by commit sha (never by a moving ref name). Dependency dist was built first (tsconfig.test.json sets paths to empty, so the @object-ui packages resolve through dist); the hook itself is a RELATIVE import from the test file, so no rebuild is needed between legs. Both probe files were deleted afterwards and git status is clean.",
"control_that_the_two_trees_differ": "hook blob at PR head 02b60d8ec = 18ddadad7fecd2e38341060c0bf1e0251e4ca1d5; at the PR base 2e471dc0a AND at origin/main b67b53bc0 = 00bb1307c972dbb27a64476b6594ae2e2e651cd7 (byte-identical base and main, so 'compiles on main' and 'compiled on the base' are the same reading). packages/react/tsconfig.test.json is byte-identical across all three (aa43dc225). On the mutated tree the one-parameter spelling reads 1 occurrence and the assertion 'onRowClick as' reads 1; at the head they read 0.",
"leg_head_refused": {
"tsc_exit": 2,
"diagnostic_verbatim_generics_in_words": "packages/react/src/hooks/__tests__/probe-9357.ts(7,14): error TS2322: Type '(_record: RECORD, _ev?: ReactMouseEvent) => void' is not assignable to type '(record: RECORD, event?: HandleClickModifiers | undefined) => void'.\n Types of parameters '_ev' and 'event' are incompatible.\n Type 'HandleClickModifiers | undefined' is not assignable to type 'MouseEvent of Element | undefined'.\n Type 'HandleClickModifiers' is missing the following properties from type 'MouseEvent of Element': altKey, buttons, clientX, clientY, and 26 more.",
"note": "Matches the reviewer's quoted diagnostic exactly, apart from the probe's own identifier names."
},
"leg_main_accepted": {
"tsc_exit": 0,
"output": "(empty -- the same narrower-annotation handler compiles against origin/main's hook)"
},
"controls_on_both_legs": "A one-parameter handler and a handler annotated exactly HandleClickModifiers were in the same probe file and were ACCEPTED on both trees, so the head leg's single error is the narrowing and not a broken probe.",
"restore_proof": "Mutation and restore under a trap on EXIT INT TERM with absolute paths; restore is 'git checkout HEAD -- PATH' (never a bare checkout), verified by blob-hash equality against the HEAD blob AND an empty 'git diff HEAD', never by an exit code. Both legs restored to 18ddadad7fecd2e38341060c0bf1e0251e4ca1d5."
},
"ablation_of_the_landed_pin_row": {
"run_from": "the committed merged tree, HEAD = e0c5c14b6, so the restore leg has a real restore point",
"command": "pnpm exec tsc -p packages/react/tsconfig.test.json",
"baseline_exit": 0,
"mutation": "git checkout b67b53bc0 -- packages/react/src/hooks/useNavigationOverlay.ts (origin/main's one-parameter declaration)",
"on_disk_proof_before_reading_anything": "blob on disk after the swap = 00bb1307c972dbb27a64476b6594ae2e2e651cd7; injected one-parameter spelling counted with 'grep -o ... | wc -l' = 1; deleted two-parameter spelling = 0. (grep -c counts LINES, so occurrences were counted with grep -o piped to wc -l.)",
"ablated_exit": 2,
"ablated_errors": [
"...onRowClickArity-9357.test.tsx(89,31): error TS2344: Type 'false' does not satisfy the constraint 'true'.",
"...onRowClickArity-9357.test.tsx(92,3): error TS2344: Type 'false' does not satisfy the constraint 'true'.",
"...onRowClickArity-9357.test.tsx(92,26): error TS2493: Tuple type '[record: RECORD]' of length '1' has no element at index '1'.",
"...onRowClickArity-9357.test.tsx(101,3): error TS2344: Type 'false' does not satisfy the constraint 'true'.",
"...onRowClickArity-9357.test.tsx(117,43): error TS2344: Type 'false' does not satisfy the constraint 'true'.",
"...onRowClickArity-9357.test.tsx(151,1): error TS2578: Unused '@ts-expect-error' directive. [THE NEW ROW]"
],
"reading": "The first five are the pre-existing pin's red-first errors, reproduced at +1 line from the reviewer's control leg (88/91/91/100/116 became 89/92/92/101/117) because one import line was added above them -- an independent control that the existing assertions were not disturbed. The sixth, at line 151 column 1, is the new directive and is the ablation result: the row fires when the boundary exists and reds loudly when it does not.",
"restore": "blob back to 18ddadad7...; 'git diff HEAD' empty; restored reading exit 0."
},
"checks": {
"all_run_on": "the merged tree e0c5c14b6 unless marked (pre-merge)",
"locked_runs_verdict_lines_quoted": [
"VERDICT command-exit 0 - held the lock 32s - waited 0s => (pre-merge) pnpm --workspace-concurrency=2 --filter '@object-ui/react^...' build",
"VERDICT command-exit 0 - held the lock 15s - waited 0s => (pre-merge, post-edit) pnpm --filter @object-ui/react type-check (tsc --noEmit && tsc -p tsconfig.test.json)",
"VERDICT command-exit 0 - held the lock 385s (6m25s) - waited 0s => pnpm exec turbo run build --concurrency=2 : Tasks: 44 successful, 44 total",
"VERDICT command-exit 0 - held the lock 371s (6m11s) - waited 0s => pnpm exec turbo run type-check --concurrency=2 : Tasks: 81 successful, 81 total (this is the run that executes packages/react's tsconfig.test.json, i.e. the pin)",
"VERDICT command-exit 0 - held the lock 54s - waited 0s => pnpm exec vitest run packages/react/ : Test Files 84 passed (84), Tests 995 passed (995)",
"VERDICT command-exit 0 - held the lock 1088s (18m08s) - waited 0s => pnpm test --shard=1/4 : Test Files 770 passed | 1 skipped (771), Tests 10455 passed | 2 skipped (10457)",
"VERDICT queue-timeout (exit 99) - never acquired - waited 540s (9m00s) => pnpm test --shard=2/4 : NOT MEASURED",
"VERDICT queue-timeout (exit 99) - never acquired - waited 540s (9m00s) - holder pid 12379, held 263s (a run in objectui-review-9343-merged) => vitest run scripts/ packages/types/ packages/cli/, attempt 1 : NOT MEASURED",
"VERDICT queue-timeout (exit 99) - never acquired - waited 540s (9m00s) - holder pid 12379, held 893s (same holder) => same command, attempt 2 on the resumed slot : NOT MEASURED"
],
"lock_slot": "OS_VERIFY_LOCK_SLOT=objectui-9360, set before the first attempt and resumed on every retry.",
"unlocked_runs_exit_captured_by_redirect_before_any_pipe": [
"pnpm run type-check:scripts exit 0 (the leg the dispatch warns is skipped because scripts/ is not a workspace package)",
"pnpm run type-check:vitest-config exit 0",
"pnpm run type-check:vitest-setup exit 0",
"pnpm run type-check:e2e exit 0",
"pnpm exec turbo run lint --concurrency=2 exit 0 : Tasks: 47 successful, 47 total; 0 errors in every package (warnings only, all pre-existing)",
"pnpm --filter @object-ui/react lint exit 0 : 356 problems (0 errors, 356 warnings)",
"pnpm exec eslint THE-EDITED-TEST-FILE exit 0, no output",
"node scripts/check-changeset-presence.mjs exit 0",
"node scripts/check-changeset-no-major.mjs exit 0",
"node scripts/check-changeset-fixed.mjs exit 0",
"node scripts/check-changeset-overwrite.mjs exit 0",
"node scripts/check-changeset-claims.mjs exit 0",
"node scripts/check-control-bytes.mjs exit 0",
"node scripts/check-comment-mask-corpus.mjs exit 0",
"pnpm run check:new-line-citations exit 0 : 'VERDICT new-cross-file-line-citations: 0 new citation(s), enforcement report-only -> exit 0'",
"node scripts/check-test-path-roots.mjs exit 0",
"node scripts/check-type-check-coverage.mjs exit 0",
"pnpm run check:doc-snippets exit 0 : 'Semantic phase: 649 of 649 block(s) judged, 0 failed.'",
"pnpm run check:skill-examples exit 0 : 'Semantic phase: 14 of 14 ts fence(s) judged, 0 failed.'",
"node scripts/check-governed-queue-guard.mjs --test THE-TWO-CHANGED-PATHS exit 0 : 'NOT GOVERNED -- 2 path(s) checked against 5 governed surface(s); none matched.'",
"pnpm exec vitest run THE-27-CHANGESET-READING-TEST-FILES exit 0 : Test Files 27 passed (27), Tests 960 passed (960)",
"direct control-byte scan over both changed files with grep -naP over the C0/DEL class: 0 hits"
],
"two_previously_red_checks_are_green_on_the_merged_tree": "Doc Snippet Type Check and Skill Example Check were red on 02b60d8ec and the reviewer judged them stale/inherited from main. Both gates now exit 0 locally on the merged tree (figures above), so the merge is expected to clear them on CI as well.",
"narrowings_declared": [
"pnpm test shards 2/4, 3/4 and 4/4 were NOT MEASURED. Shard 2/4 returned exit 99; shard 3/4 sat in the queue 683s behind another seat's run and I stopped it (by recorded pid, never by process name) rather than starve two other queued seats further; shard 4/4 never started. Substituted, and the substitution is argued from reach rather than assumed: my diff is two files. (a) The test file lives in packages/react and nothing imports it -- 'git grep -l onRowClickArity-9357' over every test file in the tree returns 0 hits outside itself -- and the WHOLE packages/react suite ran green (84 files / 995 tests). (b) The changeset file -- 'git grep -l 9357-navigation-overlay' over the same corpus returns 0 hits, so nothing names it; the tests that read the .changeset DIRECTORY at all number 27, and all 27 ran green. On top of that, shard 1/4 of the full suite ran green (771 files), and the FULL build farm (44/44), the FULL type-check farm (81/81) and the FULL lint farm (47/47) all ran green on the merged tree. CI still runs all four shards.",
"The e2e / live-e2e and performance-budget workflows were not run locally at all; they are CI's."
]
},
"measured_false_in_this_order_or_in_my_own_method": [
"Nothing in the order was measured false. The dispatch's compiler fact reproduced exactly, in both directions, with the diagnostic matching the reviewer's record; the blast radius inside the tree is nil as stated; packages/react's type-check really is 'tsc --noEmit && tsc -p tsconfig.test.json' and the second leg really is the one that reads the pin.",
"One refinement, not a contradiction: the dispatch says tsconfig.test.json resolves the @object-ui packages through dist (paths set to empty), so build what it needs first. True, and I did build the dependency closure first -- but the hook under test is reached by a RELATIVE import from the test file, so the two reproduction legs did not need a rebuild between them. Both facts matter: without the dist build the project does not compile at all; with it, swapping the hook source is immediately visible.",
"A method attempt of MINE that measured false: 'pnpm exec vitest list --filesOnly --shard=N/4' IGNORES --shard -- all four values printed the identical 3084-line list. So I could not determine which shard holds the pin file, and I do not claim it. (--shard does work on 'vitest run': shard 1/4 ran 771 of 3084 files.) Reported because I nearly used that output as evidence of coverage.",
"Template conflict, reported rather than resolved: this session's injected attribution template asks for a commit trailer naming a model. The order forbids a model identifier in any repository artifact, and so does my standing contract. I followed the order and the branch's existing convention -- 'Co-authored-by: Claude' with the noreply address, plus 'Claude-Session:' -- and flag the conflict here instead of resolving it with a model name."
],
"mcp_calls": "0 -- every GitHub read and this write went through repo-scoped REST (probed first: GET on the repo returned 200); no MCP GitHub tool was called.",
"open_questions": [],
"out_of_scope_findings": [
"noted, not filed: the PR BODY of #9360 still carries the same unconditional generalisation the changeset carried -- its 'Source compatibility' section ends 'No consumer breaks in either direction, which is why no consumer is edited here.' The order is 'exactly three things ... nothing else', and a body edit is also the one operation GitHub is known to mutate (it appends a second attribution footer and downgrades the session-URL form), so I did not touch it. Successor: the light re-review named in the adoption comment, or the PM, can decide whether the body should be qualified too. Flagged here because the body is what the re-reviewer reads.",
"noted, not filed: nothing else was found. The hook, the deleted cast and the existing pin assertions were read and left byte-identical, as ordered."
],
"landing": "Untouched by me, as ordered: PR #9360 is still draft, auto-merge is null, labels are still exactly 'package: react' and 'tests'. Landing is the PM's."
}Generated by Claude Code |
|
Contract reviewReviewed head: ① derived judgments
Diff read against the merge-base Public-surface census, taken on the BUILT declarations rather than the source.
② semver gradingChangeset file: What the diff actually is: a breaking change to a published type. Measurement (2) is a source-level refusal of code that compiled against the previous release — semver-major in substance. It is nonetheless correctly declared The rule's condition is that the breaking semantics be written out in the body. The changeset body now does exactly that — it names the refused class, quotes the TS2322, gives the one-line remedy, and no longer generalises. ⇒ the declared level is right, and the changeset body now earns it. The grading defect is not the level: it is that the compensating prose this rule demands was written on the artifact with one reader at release time, while the artifact with every reader forever still carries the sentence it was written to replace (①.4). ③ boundary flagsThe earlier FAIL (
⇒ and item 3 is why this PR still fails. That review located the defect as "a CHANGELOG line the compiler refutes" and scoped its remedy to the changeset; it did not read the emitted Implementer flags and
Clause-② declaration. The PR body carries no Inherited-red paragraph — STALE at this head, in both directions. It names Doc Snippet Type Check and Skill Example Check; at Also stale, and not verdict-bearing. The PR body has not been updated for Implemented-by: claude/issue-9357-onrowclick-arity (mode:subagent) FAIL Generated by Claude Code |
…pped JSDoc
Comments are not stripped for this package, so the `onRowClick` docblock lands
verbatim in `dist/hooks/useNavigationOverlay.d.ts` — the hover text every
consumer of `@object-ui/react` reads. Its closing sentence still said a
one-parameter handler stays assignable "so nothing a caller already wrote has
to change", which the widening refutes for one class: a handler whose second
parameter is annotated narrower than `HandleClickModifiers` is now refused with
TS2322. The changeset two files away already names that class in bold, so the
release would have published a CHANGELOG and a `.d.ts` that disagree about the
same line.
Replaced with the qualified form: what stays assignable, the one class that has
to change, and the one-line remedy. Two residual restatements of the same
generalisation in the pin test's own prose ("nothing breaks either way", "no
consumer is broken by the widening") are corrected the same way; both sit in a
file this PR already adds and both are contradicted 80 lines below by that
file's own ACCEPT-SET BOUNDARY block.
Comment text only. Re-measured at this head: the accept set is unchanged (a
second parameter annotated React.MouseEvent is accepted at the merge-base
b67b53b and refused here with TS2322, while one-parameter and unannotated
handlers are accepted on both trees), the emitted declaration corpus is still
65 files with 295 exported declaration names and an empty symmetric difference,
and `dist/index.d.ts` is byte-identical.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L5xpA5q533BgTTNADibEFt
✅ 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 reviewReviewed head:
How this head was read. Diff taken against the fork point ① derived judgmentsPublic-surface census, on the BUILT artefacts. All three trees emit 65 1.
⇒ the line relaxes the option for anyone who wants the payload (control B could not even be 2. The 25-line JSDoc block on that member — a published surface, because it ships. It is 15 of
The lit controls are what make that 3. The type assertion at the call site is deleted; 4. The pin 5. Nothing else in this diff touches an accept set or a public surface. ② semver gradingChangeset file What the diff actually is: breaking. ①.1's SUBJECT and control C are source-level refusals of That rule's condition is that the breaking semantics be written out in the body. The changeset ③ boundary flagsThe FAIL of record (
Found by me, measured, and below the bar — none of these blocks landing.
Implemented-by: claude/issue-9357-onrowclick-arity (mode:subagent) PASS Generated by Claude Code |
ADOPTED — the PASS at
|
| check | reading | |
|---|---|---|
| ① same-form PASS at tier | ✅ | record 5657155742, head 0d983a987acb2cbcf51b3e4561d7923eadaf2d61, ## Contract review heading · sha in a code span · ①②③ itemized · independence pair · exactly one unhedged **PASS**, zero **FAIL**. Transcript tier read back claude-opus-5 on all 110 harness-stamped messages, no fallback. |
| ② both carriers, machine-read | ✅ | PM_SWEEP_REPO=objectstack-ai/objectui node scripts/pm/check-clause2-carriers.mjs --pair 9360 → exit 0, run live just now: "the clause-② declaration is readable in the fixed spelling and both carriers agree". |
| ③ every check | ✅ | 36 check runs, all complete: 33 success / 3 skipped / 0 failure / 0 cancelled / 0 timed_out. Bundle Analysis is green at this head, so the body's inherited-red section is moot rather than merely stale — the base fix reached this PR through the repair push, which is the only way it can. |
Independence pair. Implemented-by: claude/issue-9357-onrowclick-arity (mode:subagent) · Reviewed-by: session_01L5xpA5q533BgTTNADibEFt. Both reviews in this PR's history ran as freshly spawned isolated subagents fed only the card, the rulings it references and the PR itself — ⛔ never the dispatch order or this seat's conclusions. Under the maintainer's ruling the pair is judged by independent measurement, ⛔ not by Claude-Session trailer identity.
Two rounds, and the second one earned its cost. The first review FAILed at e0c5c14b67 on a JSDoc paragraph that shipped a refuted generalisation into dist/hooks/useNavigationOverlay.d.ts. The repair discharged it — measured on the emitted file, not the source: the old sentence 1 → 0, the qualified replacement 0 → 1, TS2322 / MouseEvent / contravariantly 0 → 1, against four controls lit in that same file (HandleClickModifiers 4 → 7, objectui#9357 1/1, Cmd/Ctrl 1/1, UseNavigationOverlayOptions 2/2). It also found two further restatements the first census missed, in a file this PR itself adds.
Invariance measured on the artefact rather than argued: 65 .d.ts + 65 .js at every ref with exactly one of each moving fork-point → head; exported-declaration census 297 = 297, symmetric difference empty, against a poison-sentinel control that does compare unequal; dist/index.d.ts byte-identical; and the emitted hooks/useNavigationOverlay.js identical once comments are stripped, with the raw bytes differing as the control. ⭐ Worth carrying: comments are kept in the emitted .js too, so a call-site comment ships there as well — the .d.ts is not the only published face of a comment.
⭐ Six accept-set probes across two trees, each hash-proved to disk and removed under trap, with a POISON leg that must fail so an "accepted" reading cannot be confused with a probe the compiler never opened. The reviewer added its own control C — an unrelated annotation (ev?: string) — and it is refused too.
Recorded, ⛔ not blocking, owed to whoever next touches this surface
- The shipped sentence calls the refused class "annotated NARROWER than
HandleClickModifiers"; control C shows an unrelated annotation is refused as well. Saved by the next sentence, which states the exact rule ("must ADMITHandleClickModifiers") and a remedy correct for the whole class. A successor could tighten one word. - The refusal is conditional on
strictFunctionTypesand is stated unconditionally. Measured with one probe and one flag:false→ exit 0,true→ exit 2. ⇒ it over-warns a non-strict consumer and never under-warns, which is the safe direction. AGENTS.md's version-alignment prose says the fixed group is 39 packages;.changeset/config.jsonsays 40. Pre-existing, unowned, governed surface — already on this seat's list for the maintainer.
Carriers cleared from both this PR and card objectui#9357 in one pass. Landing next: ready → auto-merge → merge queue. ⛔ Card objectui#9357 stays open and keeps its state — this PR carries no closing keyword, by design.
Generated by Claude Code
|
I wrote: "comments are kept in the emitted Measured on ⇒ whether a source comment reaches the emitted Generated by Claude Code |
Part of objectui#9357
The defect, re-verified against
origin/mainbefore any editpackages/react/src/hooks/useNavigationOverlay.ts, both halves byte-for-byte, at2e471dc0aa23ff807aa9960618c419ecccdfdcef:premise_still_valid: true— all four lines are exactly where the card says,126 lines apart in one file. The declaration promises one parameter; the call
site asserts the declaration away to pass two. The assertion is the only thing
holding the two apart.
One correction to the dispatch note:
HandleClickModifiersis not importedinto this file — it is declared and exported from this very file, fourteen
lines below the option. Naming it in the option therefore costs no import at
all, which is a stronger version of the same point.
The repair
UseNavigationOverlayOptions.onRowClicknow declares both parameters:the record, and
event?: HandleClickModifiers.handleClicknow callsonRowClick(record, event)straight through the declaration.repair and neither touches an executable line — see the next section.
What the follow-up commits changed
02b60d8ecis the repair above. Both commits after it are prose and pin only:d8325fa79— after the first contract review, the changeset stoppedgeneralising. It now names the one refused class, quotes the
TS2322thatclass raises, gives the one-line remedy, and the pin grew a
@ts-expect-errorrow that goes red (asTS2578-unused) if that boundaryever moves. The class: a handler whose second parameter is annotated
narrower than
HandleClickModifiers— React'sMouseEventin practice.0d983a987— the second contract review found the same refutedgeneralisation still standing in the hook's JSDoc, which is the carrier that
actually ships. Comments are not stripped for this package, so that block
lands verbatim in
dist/hooks/useNavigationOverlay.d.ts— the hover textevery consumer of
@object-ui/reactreads. The sentence "a one-parameterhandler stays assignable here, so nothing a caller already wrote has to
change" is replaced by the qualified form the changeset already uses, so the
CHANGELOG and the declaration now agree about the same line. Two residual
restatements of the same claim in the pin's own prose ("nothing breaks either
way", "no consumer is broken by the widening") are corrected the same way —
both sit in a file this branch adds, and both were already contradicted 80
lines below by that file's own ACCEPT-SET BOUNDARY block. Comment text only.
Consumers, enumerated — and which ones the repair reaches
Thirteen sites call
useNavigationOverlay; nine of them feed the repairedoption. The repair reaches the option itself, so every one of these nine may
now pass a two-parameter handler without an assertion of its own:
packages/plugin-grid/src/ObjectGrid.tsxonRowClickpackages/plugin-list/src/ListView.tsxonRowClickpackages/plugin-list/src/ObjectGallery.tsxprops.onRowClickelseprops.onCardClickpackages/plugin-kanban/src/ObjectKanban.tsxexternalClickpackages/plugin-calendar/src/ObjectCalendar.tsxonRowClickwhen not an overlaypackages/plugin-gantt/src/ObjectGantt.tsxonRowClickwhen not an overlaypackages/plugin-map/src/ObjectMap.tsxonRowClickpackages/plugin-timeline/src/ObjectTimeline.tsxonRowClickelseonItemClickpackages/plugin-tree/src/ObjectTree.tsxonRowClickFour call the hook without the option and are untouched either way:
app-shell'sInterfaceListPage.tsx,ObjectDataPage.tsxandObjectView.tsx, plus the doc example inpackages/components/src/custom/navigation-overlay.tsx.What the repair does NOT reach — each is a published face of its own, and
naming the payload on it is the ruling objectui#9357 leaves open:
ObjectKanbanComponentProps.onRowClickand.onCardClick(plugin-kanban)KanbanRendererProps.schema.onCardClick(plugin-kanban/src/index.tsx)ObjectGallery'sonCardClick/onRowClickpair (plugin-list)onRowClickprop onObjectGrid,ListView,ObjectCalendar,ObjectGantt,ObjectMap,ObjectTimeline,ObjectTreeObjectGridSchema.onRowClickandObjectKanbanSchema.onCardClickin@object-ui/types, where the card records thatHandleClickModifiersisunreachable (phantom dependency plus a cycle) and
event?: anyis theestablished spelling
The three workaround call sites the card names —
app-shell'sObjectView.tsxat lines 2734, 3120 and 3163, each spelling
(record: any, event?: OPT any)—are consumers of those component props, not of the hook option, so they
still need the workaround and are deliberately left alone.
Source compatibility — one class DOES break, measured here rather than inherited
Two assignability directions hold between the old spelling and the new one, and
both are asserted in the pin so they cannot silently stop holding:
(
_NarrowIsAssignableToWide);one-parameter spelling (
_WideIsAssignableToNarrow) — its minimum argumentcount is still one.
That is a statement about those two spellings and nothing wider. An earlier
revision of this section read "No consumer breaks in either direction". That is
false, and the probe below refutes it.
The one class that has to change. A handler passed directly to
useNavigationOverlaywhose second parameter is annotated narrower thanHandleClickModifierscompiled before this branch and is refused now withTS2322. React'sMouseEventis the shape this hits in practice, because thepayload used to be discoverable only from the implementation, so a host that
wanted it wrote the annotation it saw arrive. The parameter is checked
contravariantly, so the annotation now has to admit
HandleClickModifiers.The remedy is one line at that call site: annotate the parameter
HandleClickModifiers(exported from@object-ui/react), or drop theannotation and let it be inferred. Either way the handler keeps receiving
exactly what it received before — a type-level change with no runtime behaviour
attached. The changeset publishes this class and this remedy, and as of
0d983a987so does the shipped declaration.The probe, re-run at
0d983a987. One temporary file per case underpackages/react/src/hooks/__tests__/, checked withtsc -p packages/react/tsconfig.test.jsonin two worktrees: this branch, andthe fork point taken from
git merge-base(b67b53bc0) — never frombase.sha, which is the base BRANCH TIP and not the fork point. Each probe washash-proved onto disk before its run and removed under
trap ... EXIT INT TERM,with absence re-checked afterwards.
b67b53bc00d983a987TS2322React.MouseEventTS2322, on the subject line onlyThe POISON row is what makes the zeros readable: without it, a probe the
compiler accepted and a probe the compiler never opened render identically.
⇒ the widening relaxes the option for anyone who wants the payload and
narrows it for exactly that one annotated class. No consumer inside this
repository is in that class, and the repository's own type-check re-derives
that on every run rather than this sentence asserting it.
Why the pin is not an assignability assertion — with the null result to prove it
Because both directions hold,
extendscannot tell the repaired declarationfrom the broken one. Two instruments that can are used instead, each with
controls proving it can fire:
(
Parametersand itslength), run only bypackages/react'stsconfig.test.json;the test file rather than the cwd.
Red-first, on the unmodified tree, verbatim
pnpm --filter @object-ui/react type-check— exit 2:(the TS2493 message's own generic is respelled in words for the reason at the
top; everything else is verbatim.)
pnpm exec vitest runon the pin — exit 1, the four assertions red and theseven controls green:
Ablation — two legs, on-disk proof before any result was read
Mechanism: mutate,
grep -cthe injected text and the deleted text beforereading anything, restore with
git checkout HEAD -- PATHundertrap ... EXIT INT TERM, and verify the restore by blob-hash equality againstthe HEAD blob plus an empty
git diff HEAD— never by an exit code. Bothrestores verified:
18ddadad7fecd2e38341060c0bf1e0251e4ca1d5, diff empty.Leg 1 — the full pre-fix defect put back (narrow declaration + assertion).
tsc --noEmitoversrc: exit 0. ⭐ The defect compiled cleanly. That isthe disease in one number: the assertion made the disagreement legal, so no
type instrument in the tree ever objected.
tsc -p tsconfig.test.json: exit 2 — the five errors above.4 failed | 7 passed.Leg 2 — the NULL RESULT the dispatch predicted, and it is a null result.
In Leg 1's error list the two assignability assertions, at lines 108 and 109,
are absent in both directions: they stayed green while the declaration was
narrowed back. So did
_FirstParamIsTheRecordat line 89, correctly — the firstparameter is identical in both spellings. Reported, not deleted: it is the
measurement that justifies the instrument choice.
Leg 3 — widened declaration kept, the assertion alone put back.
tsc --noEmit: exit 0.tsc -p tsconfig.test.json: exit 0.2 failed | 9 passed, onlyapplies no type assertion to onRowClickandcalls it with both arguments, directly.⇒ the type system is completely blind to a returning assertion once the
declaration is honest. The bytes half is the only thing in this repository that
reds on it, which is why it is in the pin.
Runtime behaviour is unchanged, observed rather than argued. The two runtime
control tests — the hook forwards
(record, event)to the supplied handler, anda one-parameter handler is still called — are green on the broken tree (they are
among the seven that passed in the red-first run and in Leg 1) and green on the
repaired one. A
tsctype assertion is erased at emit, so the call site'ssemantics could not move; this is the measurement that says so.
Dependent-set membership read
Read from the workspace graph, not inferred:
@object-ui/react— 32 workspace packagestransitively depend on it (28 directly). 31 declare a
type-checkscript;@object-ui/example-hello-worlddeclares none..changeset/config.jsonignore—@object-ui/example-*,@object-ui/site,@object-ui/test-support. This excludes packages fromversion bumping and is a different set from the one above. Five members
sit in both (
@object-ui/siteand the fourexample-*packages) and@object-ui/test-supportsits only inignore— so the overlap is partialand neither list may be read off the other.
fixedgroup — one group of 40 packages,@object-ui/reactamongthem.
majoris unavailable by repo rule; the changeset declaresminor.Verification
Every heavy run went through
../objectstack/scripts/pm/os-verify-lock.sh --on a stable slot,
os-dev-9357. VERDICT lines, quoted, never a bare exit code:idle a third time, the remaining work was narrowed, and the narrowing is
declared and measured — not assumed:
The narrowing, and the proof it excludes nothing. The published
.d.tssurface of
@object-ui/reactwas hashed file-by-file before and after therebuild: of 65 emitted declaration files, exactly one moved, and inside it
exactly one declaration line changed (the rest of that hunk is doc comment). So
the only packages whose type-check can move are the ones that read
UseNavigationOverlayOptions. The population was measured, not guessed: twelvepackages outside
packages/reactnameuseNavigationOverlay,UseNavigationOverlayOptionsorHandleClickModifiersanywhere in theirsources. All twelve plus
@object-ui/reactwere type-checked.Unlocked runs — exit codes captured after redirecting to a file, never through a
pipe:
pnpm --filter @object-ui/react builddist completeness: 1 package(s) complete (130 emitted files verified)type-checkover the 13 affected packages,--workspace-concurrency=2app-shellincludedvitest run packages/react/Test Files 83 passed (83),Tests 985 passed (985)vitest run packages/plugin-kanban/ packages/plugin-list/Test Files 130 passed (130),Tests 1263 passed (1263)pnpm --filter @object-ui/react lintGates derived by hand from the root
package.jsonand.github/workflows/(there is no dispatch-gates script in this repository) — all exit 0:
check:control-bytes,check:test-path-roots,check:new-line-citations,check:doc-example-readers,check:handler-key-reads,check:changeset-claims,check:phantom-deps,check:published-tsconfig-exclude,check:comment-mask-corpus,check:doc-examples,check:unreferenced-sources,changeset:check. A directcontrol-byte scan over the three touched files found none.
NOT MEASURED locally, and left to CI rather than claimed: a full
pnpm buildon the post-repair tree (the one that ran green was on the unmodified tree), the
four-shard
pnpm test, the nineteen packages of the dependent set that thedeclaration diff proves cannot be affected,
@object-ui/site's type-check(it needs
next typegen), and the repo-widepnpm lint.Re-verified at
0d983a987— the comment-only repairHeavy runs through
../objectstack/scripts/pm/os-verify-lock.sh -c, stable slotissue-9357-repair; exit codes captured by redirect before any pipe, andthe wrapper's own
VERDICT command-exitline read rather than a bare exitvariable.
pnpm --filter @object-ui/react builddist completeness: 1 package(s) complete (130 emitted files verified)pnpm --filter @object-ui/react type-checktsc --noEmitandtsc -p tsconfig.test.json, so both@ts-expect-errorrows are live: neitherTS2578-unused nor masking a second errorpnpm --filter @object-ui/react testTest Files 84 passed (84),Tests 995 passed (995)The carrier, before and after, read on the EMITTED declaration rather than on
the source. Census over
packages/react/dist/hooks/useNavigationOverlay.d.tsafter a real rebuild, newline- and JSDoc-continuation-tolerant (
perl -0777,continuations flattened so a
*between two words cannot hide a match):TS2322MouseEventHandleClickModifiersobjectui#9357Cmd/CtrlUseNavigationOverlayOptionsThe lit controls are what make that
0a reading rather than a broken grep.Nothing else in the published surface moved. Both trees hashed file by file
from the same build: 65 emitted
.d.tsbefore and after, exactly onehash moved (
hooks/useNavigationOverlay.d.ts); the exported-declaration censusis 295 = 295 with an empty symmetric difference — and the comparison can
see a difference, because the same set poisoned with one sentinel name does
compare unequal; and
dist/index.d.tsis byte-identical,5a1f5d143c595f68427d4a9eceb5bd87680ac069both times. A control-byte scan overboth touched files found none, paired with a lit control on a deliberately
poisoned file.
Inherited reds — NOT from this change, and re-derived rather than recalled
An earlier revision of this section named Doc Snippet Type Check and Skill
Example Check. Both names were wrong: at
e0c5c14b6both readsuccess.The instrument, so this paragraph can be re-derived instead of believed:
GET /repos/objectstack-ai/objectui/commits/REF/check-runs?per_page=100,paginated up to
total_count, bucketed byconclusion— and bystatusforthe entries still running, which a
conclusion-only bucket silently drops.Readings, each carrying the ref it was taken at, 2026-09-13:
e0c5c14b6— the reviewed headBundle Analysis, 0 cancelledb67b53bc0— this branch's fork pointBundle Analysisfailuree2feb13e1— themaintip the review readBundle Analysisfailureefc1c9c400— themaintip at the time of writingBundle Analysissuccesspoint and red on
mainwhen the review ran, butmainhas since gone green onit. This branch is not synced to that tip, so re-run the instrument above at
whatever head you are looking at rather than trusting this table.
Bundle Analysis's own bot comment on this PR (comment5653469550) calls itsverdict a broken-gauge reading on a ceiling and says explicitly that it is "not
a budget violation". This branch touches no bundle input, no chunk ceiling and
no baseline.
The three skipped entries at
e0c5c14b6aredependabot,Test (coverage)andthe coverage shard matrix.
Acceptance notes
app-shell/src/views/ObjectView.tsxaround line 2191declares an inline structural duplicate of
HandleClickModifiers(
OPT metaKey,OPT ctrlKey,OPT button) rather than importing theexported interface. Cosmetic, one file, and it belongs to whichever PR takes
the component-prop half of objectui#9357 — successor: that PR.
arity in exactly the way this card describes. They are not a separate finding
— objectui#9357's own "What is not decided here" section already carries them,
and picking their spelling is a ruling.
both review comments and in the earlier body — they credit
tsconfig.base.json'sremoveComments: false.packages/react/tsconfig.jsonextends the ROOT
tsconfig.json, which does not extendtsconfig.base.jsonand sets no
removeCommentsat all; onlytsconfig.node.json,tsconfig.scripts.json,tsconfig.react.jsonand one example package extendthe base. The comments ship because TypeScript's DEFAULT for
removeCommentsis off, not because this repo turned it off. The conclusion is unchanged and
is measured on the built artefact above; only the cited cause was wrong.
Successor: whichever PR next edits that base config, or none.
Disjointness against the in-flight PRs
File lists read for objectui#9356, #9343, #9144, #9339, #9351 and #9352: no
path overlap with this branch's three files. Beyond the file faces, the nearest
PR (#9356) asserts on
plugin-kanbanand@object-ui/typessymbols; thisbranch removes only one span of text, the assertion inside
useNavigationOverlay.ts, and a repo-wide search finds no test outside thisbranch that names
UseNavigationOverlayOptionsor reads that span. Theplugin-kanbanandplugin-listsuites were run on this branch and are green.Status
⛔ Draft on purpose. The seat does not flip ready, arm auto-merge or enqueue —
the PM's — and no label is added or removed from here.
needs:contract-reviewwas hung on this PR because the change moves a publishedtype in
@object-ui/react. It is no longer on the PR: the clause-② reviewreturned FAIL at
e0c5c14b6(comment5656788876) and the gate label wasstripped from both this PR and the card as part of that FAIL, with the handover
recorded on the card as comment
5656802868.0d983a987is the one repair thatFAIL owed, plus the two body corrections and the pin-prose corrections it
flagged below the bar.
Sessions for this work, as prose so an edit cannot strip them — the
implementation session
https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ,and the repair session
https://claude.ai/code/session_01L5xpA5q533BgTTNADibEFt.Generated by Claude Code
Generated by Claude Code