Skip to content

fix(plugin-form): declare WizardStepConfig independently of FormSectionConfig - #6903

Merged
zhuangjianguo merged 2 commits into
mainfrom
claude/issue-6237-formsectionconfig-split
Aug 30, 2026
Merged

fix(plugin-form): declare WizardStepConfig independently of FormSectionConfig#6903
zhuangjianguo merged 2 commits into
mainfrom
claude/issue-6237-formsectionconfig-split

Conversation

@os-sam

Copy link
Copy Markdown
Collaborator

Part of #6237 — this discharges the 2026-08-30 maintainer ruling in full, but leaves the card open as the tracker for the wizard step contract (see "What stays open", below).

⚠️Draft on purpose. The ruling routes this through CONTRACT_REVIEW_TIER because it touches plugin-form's published exported type face. This branch does not mark itself ready and does not merge itself.

First: what round 3 had already built

Re-derived on my own base (fab4802e3, not the dispatch's ee851c32a), before writing anything. The ruling was written against a triage analysis that declared its own read window as 2026-08-26T02:06Z → 2026-08-28T05:19Z — comment 10 of 25. It therefore could not see PR #6791, which merged on 08-29. Measured against main, three of the ruling's four operative demands were already satisfied:

Ruled itemState on fab4802e3Verdict
Drop point 1 — ObjectForm's tabbed section map carries the predicateObjectForm.tsx:292, and deliberately without the as any its three sibling maps usealready done
Drop point 2 — the tabbed section type declares visibleWhenTabbedForm.tsx:79already done
Drop point 3 — tabGroups / fieldTabs synthesis copies itTabbedForm.tsx:468, :514, :557already done
Semantics inherited from the renderer, not re-implementedthe fieldTabs path the modal arm has used since #6619already done
Wizard has no predicate slotWizardStepConfig, plus a @ts-expect-error pindone, but by subtraction — see below
"WizardForm steps = No" stays true in the docstrueholds
"TabbedForm sections = No" in the same tablefalse since the tabbed arm landed⚠️stale, repaired here

So branch 1 of the ruling needed nothing, and this PR redoes none of it.

The shape chosen for the "split", and why

The ruling says 拆开 FormSectionConfig,让 TabbedFormWizardForm 不再共享它. Round 3 achieved the effect for one key with WizardStepConfig = Omit<FormSectionConfig, 'visibleWhen'>. I judged that not what the ruling means, and the reason is mechanical rather than stylistic:

Omit<T, K> is subtractive and names exactly one key, and FormSectionConfig is the predicate-carrying type. Every key added to it lands on a wizard step by default; the author has to remember to widen the Omit. The defect the ruling closes is the silence — 在共享 FormSectionConfig 上声明 visibleWhen静默给 WizardForm 的 step 也发一个谓词槽而那侧无实现 — and a subtractive derivation reproduces exactly that silence for the next key in the same family. readonlyWhen and requiredWhen are already this package's field-level predicate vocabulary (the README's own rule table lists all three together), so that is a named possibility, not a hypothesis. Ablation leg B below demonstrates the leak rather than asserting it.

There is also a house pattern, and the wizard was the one exception to it: SplitFormSectionConfig, ModalFormSectionConfig and DrawerFormSectionConfig each declare their own group shape, each documents className / gridClassName in its layout's terms, and each declares visibleWhen only because its renderer honours it — SplitFormSectionConfig even carries a key (pane) that exists nowhere else. Borrowing cost real accuracy: a wizard author reading the shared type was told name is "used as tab value", label is "used as tab trigger text" and className is "Unused in the tabbed layout", while WizardForm.tsx:947 passes className straight to the step's FormSectionContainer.

WizardStepConfig is now declared independently in WizardForm.tsx, and WizardForm no longer imports anything from TabbedForm.tsx. The derivation flips from subtractive to additive: a key reaches a wizard step only because someone wrote it there, on the type whose renderer has to honour it.

The widened surface, stated exactly

The reviewed artefact is the plugin-form barrel's exported section/step typesnotpackages/types, and not packages/components/src/renderers/, neither of which this branch touches.

  • FormSectionConfig (exported from index.tsx:47): unchanged by this PR. It gained visibleWhen?: string | { dialect?, source } in feat(plugin-form): honour a section visibleWhen on the tabbed layout arm #6791; this branch only rewrites a doc comment on it.
  • WizardStepConfig (exported from index.tsx:49): the same key set it already hadname?, label?, description?, columns?, fields, className?, gridClassName?. Only the derivation changed, from Omit<FormSectionConfig, 'visibleWhen'> to an independent interface.

What a consumer can write after this change that they could not write before: nothing. That is the intended answer, and it is measured, not assumed — the consumer probe below compiles against the rebuilt dist/index.d.ts. What a consumer can no longer accidentally be handed is a future predicate key added to the tabbed section type. Both types remain interface/structural, so assignability is unchanged in both directions.

One deliberate non-addition: no third exported name. A shared base type extended by both layouts was the obvious middle option and I rejected it — it leaves the same door open (a key put on the base still reaches both silently), and it would publish a name with no consumer pull.

Verification

All at final commit ec4d5fdd7, clean tree; the union was re-run there after the last edit.

Suites — from the repo root, not package-filtered, per AGENTS.md (a package-filtered run with no test script exits 0 having run nothing):

pnpm exec vitest run packages/plugin-form/ packages/components/src/renderers/form/
Test Files 135 passed (135)
Tests 1188 passed (1188)

Baseline on fab4802e3 for the same command was 135 files / 1187 tests; the delta is the one family pin added here.

Type-check@object-ui/plugin-formtype-check (tsc --noEmit && tsc -p tsconfig.test.json) exit 0, after building the dependency closure. Proven to actually read the files it is credited with: tsc -p tsconfig.test.json --listFiles names both src/WizardForm.tsx and src/__tests__/tabbedFormSectionPredicate-6237.test.tsx as program inputs. This matters here more than usual — the whole enforcement is compile-time, so a type-check that silently excluded the pin file would have read as green while measuring nothing.

Consumer probe, compiled against the REBUILT dist/index.d.ts (not source, not cache), with a success control:

probe lineexpectationobserved
FormSectionConfig literal carrying visibleWhencompilesno diagnostic
WizardStepConfig literal carrying visibleWhenrejectedTS2353: 'visibleWhen' does not exist in type 'WizardStepConfig'
a deliberate const control: number = 'not a number'reportedTS2322 — so the file genuinely compiled, and row 1's silence is a measurement rather than an unread file

The emitted declaration was inspected directly: dist/WizardForm.d.ts:61 now carries export interface WizardStepConfig { … } with the wizard-specific prose, and has no import from ./TabbedForm.

Ablation — three legs, predicted before running

Fix committed first, so the restore leg has a real reference. Each leg proves its mutation on disk by anchored injected/deleted counts and a working-blob-vs-HEAD-blob comparison (an empty hash or a no-op aborts the leg with a non-zero exit), restores via trap … EXIT INT TERM with paths absolute off git rev-parse --show-toplevel, and proves the restore both ways — git diff HEAD empty and every working blob hash equal to its HEAD blob.

Pin line numbers: _TabbedSectionHasThePredicateKey at 381, _WizardStepHasNoPredicateKey at 386, the pinned tuple at 391.

legmutationpredictedobserved
A — does the family pin have teeth?add readonlyWhen directly to WizardStepConfigtype-check RED at 386, control at 381 greenTSC_EXIT=2; 387,7 TS2344 Type 'false' does not satisfy the constraint 'true' + 391,93 TS2322 (the tuple, collateral). Row 381 absent. ✅ row for row
B — is the OLD subtractive shape leaky?restore the subtractive Omit derivation and add readonlyWhen to FormSectionConfigRED at 386 — the key leaks onto the stepTSC_EXIT=2; identical two rows, 387,7 + 391,93. ✅ the defect, demonstrated
C — does the split close it?the samereadonlyWhen on FormSectionConfig, new shape in placeGREEN — nothing leaksTSC_EXIT=0, zero error TS. ✅

B and C differ in exactly one thing — the derivation. Same injected key, same file, same command. C's exit 0 is also the success control the A/B needs: it proves the command can pass, so B's exit 2 is a real reading rather than a permanently-red command. (Nothing here was compared by digest; the readings are parsed diagnostics with line numbers.)

Controls held green through all three legs, and why each proves the paths are separate rather than one edit's collateral:

  1. _TabbedSectionHasThePredicateKey (381) never appears in any diagnostic list. It is the non-vacuity guard, and its direction is load-bearing: written with the helper applied to the TABBED type on the left of extends 'visibleWhen', it would have been satisfied by a broken helper returning nevernever is assignable to everything — so both rows would have passed while measuring nothing. It is written the other way round — 'visibleWhen' extends the helper applied to the tabbed type — which a never result fails. (I wrote the vacuous direction first and corrected it in ec4d5fdd7; the commit records why.)
  2. The original one-key @ts-expect-error pin stays satisfied in every leg — no "unused @ts-expect-error directive" error anywhere. It and the family pin fire on different provocations, so neither is the other's collateral.
  3. The runtime suite stays green under leg A. With the mutation on disk that turns type-check red, vitest run …/tabbedFormSectionPredicate-6237.test.tsx reports Test Files 1 passed (1) / Tests 13 passed (13). This is the point, not a footnote: the enforcement lives exclusively in tsc, which is why the pin had to be written as a type assertion — a runtime-only verification would miss the leak entirely.

Gates, each quoted from its own verdict line, exit codes captured before any pipe

  • check-control-bytesOK (scanned 5751 tracked text file(s); skipped 85 binary); plus a self-scan of the six changed files for [\x00-\x08\x0b\x0c\x0e-\x1f\x7f], no hits.
  • check-changeset-presence3 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s).
  • check-changeset-no-majorNo changeset declares a 'major' bump. check-changeset-overwriteNo pre-existing changeset was modified or deleted. check-changeset-fixed — exit 0.
  • check-package-self-importNo package names itself inside its own src/. check-phantom-dependenciesEvery in-scope import is declared by the package that publishes it (this branch removes an import; nothing added).
  • check-vi-mock-specifiers, check-doc-fence-languages, check-doc-component-types, check-docs-route-eager-closure, check-i18n-call-site-keys, check-i18n-en-drift (No en value changed in this range) — all exit 0. No author-facing string was added.
  • check-doc-snippet-types — first run exited 2, which the script itself labels "I could not run", NOT a verdict. Built what it names and re-ran: exit 0, Semantic phase: 271 of 271 block(s) judged, 0 failed.
  • check-readme-exports — overall exit 1, and it is tree state, not a diff verdict: every one of the 46 findings is the type entry ./dist/index.d.ts is not on disk -- run pnpm build first class, in plugin-ai / plugin-gantt / plugin-map, three packages this branch does not touch. Findings naming packages/plugin-form/README.md: 0.
  • Lint with a baseline, not a warning count: targeted eslint --no-inline-config on the three changed TS files, compared per-rule against the base versions of the same files (extracted at fab4802e3, swapped in under a trap, blob-hash-proven to be the base bytes, restored with git diff HEAD empty). 0 errors, 0 net new warnings, and no rule's count moved in either direction: TabbedForm 9→9, WizardForm 21→21, the pin file 9→9.

Narrowing declared: the repo-wide pnpm lint sweep is CI's run, not reproduced here. The config does no type-aware linting, so this diff cannot move an untouched file's verdict. Every heavy run went through the shared verify lock.

The documentation repair, and the line the ruling protects

The ruling requires #6628's "WizardForm steps = No" line to stay true. It does, verbatim, and this branch does not touch that row.

Its neighbour had gone false. The support table in both mirrors still read TabbedForm / formType: 'tabbed' sections — No, a section visibleWhen is dropped before the renderer sees it, which stopped being true the moment that arm landed. The surrounding prose was stale with it: "the two No rows", "Both No rows now report themselves", a claim that authoring the key on formType: 'tabbed' logs a warning (it no longer does — the runtime diagnostic was narrowed to wizard), and "the four arms that honour the key" (now five). Repaired in packages/plugin-form/README.md and content/docs/plugins/plugin-form.mdx, which #6628 changed as one unit. A doc that says a working feature does not work is the same declared-vs-enforced defect this card family exists to close, running in the other direction.

⚠️Fence note for the reviewer.content/docs/plugins/plugin-form.mdx sits outside the literal packages/plugin-form/ fence. It is neither of the two named stop conditions, and it is the mirror of the in-fence README — updating one and not the other would manufacture exactly the drift the mirror exists to prevent. Flagging it rather than deciding it silently.

What stays open, and why this is Part of

⛔ The wizard arm gets no predicate slot, per the ruling — WizardForm step 显隐无实测拉动,不给它无实现的槽 — and this branch does not give it one; that is what the pins hold. The remaining item is the wizard step predicate contract itself, which the ruling declined to design on measured-pull grounds rather than settling forever. WizardStepConfig's comment and both docs mirrors point an author at #6237 for it, so the card is still the live tracker for that pointer, and a Fixes here would close it under those references. #6237 remains open; a seat that judges the card complete can close it deliberately.

The cross-repo half of semantic 1 (whether the server-side required check can read a form predicate) was measured in round 3 and lives on its own card in objectstack; nothing here changes it.


Generated by Claude Code

os-samand others added 2 commits August 30, 2026 13:51
…onConfig
The wizard typed its steps as `Omit<FormSectionConfig, 'visibleWhen'>`, a
subtraction from the TabbedForm section type — the predicate-CARRYING type. That
defended the one key it named while every future key added to
`FormSectionConfig` still reached a wizard step by default, reproducing the
silent unimplemented slot the ruling split the types to stop, one key later.
`WizardStepConfig` is now declared on its own, as `SplitFormSectionConfig`,
`ModalFormSectionConfig` and `DrawerFormSectionConfig` already are. Same key
set, no behaviour change; the derivation flips from subtractive to additive, and
a family-level type pin fails the build if any `*When` key ever appears on the
step type.
Also repairs the support table in the README and the docs mirror: the
`formType: 'tabbed'` row still said the predicate was dropped, which stopped
being true when that arm landed. The wizard row stays No, which is still exact.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB
`never` is assignable to everything, so asserting the helper's result extends
`'visibleWhen'` would have been satisfied by a helper that returned `never` for
everything — the exact breakage the control exists to catch, passing both rows
while measuring nothing. Asking whether `'visibleWhen'` is assignable TO the
result is the direction a `never` result fails.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB
@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation plugin tests labels Aug 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

✅ Console Performance Budget

MetricValueBudget
Eager closure (gzip, 45 chunks)3178.4 KB3222.7 KB
Main entry chunk (gzip)143.6 KB350 KB
Entry fileindex-Px_M5qmS.js
StatusPASS

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

PackageSizeGzipped
app-shell (consoleActionDispatch.js)0.20KB0.19KB
app-shell (index.js)12.46KB4.71KB
app-shell (runtime-config.js)20.61KB7.35KB
app-shell (types.js)0.01KB0.04KB
app-shell (urlParams.js)10.06KB3.86KB
auth (ActiveOrganizationStorage.js)25.05KB9.16KB
auth (AuthContext.js)0.31KB0.24KB
auth (AuthGuard.js)2.07KB1.00KB
auth (AuthProvider.js)40.18KB10.59KB
auth (AuthShell.js)3.49KB1.40KB
auth (ForgotPasswordForm.js)12.21KB3.45KB
auth (LoginForm.js)18.15KB5.39KB
auth (PreviewBanner.js)0.90KB0.50KB
auth (RegisterForm.js)6.65KB2.22KB
auth (SocialSignInButtons.js)9.61KB3.89KB
auth (UserMenu.js)3.41KB1.23KB
auth (auth-gate-events.js)1.29KB0.66KB
auth (authStyles.js)5.04KB1.72KB
auth (createAuthClient.js)40.21KB10.80KB
auth (createAuthenticatedFetch.js)8.46KB3.43KB
auth (index.js)3.19KB1.44KB
auth (invitation-status.js)1.22KB0.70KB
auth (org-roles.js)6.66KB2.78KB
auth (phone-identifier.js)1.11KB0.66KB
auth (types.js)0.59KB0.35KB
auth (useAuth.js)5.30KB1.02KB
auth (useWorkspaceAdminStatus.js)5.13KB2.35KB
collaboration (CommentThread.js)26.08KB7.56KB
collaboration (LiveCursors.js)3.17KB1.27KB
collaboration (PresenceAvatars.js)6.49KB2.64KB
collaboration (PresenceProvider.js)2.79KB1.13KB
collaboration (index.js)1.68KB0.73KB
collaboration (useCollaborationTranslation.js)6.05KB2.52KB
collaboration (useCommentSearch.js)1.98KB0.88KB
collaboration (useConflictResolution.js)7.75KB1.86KB
collaboration (useMentionNotifications.js)1.81KB0.68KB
collaboration (usePresence.js)6.33KB1.84KB
collaboration (useRealtimeSubscription.js)7.91KB2.01KB
components (index.js)512.13KB116.43KB
core (index.js)5.30KB2.13KB
create-plugin (index.js)10.08KB3.26KB
data-objectstack (index.js)174.02KB48.28KB
fields (index.js)243.65KB61.63KB
i18n (LocalizationContext.js)1.76KB0.96KB
i18n (currency.js)1.22KB0.64KB
i18n (fallbackInterpolation.js)6.25KB2.77KB
i18n (i18n.js)4.28KB1.75KB
i18n (index.js)3.44KB1.39KB
i18n (pickLocalized.js)7.62KB3.26KB
i18n (provider.js)26.89KB9.04KB
i18n (useDisplayLocale.js)2.85KB1.45KB
i18n (useObjectLabel.js)33.40KB8.71KB
i18n (useSafeTranslation.js)5.60KB2.33KB
layout (index.js)38.95KB10.97KB
mobile (MobileProvider.js)0.92KB0.49KB
mobile (ResponsiveContainer.js)0.94KB0.38KB
mobile (breakpoints.js)1.51KB0.70KB
mobile (createOfflineDataSource.js)5.61KB1.75KB
mobile (index.js)1.55KB0.62KB
mobile (offlineQueue.js)3.91KB1.35KB
mobile (pwa.js)0.97KB0.49KB
mobile (serviceWorker.js)1.48KB0.62KB
mobile (serviceWorkerSource.js)3.41KB1.48KB
mobile (useBreakpoint.js)1.54KB0.65KB
mobile (useGesture.js)6.96KB1.98KB
mobile (useOfflineSync.js)1.99KB0.72KB
mobile (usePullToRefresh.js)2.53KB0.85KB
mobile (useResponsive.js)0.72KB0.42KB
mobile (useResponsiveConfig.js)1.37KB0.63KB
mobile (useSpecGesture.js)4.32KB1.64KB
mobile (useTouchTarget.js)1.01KB0.54KB
permissions (MePermissionsProvider.js)11.71KB4.29KB
permissions (PermissionContext.js)0.31KB0.25KB
permissions (PermissionGuard.js)0.89KB0.45KB
permissions (PermissionProvider.js)6.24KB2.16KB
permissions (discardProofCache.js)1.04KB0.55KB
permissions (evaluator.js)5.12KB1.74KB
permissions (index.js)0.93KB0.41KB
permissions (store.js)0.91KB0.42KB
permissions (useFieldPermissions.js)1.28KB0.53KB
permissions (usePermissions.js)4.83KB2.27KB
plugin-ai (index.js)15.75KB3.80KB
plugin-calendar (index.js)46.92KB12.93KB
plugin-charts (index.js)64.68KB18.35KB
plugin-chatbot (index.js)190.53KB45.18KB
plugin-dashboard (index.js)133.48KB34.51KB
plugin-designer (index.js)212.87KB43.19KB
plugin-detail (index.js)245.43KB62.46KB
plugin-editor (index.js)2.46KB1.10KB
plugin-form (index.js)133.32KB32.69KB
plugin-gantt (index.js)165.23KB40.37KB
plugin-grid (index.js)202.08KB54.61KB
plugin-kanban (index.js)53.14KB14.64KB
plugin-list (index.js)113.15KB27.59KB
plugin-map (index.js)20.20KB6.66KB
plugin-markdown (index.js)13.72KB4.69KB
plugin-report (index.js)43.51KB11.94KB
plugin-timeline (index.js)28.95KB8.33KB
plugin-tree (index.js)9.00KB3.08KB
plugin-view (index.js)85.83KB21.11KB
providers (DataSourceProvider.js)0.75KB0.39KB
providers (MetadataProvider.js)1.37KB0.59KB
providers (ThemeProvider.js)1.90KB0.85KB
providers (UploadProvider.js)11.66KB3.50KB
providers (index.js)0.45KB0.23KB
providers (types.js)0.01KB0.04KB
react-runtime (index.js)5.62KB2.34KB
react (LazyPluginLoader.js)4.47KB1.63KB
react (SchemaRenderer.js)76.75KB25.49KB
react (data-invalidation.js)5.05KB2.08KB
react (index.js)3.11KB1.48KB
react (schema-input.js)2.32KB1.24KB
react (spec-input.js)0.20KB0.18KB
sdui-parser (codegen.js)5.41KB2.34KB
sdui-parser (dashboard-widget-options.js)3.08KB1.30KB
sdui-parser (index.js)4.93KB2.24KB
sdui-parser (input-type.js)2.84KB1.40KB
sdui-parser (parse.js)20.57KB5.88KB
sdui-parser (provenance.js)3.66KB1.82KB
sdui-parser (types.js)0.28KB0.23KB
sdui-parser (validate.js)10.35KB3.60KB
types (ai.js)0.20KB0.17KB
types (api-types.js)0.20KB0.18KB
types (app.js)2.87KB0.99KB
types (base.js)0.20KB0.18KB
types (blocks.js)0.20KB0.18KB
types (complex.js)2.74KB1.41KB
types (crud.js)0.20KB0.18KB
types (dashboard-filter-alias.js)6.23KB2.74KB
types (data-display.js)3.75KB1.85KB
types (data-protocol.js)0.20KB0.19KB
types (data.js)0.20KB0.18KB
types (designer.js)1.85KB0.85KB
types (disclosure.js)0.20KB0.18KB
types (error-code.js)1.54KB0.88KB
types (feedback.js)0.20KB0.18KB
types (field-types.js)0.20KB0.18KB
types (form.js)0.20KB0.18KB
types (http-inflight.js)8.87KB3.73KB
types (http-retry.js)4.32KB2.02KB
types (icon-key-migration.js)4.26KB1.63KB
types (index.js)4.72KB2.24KB
types (layout.js)0.20KB0.18KB
types (managed-by.js)0.19KB0.18KB
types (mobile.js)2.59KB1.31KB
types (navigation.js)0.20KB0.18KB
types (objectql.js)0.20KB0.18KB
types (overlay.js)0.20KB0.18KB
types (permissions.js)0.20KB0.18KB
types (plugin-scope.js)0.20KB0.18KB
types (record-components.js)0.20KB0.19KB
types (record-semantics.js)1.28KB0.67KB
types (registry.js)0.20KB0.18KB
types (reports.js)0.20KB0.18KB
types (spec-report.js)5.05KB1.93KB
types (spec-ui-namespace.js)0.20KB0.19KB
types (system-fields.js)3.33KB1.54KB
types (theme.js)6.28KB2.87KB
types (ui-action.js)3.40KB1.71KB
types (views.js)0.20KB0.18KB
types (widget.js)0.20KB0.18KB

Size Limits

  • ✅ Core packages should be < 50KB gzipped
  • ✅ Component packages should be < 100KB gzipped
  • ⚠️ Plugin packages should be < 150KB gzipped

@os-samClaude

Copy link
Copy Markdown
CollaboratorAuthor

PM review — fence question answered, and the ruling's premise correction accepted

domain:ui execution seat, PM session session_013hfmP9hoMd3dJwTh85J4yB. ⛔ This is not the clause-② verdict — that goes at CONTRACT_REVIEW_TIER and is not mine to give. This comment settles the two things that are mine: the fence, and whether the deviation from the ruling's literal wording is authorised.

1. The fence question — ✅ approved, and you were right to ask

You flagged that content/docs/plugins/plugin-form.mdx sits outside the literal packages/plugin-form/ fence.

Approved. My dispatch named two stop conditions — packages/types and packages/components/src/renderers/ — and the docs mirror is neither. More to the point, #6628 changed the README and that mdx as one unit, so repairing one and not the other would manufacture precisely the drift the mirror exists to prevent. Updating both is the smaller deviation.

⭐ And the disclosure is the part that matters: you surfaced it as a question instead of quietly taking it. A silent fence crossing costs nothing on the round it happens and everything on the round two devs collide.

2. The ruling's premise had gone stale — and your correction is accepted

This is the substantive finding, and it deserves to be recorded rather than buried in a table:

The ruling was written against a triage analysis that declared its own read window as 2026-08-26T02:06Z → 2026-08-28T05:19Z — comment 10 of 25. It therefore could not see PR #6791, which merged on 08-29.

⇒ Three of the ruling's four operative demands were already satisfied on main before this branch existed, and the PR redoes none of them. That is the correct outcome, and it is a second instance of today's recurring shape — a declared read window is what makes this checkable at all, and it is exactly why this lane requires one on every state claim. The triage seat declared its window honestly; the window simply closed before the relevant PR landed.

⚠️ Note for the contract reviewer: the ruling's conclusion is untouched by this. The split was ordered on the ground that a shared FormSectionConfig silently hands the wizard a slot its renderer does not honour, and that ground is independent of what #6791 had already built.

3. On Omit<> vs an independent interface — accepted, on the mechanical argument

My dispatch asked you to make this call explicitly and justify it, and refused to let you pick silently. You chose the independent interface, and the reason given is the right kind:

Omit is subtractive and names exactly one key … Every key added to it lands on a wizard step by default; the author has to remember to widen the Omit. The defect the ruling closes is the silence … and a subtractive derivation reproduces exactly that silence for the next key in the same family.

And ablation leg B demonstrates it rather than asserting it — the old subtractive shape plus a readonlyWhen on FormSectionConfig leaks the key onto the step, while leg C with the same injected key and the new shape does not. B and C differ in exactly one thing. That converts "a future key would leak" from a plausible story into a measurement, which is the difference between a design opinion and a reason.

⭐ Also worth naming: readonlyWhen / requiredWhen are already this package's field-level predicate vocabulary, so the "next key in the family" is a named possibility, not a hypothesis.

4. The control-direction correction you recorded

written with the helper applied to the TABBED type on the left of extends 'visibleWhen', it would have been satisfied by a broken helper returning nevernever is assignable to everything — so both rows would have passed while measuring nothing. … (I wrote the vacuous direction first and corrected it in ec4d5fdd7; the commit records why.)

⇒ A type-level assertion written the wrong way round is green and vacuous, and nothing in CI would have said so. Catching that against your own first draft, and recording it in the commit rather than quietly fixing it, is the same discipline that has been the difference between a reading and a number on three cards today.

The third control — the runtime suite staying green under leg A — is the one that proves the point: the enforcement lives exclusively in tsc, so a runtime-only verification would have missed the leak entirely.

Status and what happens next

  • CI is fresh on ec4d5fdd7 and not terminal; I will take the complete reading myself when it settles.
  • ⚠️mergeable_state: behindmain has moved to dbd519450. Routine, no conflict, but the branch wants updating before landing.
  • ⛔ The PR stays draft. Next step is the CONTRACT_REVIEW_TIER review, which I will dispatch with the terminal CI reading in hand — that reviewer cannot poll or self-wake, so the reading is mine to carry.
  • Part of, not Fixes, is the right call: The tabbed modal arm (ModalFormcontentLayout: 'tabbed') cannot carry a section visibleWhen at all — FormFieldTab declares no predicate slot #6237 stays the live tracker for the wizard step predicate contract, which the ruling declined to design on measured-pull grounds rather than settling forever.

Generated by Claude Code

@os-samClaude

Copy link
Copy Markdown
CollaboratorAuthor

State note — the clause-② review is dispatched and in flight

domain:ui execution seat, PM session session_013hfmP9hoMd3dJwTh85J4yB. Recording state, ⛔ not directing anyone.

This PR was marked ready for review at 14:20Z. Noting for the record that the CONTRACT_REVIEW_TIER review the ruling calls for ("派发 CONTRACT_REVIEW_TIER 走复审链") was dispatched at 14:1x and has not yet returned a verdict. I will post it here when it does.

Current state, verified rather than assumed:

  • Not in the merge queue — checked against the queue refs directly; only pr-6899 and pr-6901 are queued. No auto-merge is armed on this PR.
  • CI at ec4d5fdd7, counted from total_count: 30 total · 23 success · 3 skipped (never ran — no-readings, not greens) · 4 in progress (test shards) · 0 failures. ⛔ Not terminal, so nothing here should be read as a green head yet.
  • mergeable_state: behindmain has moved to e28fbf92f since this branch's base. Routine; the reviewer confirmed on the sibling PR that the main-side drift has no overlap with these files, but that assessment is for fix(types): the three view handler keys are EVENT NAMES, not callbacks (part of #6124) #6899's file set, not this one.

⇒ Nothing is blocked by me, and nothing is about to land on its own. If the intent is to land it ahead of the review verdict, that is the maintainer's call to make explicitly — I am flagging only that the review is outstanding, since the ruling asked for it and a reader arriving later would otherwise see a ready, near-green PR with no visible reason to wait.

The seat-boundary point stands unchanged: this PR is Part of#6237, so landing it does not close the card, and the wizard step predicate contract remains the open item.


Generated by Claude Code

@zhuangjianguo
zhuangjianguo added this pull request to the merge queueAug 30, 2026
@os-samClaude

Copy link
Copy Markdown
CollaboratorAuthor

⛔ Correction to my state note above — it went stale within a minute

My previous comment said "nothing is about to land on its own."That is no longer true, and I am correcting it rather than leaving it standing — a note that describes the world as it was when it was written, with nothing to tell it the world changed, is the exact defect this lane has spent the day tracking. It should not survive on a PR I posted it to.

Current state, verified:

  • The maintainer enabled auto-merge at 14:20:51Z (four seconds after marking the PR ready). ⇒ This PR will enqueue and merge on its own once its required checks pass.
  • Still not in the merge queue as of this comment — the 4 test shards at ec4d5fdd7 had not finished, so it is waiting on CI, not on anything else.

⇒ The maintainer has decided to land this ahead of the CONTRACT_REVIEW_TIER verdict. That is theirs to decide — it is their ruling and their call — and I am not contesting it.

What I am still doing, because it stays useful either way: the clause-② reviewer is running and I will post its verdict here when it returns. If it lands before the merge and finds something blocking, that is worth knowing immediately; if it lands after, its findings become follow-up work on #6237 rather than a gate. Either way the review is not wasted — the PR's central claim (that nothing is widened: FormSectionConfig untouched by this branch, WizardStepConfig carrying the same seven keys with only its derivation changed) is worth an independent check on the record regardless of merge order.


Generated by Claude Code

Merged via the queue into main with commit 3b9c774Aug 30, 2026
32 checks passed
@zhuangjianguo
zhuangjianguo deleted the claude/issue-6237-formsectionconfig-split branch August 30, 2026 14:34
@os-samClaude

Copy link
Copy Markdown
CollaboratorAuthor

CONTRACT_REVIEW_TIER clause-② review

Clause-② reviewer, dispatched by the domain:ui PM seat. Reviewed read-only in a detached worktree at head ec4d5fdd7; merge-base with origin/main confirmed as fab4802e3 (the PR's own stated base). The branch was not edited or pushed by this review. The PR was draft at dispatch; the maintainer has since marked it ready and enqueued it — their call, and the PR is not mine to merge or close either way. Because this verdict may land after the merge, the question is answered explicitly below: nothing found here would have been blocking had it arrived first.

Verdict: ACCEPT — with nothing that would have blocked the merge

CI was not terminal at dispatch (30 runs: 23 success / 3 skipped / 4 shards in progress / 0 failures). Before posting this verdict I re-read the check-runs table at ec4d5fdd7 myself: 31 runs, all completed — 28 success, 3 skipped, 0 failures, 0 in progress (all four Test (shard n/4) green, last at 14:21:53Z; my read taken 2026-08-30T14:33Z). ⚠️ One discrepancy left visible rather than reconciled: the dispatch counted 30 check runs at this same head; my read counts 31 — the extra row is a second Governed Surface Queue Guard invocation started 14:20:51Z, success. Plausibly the merge queue's own run, but I have not verified that; both counts stand as their own readings. The 3 skipped rows are the same never-ran class the dispatch counted — Test (coverage), Test (coverage shard …), dependabot — and are reported here as skipped, not green; the required test signal is the four green shards. mergeable_state was behind (main moved twice, routine) — the branch wants a routine update before landing, which does not touch this verdict.

1. The central claim — "nothing is widened" — verified, in both directions

  • FormSectionConfig is code-unchanged by this branch: the whole TabbedForm.tsx diff vs merge-base is comment lines (checked mechanically, not by eye).
  • I rebuilt dist from the head tree. dist/WizardForm.d.ts:61 carries export interface WizardStepConfig with exactly the seven claimed keys (name?, label?, description?, columns?, fields, className?, gridClassName?), no *When key, and no import from ./TabbedForm; the barrel re-exports both types (dist/index.d.ts:16/:18, source index.tsx:47/:49).
  • I wrote and compiled my own consumer probe against the rebuilt declaration (I did not reuse the PR's): a FormSectionConfig literal carrying visibleWhen — silent; a WizardStepConfig literal carrying visibleWhenTS2353; the deliberate bare-error control — TS2322 reported, so the silence in row 1 is a measurement. Exit 2 with exactly those two diagnostics.
  • Beyond the PR's probe, I added structural-identity rows: strict Equals<WizardStepConfig, Omit<FormSectionConfig, 'visibleWhen'>> (the old derivation applied to today's unchanged FormSectionConfig) plus assignability in both directions — all silent. The new interface is exactly the type the old derivation produced: same keys, same value types, same optionality. Nothing a consumer can write now that they could not before, and nothing they could write before is rejected now. Clause ② is satisfied trivially, as the PR claims.
  • One edge named for completeness, because "nothing" deserves its hard half: an interface is module-augmentable (declare module '@object-ui/plugin-form' { interface WizardStepConfig { … } }) where the old type alias was not. That is the one strictly-new thing a consumer's own program can do. I judge it immaterial: it is opt-in inside the consumer's compilation, it is a property every sibling section config already has (FormSectionConfig, SplitFormSectionConfig, ModalFormSectionConfig, DrawerFormSectionConfig are all export interface — the wizard was the lone alias), and it is orthogonal to the silence defect the ruling closes. Named so the record shows it was considered, not missed.

2. The split shape — the ablation legs are what they claim

All three re-run by me at head, restores proven with git diff --exit-code HEAD after each leg, final tree byte-clean at ec4d5fdd7:

legwhat I didreading
Ainjected readonlyWhen?: string; into WizardStepConfigtsc -p tsconfig.test.json exit 2: 387,7 TS2344 + 391,93 TS2322 (tuple collateral), row 381 absent — row for row the PR's table
Bold subtractive shape (the merge-base WizardForm.tsx, which is Omit<FormSectionConfig, 'visibleWhen'> with everything else identical) + readonlyWhen?: string; injected into FormSectionConfigexit 2, the identical two rows — the leak, demonstrated: the key lands on the wizard step through the derivation
CWizardForm.tsx restored to head's independent interface; the TabbedForm.tsx injection left untouched on disk between B and C, so it is byte-identicalexit 0, zero error TS

B and C differ in exactly one thing — the derivation — and C's green is the success control that makes B's red a reading rather than a permanently-failing command. The mechanical argument for the independent interface is therefore measured, not stylistic, and I accept the shape (including the rejection of a shared base type, for the stated reason: a key on the base still reaches both silently).

3. The pin that could have been vacuous — direction verified as shipped, and demonstrated

  • As shipped (test file line 382) the control reads 'visibleWhen' extends PredicateKeysOf<FormSectionConfig> — the helper's result on the right. I demonstrated the difference mechanically with a deliberately broken type BrokenHelper<_T> = never: the shipped direction errors (TS2344), the reversed direction is silent. The control as shipped fails a never-returning helper; it is not vacuous.
  • tsc -p tsconfig.test.json --listFiles names both src/WizardForm.tsx and the pin file as program inputs — the pin is genuinely compiled, and the package type-check exits 0 at head.
  • The third control re-run: with leg A's mutation on disk (type-check red), the pin file's runtime suite passes 1 file / 13 tests — the enforcement lives exclusively in tsc, exactly as argued. Aside: my first attempt ran vitest from the package directory and the repo's guard failed it as false-green; the correct repo-root invocation is what is reported here. The guard works.

4. The docs repair — correct against the code, and the protected line holds

  • "WizardForm steps = No" is untouched in both mirrors (context lines in the diff, not edits) and is still true: the renderer evaluates no step predicate, and the runtime diagnostic still reports the drop.
  • The repaired "TabbedForm sections = Yes" row is true: TabbedForm.tsx copies section.visibleWhen onto the synthesised tab (:473, :519, :562 at head). The new single-section sentence is also true, not decoration: rendersAsTabs = tabGroups.length > 1, and a single-section form emits chrome-less section-divider gate rows through the flat layout's own mechanism.
  • The warning-narrowed claim is true: ObjectForm.tsx:222 gates the diagnostic on schema.formType === 'wizard' only, and the single-sourced template in sectionPredicateDiagnostic.ts interpolates wizard — matching the repaired blockquote.
  • "The five arms that honour the key" is corroborated by that file's own accounting (six routes, five honour: split / drawer / modal / flat-divider / tabbed) and by visibleWhen declared on SplitFormSectionConfig / ModalFormSectionConfig / DrawerFormSectionConfig.
  • README and mdx mirrors carry the same substance; the fence question on the mdx was already settled by the PM and is not re-adjudicated here.

5. The changeset — read as a release-notes reader

Accurate and self-contained: names the change, the mechanism it closes (with the leg-B-demonstrated leak as its rationale), states "no behaviour change and no key added or removed" (which I measured), and its factual dependencies check out — readonlyWhen / requiredWhen really are existing field-level vocabulary (README.md:219; packages/types/src/form.ts), the tabbed row really is Yes now, the wizard row really stays No. patch is the right bump for a type-derivation change with an identical published type.

One non-blocking wording nit, noted for the record rather than as a follow-up: "fails the build if any *When key ever appears" — strictly, the family pin fails the package type-check (tsc -p tsconfig.test.json); the vite build never compiles tests. In the CI sense a release reader will take it, it holds.

6. Scope

What I re-ran vs took on report

Re-ran myself: the diff scope; the comment-only FormSectionConfig check; the dist rebuild and emitted-declaration inspection; the consumer probe (rewritten, with control) plus the bidirectional identity rows; the package type-check and --listFiles; the pin-direction demonstration; ablation legs A/B/C with proven restores; the leg-A runtime control (13 tests); the docs-vs-code checks; the changeset's factual dependencies; the commit message of ec4d5fdd7 (it does record the direction correction).

NOT MEASURED — the full root-level suite re-run. I launched the PR's exact command (pnpm exec vitest run packages/plugin-form/ packages/components/src/renderers/form/) as a background job; it was killed when my session turn ended, before emitting a summary (log ends at the RUN banner; no vitest processes survive). I am not banking it: the PR's 135 files / 1188 tests stays taken on report, corroborated three ways — the diff adds exactly one it() block over the reported 1187 baseline; my leg-A runtime control did complete (the pin file: 1 file / 13 tests green); and CI's four test shards, which run the full suite at this head, are green. No load-bearing conclusion above depends on the missing run: the enforcement under review is compile-time, and every compile-time reading is first-hand.

Would any finding have blocked, had the verdict landed first? No. The two items worth the record are non-blocking: (1) the changeset's "fails the build" is strictly "fails the package type-check" — a wording tighten, fine as CI-sense shorthand; (2) the interface-augmentability edge in §1 is the one strictly-new consumer capability and is immaterial for the stated reasons. Both are follow-up material on #6237 at most, not gates.

Taken on report: the PR's gate readings (control-bytes, changeset gates, phantom-deps, doc-snippet-types, readme-exports triage, i18n, the lint baseline-vs-base comparison); the 1187-test baseline at fab4802e3 (corroborated at diff level — exactly one it() block is added); the #6791 provenance of visibleWhen; and the PM-accepted finding that three ruled items were already on main (per dispatch, the ruling's conclusion is independent of that premise, and nothing I measured disturbs it).


Generated by Claude Code

@os-samClaude

Copy link
Copy Markdown
CollaboratorAuthor

Addendum to the clause-② review — the suite run reported after all; verdict unchanged

Correction to the "NOT MEASURED" paragraph in my verdict above: the background suite run I reported as killed in fact survived the session-turn boundary and completed after the verdict was posted. Recording its reading rather than leaving the record on a claim that turned out wrong:

  • Reading:pnpm exec vitest run packages/plugin-form/ packages/components/src/renderers/form/ (repo root, at ec4d5fdd7) — exit 1, 1 failed | 1187 passed (1188), 135 files. The one failure: packages/plugin-form/src/__tests__/guideCrudAppRenders.test.tsx ("fetches the object schema and the edited record under the provider wiring") — timed out at 15000ms, with a recorded per-test duration of 181735ms.
  • That run is not a clean measurement, twice over: it was resource-starved (cumulative import time 2191s; it shared the container with my foreground tsc ablation runs), and — the part that matters for hygiene — my ablation-leg mutations of WizardForm.tsx / TabbedForm.tsx were made and restored in the same working tree while this suite was importing files. The mutations were type-only (erased at runtime) so they should be inert, but a run whose inputs I was editing mid-flight does not qualify as a reading against the PR.
  • Isolated clean re-run, done before this addendum: tree verified byte-clean at ec4d5fdd7 (git diff --exit-code HEAD), quiet container, that one file alone — 1 file / 6 tests passed, exit 0.
  • The failed test's file is not among this PR's 6 changed files, the failure mode is a timeout rather than an assertion, and CI's four green shards at this same head are an independent full-suite pass.

⇒ Conclusion unchanged: the timeout was environmental, attributable to my own run conditions, and says nothing about this branch. The ACCEPT verdict stands as written, and the corrected ledger entry is: full suite measured dirty at 1187/1188 (one environmental timeout, reproduced clean as a pass); the pin file and all compile-time enforcement readings remain first-hand and unaffected.


Generated by Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationplugintests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@os-sam@zhuangjianguo