Skip to content

fix(components): filter-builder renderer hands onChange the declared FilterGroup - #26590

Merged
objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/project-thread-eq0t7c
Oct 27, 2026
Merged

objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/project-thread-eq0t7c

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Requested by Steve · project thread
Fixes #26382

Clause-②: no
Metadata-protocol: none

维护者速读

  • 改了什么:filter-builder 渲染器调用 onChange 时直接传 FilterGroup,不再传 { target: { name, value } } 表单事件;渲染器自己的 onChange 类型改为 FilterBuilderSchema['onChange'],去掉了那个 any。
  • 为什么改:schema 声明 onChange(filter: FilterGroup),渲染器却传事件对象,按声明写的处理函数读 .logic / .conditions 只会拿到 undefined,而 any 让编译器看不见。仓内没有任何宿主按事件形状绑定这个节点,所以以声明为准。
  • 风险与回滚:低。仓外若有宿主读 event.target.value,要改成直接读参数(changeset 已写明)。宿主经节点转发的 extraOperators 不再传进构建器,所以本节点永远不提供 exists / notExists;若存储的 value 绕过校验仍带这类运算符,渲染器不调用 onChange,并在 console 报出该运算符,不会抛错,也不会传出错的形状。回滚即 revert 本 PR。
  • 先看:ui/packages/components/src/renderers/complex/filter-builder.tsx(extraOperators 的解构、isAuthoredFilterGroup 与 handleChange);ui/packages/components/src/__tests__/filter-builder-renderer-onchange-26382.test.tsx。
  • 两分钟验证:仓根执行 pnpm --dir ui exec vitest run packages/components/src/__tests__/filter-builder-renderer-onchange-26382.test.tsx,预期 5 个测试全绿。
  • 需要你做的:无,席位落地。

What changes

  • ui/packages/components/src/renderers/complex/filter-builder.tsx: the renderer's onChange prop is typed FilterBuilderSchema['onChange'] instead of (event: any) => void, and handleChange is typed on the builder's FilterBuilderRowGroup instead of any. It calls onChange(group) with the group itself.
  • The builder's row type admits two opt-in operators (exists / notExists) that FilterGroup cannot carry, so FilterBuilderRowGroup is not assignable to FilterGroup (measured, below). The renderer narrows through a type predicate, isAuthoredFilterGroup, that checks every row's operator against the spec's VIEW_FILTER_OPERATORS.
  • The renderer takes extraOperators out of the host spread and never forwards it to FilterBuilder. It is the builder's only operator-granting prop, FilterBuilderSchema declares no such key, and what it grants (exists / notExists) is outside the declared FilterGroup. So the node's dropdown offers only protocol operators, and the off-contract branch cannot be reached from the node.
  • The one way left into that branch is a stored value row that skipped the FilterGroup zod mirror, which refuses such an operator. There the renderer does not call onChange and logs a console.error naming the operator. It does not hand on a group the declared type does not describe, and it does not throw inside a click handler. isAuthoredFilterGroup stays as the type guard. No as any or other cast remains on the path.
  • FilterBuilderSchema in @object-ui/types is unchanged: the declaration is the contract.
  • One root changeset, .changeset/26382-filter-builder-onchange-group.md (@object-ui/components patch).

Step 1: which side is the contract (the triage's measurement)

Searched ui/ and packages/ on origin/main for every host that could bind a filter-builder node's onChange through the event shape. Nothing does, so the renderer adopts the declared FilterGroup (Clause-②: no).

  • Form field hosts: renderFieldComponent in ui/packages/components/src/renderers/form/form.tsx resolves only field:-namespaced widgets for a form field. The filter-builder node is registered in the ui namespace, and no field:filter-builder registration exists. A form field never reaches this renderer.
  • SchemaRenderer host props: SchemaRenderer spreads the host's ...props last, so a host's onChange would reach the renderer. The only in-repo hosts that pass onChange to SchemaRenderer are two cases in plugin-view's handlerEventNameThroughRenderer-10616.test.tsx, and neither renders a filter-builder node.
  • Metadata-admin filter-builder widget: the spec's form rows (view.form.ts, page.form.ts, dataset.form.ts) name widget: 'filter-builder'. That key resolves to FilterBuilderWidget in app-shell's metadata-admin/widgets.tsx, which draws FilterBuilderField over FilterRuleLite[], and ObjectListViewInspector reads the same WIDGETS entry. Neither renders the filter-builder SDUI node, so neither reaches this onChange.
  • Docs and examples: ui/content/docs/components/complex/filter-builder.mdx declares onChange?: (filter: FilterGroup) => void, the same as the type. The five ui/examples/schema-catalog JSON documents carry no function, and their tests (filter-builder-mirror-6939, filter-builder-operator-alias-trigger-7561, filter-builder-operator-vocabulary-6939) read no onChange.
  • Other hits: the @object-ui/types tests, inline-locale-label-read-sites.test.tsx, known-schema-types.ts, widget-dom-leak-sweep.test.tsx and shadcn-components.json name the type but bind no onChange and read no target.
  • Not searchable from here: the objectstack-ai/cloud and hotcrm repositories.

Tests

  • New ui/packages/components/src/__tests__/filter-builder-renderer-onchange-26382.test.tsx invokes the registered renderer directly:
    • a host onChange gets the group, with no target, the group's logic, the existing row and the added row;
    • a handler typed from FilterBuilderSchema's own onChange (checked with satisfies), the channel a node-authored onChange arrives on, gets the same group;
    • a host forwarding extraOperators: ['exists', 'notExists'] through the node gets an operator dropdown that offers neither Is set nor Is not set (and does offer Is empty, so the dropdown really opened);
    • a stored row holding exists (off contract) makes the renderer skip onChange and log one console.error naming the operator;
    • type level: expectTypeOf pins FilterBuilderSchema['onChange']'s parameter EQUAL to FilterGroup, and a @ts-expect-error pins that a handler taking the old { target: { name, value } } event is not assignable to it. The file is compiled by the components tsconfig.test.json project, which type-check runs.
  • No existing test needed an edit.

Acceptance notes

  • Seat ruling on the off-contract branch: close it at its source, so the renderer stops forwarding extraOperators. On the remaining branch, the choice was a throw kept as an invariant, or console.error and skip the emit. This PR picks console.error and skip. The branch is not fully unreachable (a stored value that skipped validation still reaches it), so a throw there would still be a crash in the user's hands.
  • The renderer still spreads the other undeclared host props into FilterBuilder, as before. extraOperators is the only one that grants operators, and it is now held back. No in-repo host forwarded it.

Verification

components type-check (tsc --noEmit, then tsc -p tsconfig.test.json)

os-verify-lock: VERDICT command-exit 0, with the new test file in the test project.

vitest: new file plus inline-locale-label-read-sites

Test Files 2 passed (2), Tests 10 passed (10); after the extraOperators round: Test Files 2 passed (2), Tests 12 passed (12) (the new file: 5).

Ablation 1: call reverted to the event shape

scripts/ablation-replace.mjs replaced onChange(group); with the old { target: { name, value } } call (anchor 1 → 0, blob 96162878fc0f → ece34562e098). Two tests went red: expected { target: { name: 'filters', …(1) } } to not have property "target" and Target cannot be null or undefined. (Tests 2 failed | 2 passed). Restored: blob equal to HEAD, git diff HEAD empty.

Ablation 3: extraOperators forwarded again

extraOperators: _refusedExtraOperators, removed from the destructure, so the spread forwards it again (blob 5645f25cbc23 → 57047d781a2f): expected [ 'Equals', 'Does not equal', …(11) ] to not include 'Is set', Tests 1 failed | 4 passed (5). Restored: blob equal to HEAD, git diff HEAD empty.

Ablation 4: skip-emit return removed

The return; after the console.error replaced with a no-op marker (blob 5645f25cbc23 → 8a937573edee): expected "vi.fn()" to not be called at all, but actually been called 1 times, Tests 1 failed | 4 passed (5). Restored: blob equal to HEAD. A first attempt was refused by ablation-replace before any run, because its replacement text was a substring of the anchor (the count did not rise), so it was a no-op and not a reading.

Ablation 2: type guard removed (is it load-bearing?)

With the predicate replaced by false as boolean, tsc --noEmit reported TS2345: Argument of type 'FilterBuilderRowGroup' is not assignable to parameter of type 'FilterGroup', down to Type '"exists"' is not assignable to …. Restored: blob equal to HEAD.

Full components suite

pnpm --filter @object-ui/components test --maxWorkers=2 under the lock at 070b433f: Test Files 438 passed | 1 skipped (439), Tests 4273 passed | 24 skipped (4297), VERDICT command-exit 0.

After the extraOperators round, narrowed and declared as such at 904e58cf: every filter-builder* file in packages/components/src/__tests__/ (22) plus inline-locale-label-read-sites: Test Files 23 passed (23), Tests 392 passed (392). components type-check at 904e58cf: VERDICT command-exit 0. The full suite is left to CI's Test UI.

Derived gates at 904e58c (dispatch-gates --commands, then --ran)

Re-run in full at the final head, after merging origin/main (the derivation flagged a stale tree; main touched none of this PR's files and the derived list was the same 34). All 34 exit 0; dispatch-gates --ran: 34 derived famil(ies) accounted for — 34 run, 0 NOT-MEASURED (a DERIVED zero — all 34 recorded an exit code and none of them is 3). pnpm --dir ui exec vitest run scripts/: Test Files 178 passed | 2 skipped (180). check-adr-0087-registration: no declared-breaking changeset.

node scripts/check-adr-0087-registration.mjs --base origin/main :: exit 0
node scripts/check-adr-0087-registration.mjs --self-test :: exit 0
node scripts/check-changeset-no-major.mjs --base origin/main :: exit 0
node scripts/check-closing-keyword-parity.mjs :: exit 0
node scripts/check-closing-keyword-parity.mjs --self-test :: exit 0
node scripts/check-comment-mask-corpus.mjs :: exit 0
node scripts/check-empty-changeset.mjs --base origin/main :: exit 0
node scripts/check-empty-changeset.mjs --self-test :: exit 0
node scripts/check-sdui-manifest.mjs :: exit 0
node scripts/check-sdui-manifest.mjs --self-test :: exit 0
node scripts/pm/release-rehearsal-clone.mjs --self-test :: exit 0
node scripts/release-pending-publish.mjs --self-test :: exit 0
pnpm --dir ui exec vitest run scripts/ :: exit 0
pnpm --filter @objectstack/spec run check:duration-unit-keys :: exit 0
pnpm check:changeset-gate-self-tests :: exit 0
pnpm check:closed-ceremony-text :: exit 0
pnpm check:cross-package-test-inputs :: exit 0
pnpm check:dispatcher-error-vocabulary :: exit 0
pnpm check:driver-memory-census :: exit 0
pnpm check:engine-double-contract :: exit 0
pnpm check:gitlink-declared :: exit 0
pnpm check:nul-bytes :: exit 0
pnpm check:pm-changeset-deadline-census :: exit 0
pnpm check:published-files :: exit 0
pnpm check:query-options-erasure :: exit 0
pnpm check:refd-timer-probe :: exit 0
pnpm check:skill-identifier-liveness :: exit 0
pnpm check:test-source-alias :: exit 0
pnpm check:tier-file-adoption :: exit 0
pnpm check:type-check-coverage :: exit 0
pnpm check:type-check-debt :: exit 0
pnpm check:undeclared-dep-imports :: exit 0
pnpm check:watch-hint-literal :: exit 0
pnpm check:where-matcher :: exit 0

Generated by Claude Code

…FilterGroup

The renderer typed its onChange as (event: any) => void and called it with a
form-style { target: { name, value } } event, while FilterBuilderSchema.onChange
declares (filter: FilterGroup) => void. It now takes the schema's own type and
passes the group.

Claude-Session: https://claude.ai/code/session_01ADHkmNhEw4jVVQ4UTxrxMZ
Co-authored-by: Claude <noreply@anthropic.com>
…-builder renderer

Claude-Session: https://claude.ai/code/session_01ADHkmNhEw4jVVQ4UTxrxMZ
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tooling labels Oct 27, 2026
…s out of the builder

The node declares no extraOperators, and what they grant (exists / notExists)
is outside the FilterGroup onChange declares. The off-contract branch, now
reachable only through a stored value that skipped validation, skips the emit
and logs instead of throwing in a click handler.

Claude-Session: https://claude.ai/code/session_01ADHkmNhEw4jVVQ4UTxrxMZ
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对 · ACCEPT · seat domain:ui#1 · 2026-10-27T10:03Z · #26382

Head 904e58cf (read from the PR object): the fix commit 39ae55ec plus a merge of main that touches none of these files. Fixes #26382.

  • Shape. 3 files: the filter-builder renderer, a new test and a root changeset (@object-ui/components patch).
  • The renderer now speaks the declared contract.
    • onChange is typed FilterBuilderSchema['onChange'] and called with the group.
    • It used to be (event: any) => void, called with a form-style { target: { name, value } } event. No any or cast remains on that path.
  • Measurement (step 1). No host binds the event shape; the PR body has the search evidence. That puts the PR on the Clause-②: no side, as claimed.
  • Seat FAIL round, resolved.
    • Round 1 threw on an off-contract operator inside the click handler.
    • Now the renderer takes extraOperators out of the forwarded host props, so the node never offers exists / notExists.
    • A group off the protocol vocabulary can still arrive through a stored value that skipped the zod mirror. That case logs one console.error and skips the emit instead of crashing a click.
    • isAuthoredFilterGroup stays as the type guard.
  • Tests. 5/5 pass. A host forwarding extraOperators: ['exists','notExists'] gets no 'Is set' / 'Is not set'. Two ablations each go red: restoring the spread, and removing the skip.
  • Governed surface. scripts/pm/check-governed-merges.mjs --pr 26590: not governed.
  • Checks pinned to the head. All check runs completed: 0 failing, 0 pending. The seat's landing script checked this before posting.
  • Evidence (dev). Narrowed components run: 23 files / 392 tests. Type-check passes. Gates: 34/34.

Landing: ready + auto-merge through the relay.

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 27, 2026 10:04
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 27, 2026 10:04
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 27, 2026
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: TIER_DEFAULT
Head-sha: 904e58cfa0619e2bd4bd139ef0b3b95520c5916d
Local-runs: none

Director seat (summon #52, ledger objectstack#12708), by the maintainer's word 「你负责 pr 契约复审」 — the record is on the lane PR, read-only: the diff at the head above, card #26382, the cited code on origin/main 500bc9d5, and the head's check runs. dispatch-gates --tier on the PR's paths: no path-derived mandate (no governed or published-schema path), Clause-②: no, Metadata-protocol: none ⇒ the declaration face only, TIER_DEFAULT; served at CONTRACT_REVIEW_TIER.

① Derived judgments

  • Declared accept set unchanged, implementation aligned to it. FilterBuilderSchema.onChange?: (filter: FilterGroup) => void (ui/packages/types/src/complex.ts:641, untouched) is the contract; the renderer now types its own onChange as FilterBuilderSchema['onChange'] and calls onChange(group). Declaration over implementation (the seat's standing rule): correct, and Clause-②: no is right — nothing an author or host can write changes shape.
  • The old { target: { name, value } } argument was never declared. The card's host search (form field hosts resolve only field: widgets; the two SchemaRenderer hosts that pass onChange render no filter-builder node; the metadata-admin filter-builder widget is FilterBuilderField over FilterRuleLite[], not this node) stands as the measurement behind no; cloud and hotcrm are out of reach from here and are named as such in the body.
  • extraOperators taken out of the host spread. FilterBuilderSchema declares no such key; the custom FilterBuilder accepts extraOperators?: readonly string[] (ui/packages/components/src/custom/filter-builder.tsx:153) and it is the only prop that grants exists / notExists, which a FilterGroup cannot carry. The renderer stops forwarding an undeclared prop so the node offers only protocol operators. No declared surface moves.
  • The predicate reads the protocol's vocabulary. isAuthoredFilterGroup checks every row against VIEW_FILTER_OPERATORS from @objectstack/spec/ui (packages/spec/src/ui/view.zod.ts, re-exported by ui/index.ts), the same source the FilterGroup zod mirror refuses on. The narrowing FilterBuilderRowGroup → FilterBuilderRowGroup & FilterGroup is a type predicate, no cast; ablation 2 in the body shows it load-bearing (TS2345 without it) and Type Check UI is green on this head.
  • Off-contract branch: emit skipped, console.error names the operator. Only a stored value that skipped zod validation reaches it. Loud enough for the one way in; a throw would crash the click in the user's hands. Accepted as an implementation choice, not a contract change.
  • ⚠️ PR body is stale against the committed head. The 维护者速读 line 3 (「渲染器会抛错」) and "What changes" bullet 2 ("the renderer throws an error naming the operator") describe an earlier head. The head, the changeset and the test case "skips the emit, and names the operator on the console" all say skip + console.error. The lane seat's final six lines owe the correction (the seat corrects the dev's draft against its own reading of the diff); it is a text fix on the PR body, not a code must-fix.

② Semver level

@object-ui/components (published, 18.2.0) takes a patch changeset (.changeset/26382-filter-builder-onchange-group.md), Metadata-protocol: none, both declaration lines present in the PR body and the changeset and equal. A bug fix in a released package that aligns the renderer to its declared type is patch under AGENTS.md Post-Task Checklist step 3. The changeset prose matches the head (skip + console error). Correct.

③ Boundary flags

Implemented-by: claude/project-thread-eq0t7c
Reviewed-by: session_01ADHkmQr732toX135tyo5zs

VERDICT: PASS


Generated by Claude Code

Merged via the queue into main with commit c8ed4c9 Oct 27, 2026
62 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/project-thread-eq0t7c branch October 27, 2026 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tooling

Projects

None yet

2 participants