fix(plugin-form,types): resolve form.sections[].group through the single field-group assembler, and bound the section loop that blanked the form - #8644
Merged
Conversation
…single field-group assembler
The view-level half of objectstack#13855 (objectui#7051); the `record:details`
half shipped as objectui#8497.
`@objectstack/spec` 17.3.0 declares two ways for a `form.sections[]` entry to
give itself members: enumerate `fields`, or point `group` at one of the object's
declared `fieldGroups`. No form-section renderer read `group`, and the omission
was not a no-op — measured on `origin/main`:
simple threw `Cannot read properties of undefined
(reading 'map')` out of SimpleObjectForm's own
body, ABOVE the JSX it returns, and blanked the
whole form (siblings included)
tabbed/split/drawer/modal silently rendered nothing, no diagnostic
wizard an empty step
`ObjectForm` now resolves the reference once, above its routing fork, so all six
layouts inherit it. Resolution runs through `deriveFieldGroupLayout` (ADR-0085
§5) via this package's existing single adapter — the same code path the
no-sections field-group fallback uses — so a group authored by reference and one
derived by the fallback are the same section by construction. No assembly rule
is re-implemented on this side.
The object definition is fetched only when a section actually authors `group`.
Unresolvable references render nothing and are reported once; a group-owned key
restated beside `group` is ignored (no override semantics) and reported; `group`
on a wizard step is refused and reported. All three are shapes the spec door
refuses at parse, so they reach the renderer only from programmatic SDUI
callers — the door itself is pinned rather than assumed.
`ObjectFormSection` gains `group` and makes `fields` optional, so the spec-legal
shape compiles for a TypeScript author; the shared adapter now carries the
group's `description` and `visibleWhen` onto the derived section.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
…ately (objectui#7051) With `group` resolved above the section loop, every group leg stays green with the tolerant `?? []` read reverted — the guard was unpinned by anything else. This leg is the only one that fails then. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
…rows (objectui#7051) Measured, not predicted: with the first group these rows stayed GREEN under the constant-resolution caricature, because the first group is exactly what that caricature returns. A leg a caricature satisfies by accident tests nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-justin
marked this pull request as ready for review
September 8, 2026 19:32
os-justin
enabled auto-merge
September 8, 2026 19:33
This was referenced Sep 8, 2026
`UNGATED_EXAMPLES` in `scripts/check-doc-example-types.mjs` is keyed by `path:line symbol`, where the line is the position of the `@example` JSDoc tag. Two rows on this branch named lines that no longer hold their tag, so `scripts/__tests__/check-doc-example-types.test.ts` reddened on "every row names a block that is actually in the compiled tier". Both blocks still exist and are still in the compiled tier; only their position moved, and both example bodies are byte-identical to the merge-base, so each row's recorded diagnostic codes and written reason stay accurate: ObjectForm.tsx 122 -> 123 (one added import above the JSDoc) objectql.ts 1572 -> 1604 (schema additions above the JSDoc) `origin/main` carries both tags at their original lines, so this branch is the sole cause; no merge from `main` is required. The line-number keying that makes an unrelated PR edit a CI gate script is objectui#8614. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
Collaborator
Author
CI repair:
|
| ledger row | was | now | why |
|---|---|---|---|
packages/plugin-form/src/ObjectForm.tsx ObjectForm |
122 | 123 | the added ./sectionGroups import sits above the JSDoc |
packages/types/src/objectql.ts ObjectFormSchema |
1572 | 1604 | the schema additions sit above the JSDoc |
Both blocks still exist and are still in the compiled tier — this is a move, not a deletion:
present: "packages/plugin-form/src/ObjectForm.tsx:123 ObjectForm" symbol=ObjectForm pkg=@object-ui/plugin-form
present: "packages/types/src/objectql.ts:1604 ObjectFormSchema"
Both example bodies are byte-identical to the merge-base (diff of the two regions is empty), so each row's recorded codes and written reason stay accurate — no row was re-derived, only re-addressed.
main is ruled out as the cause
The @example tag sits at its original line on both the merge-base and origin/main, and only moved on this branch:
c4326fe0a (merge-base) ObjectForm.tsx:122 objectql.ts:1572
da5e4f69e (origin/main) ObjectForm.tsx:122 objectql.ts:1572
3c57b9c2f (this branch) ObjectForm.tsx:123 objectql.ts:1604
main has not touched scripts/check-doc-example-types.mjs since the branch point. Of main's 122 changed files, exactly one carries a ledger row (packages/core/src/data-scope/DataScopeManager.ts), and its tag is unmoved at line 68 on all three refs — so merging main in would neither fix nor break anything here. No merge was needed; the repair is the re-key alone.
Verification
Reproduced the red first, then showed the same check green:
before Test Files 1 failed (1) Tests 1 failed | 45 passed (46)
AssertionError: packages/plugin-form/src/ObjectForm.tsx:122 ObjectForm: expected false to be true
after Test Files 1 passed (1) Tests 46 passed (46)
Neighbourhood re-run so one red is not traded for another:
pnpm exec vitest run scripts/— 125 passed | 2 skipped (127 files), 3684 testspnpm exec vitest run packages/plugin-form/ packages/types/— 236 passed, 3737 testsnode scripts/check-doc-example-shared-reader.mjs—OK 80 documented symbol(s), 3989 call site(s), 11 slot(s) ...node scripts/check-changeset-presence.mjs— green on the existing changeset; the gate script is not published source, so no new changeset is owed
On having to edit a CI gate script from a plugin-form PR
This is exactly the tax objectui#8614 describes: UNGATED_EXAMPLES is keyed by line number, so a one-line import added above a JSDoc block forces an unrelated PR to edit a CI gate script to stay green. Paying it here because this PR has to land; objectui#8614 remains the standing card for the keying mechanism and is deliberately not addressed in this diff — changing how the ledger is keyed carries its own design question and would widen a plugin-form PR into gate-infrastructure work.
Diff of this repair is two lines, both ledger keys.
Generated by Claude Code
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
This was referenced Sep 9, 2026
Merged
os-justin
deleted the
claude/issue-7051-form-sections-group-reference
branch
September 9, 2026 00:41
This was referenced Sep 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #7051
Deliverable 2 of that card — the view-level
form.sectionshalf of objectstack#13855. Deliverable 1 (record:details) landed as #8584 and is not touched here. Base pinned atc4326fe0a; every reading below is against that commit, not against the movingorigin/main.The premise held, and the two surfaces are NOT symmetric
Re-measured, with a lit control. Sweeping
packages/*/src/*+apps/*/src/*for a section-levelgroupread returns 4 hits, all of them PR #8584'srecord-details.tsx. The same command shape over the same population forfield.groupreturns 9 hits across 6 files, so the instrument fires and the zero is a real zero: no form-section renderer readsection.group. Premise valid.The card's warning not to assume symmetry was the right one, and the answer is that the six form layouts are not even symmetric with each other. Measured by rendering an authored
{ group: 'basic' }section on each, on the base commit:{ group }section didsimpleCannot read properties of undefined (reading 'map')and blanked the ENTIRE form — sibling sections gone, body emptytabbed/split/drawer/modalwizardThe throw was captured with an error boundary rather than inferred:
SimpleObjectForm (ObjectForm.tsx:1262) -> ObjectForm.tsx:1272, i.e. the section loop in the component's own body, above the JSX it returns. That is the same above-the-loop shaperecord:detailshad, so a per-section error boundary could not have caught it either — the card's central warning, confirmed on the one layout where it applies. The other five differ becausebuildSectionFields(sectionFields.ts) already spells the same readsection.fields ?? [].What changed
One resolution site, above the routing fork.
ObjectForm's dispatcher rebuilds each variant'ssectionsprop key by key, so agroupleft in place is dropped before any container can see it, and each container would otherwise need its own copy of the resolution. Resolving in the dispatcher's ownschemamemo is what makes "one assembler" true for all six layouts at once — and the four container rows in the pin file are what makes that claim checkable rather than asserted.Resolution goes through the one adapter.
deriveFieldGroupSections(fieldGroups.ts) is this package's single adapter ontoderiveFieldGroupLayout(ADR-0085 §5), and it is the same one the no-sections field-group fallback already used. Authoring a group by reference and letting the fallback derive it now produce the same section because one of them is the other's code path — the same construction argument #8584 made forderiveFieldGroupDetailSections. No assembly rule is re-implemented here.The load is gated. The dispatcher fetches the object definition only when a section actually authors
group, so a form that does not use the reference form issues no request and takes no new path.resolveSectionGroupReferencesreturns its input array by reference when nothing uses the form, so the memo cannot perturb an existing form either.Two containment layers, and only one of them is about
group. Layer 1 is the resolution itself. Layer 2 issection.fields ?? []atObjectForm.tsx's section loop, which bounds every other way a memberless section can reach that line — the sixth reader joining the five that already spelled it that way.One divergence from #8584, deliberate and measured
#8584 drops an unresolvable group section. Here it is emptied (
fields: []) instead. Both render nothing; the difference is forced by this surface. A form whosesectionsarray empties out stops being a sectioned form at all and falls back to the flat every-field layout, so dropping would make one mistyped key render more than the author asked for — and it would also move the wizard's step count and the container routing's condition. Reasoning is in the module docstring so the next reader does not "fix" it back.Where the spec's refusals are actually enforced — measured, not assumed
The card asked that group-owned presentation keys beside
group, andgroupon a wizard step, not be silently accepted. #8584 concluded that existence checking belongs to@objectstack/lint. These two are stronger than that: they are refused at parse, by@objectstack/spec17.3.0 itself.FormSectionSchema/FormViewSchemaverdict{ group, fields }{}/ neither keygroup+name/label/description/collapsible/collapsed/visibleWhengroup+columns/panegroupon atype: 'wizard'sectionsimple/tabbed/split/drawer/modalgroup: 'Not A Key'FIELD_GROUP_KEY_PATTERNSo authored metadata carrying any of them never reaches this renderer. What does reach it is a programmatic SDUI caller, which does not pass the spec door — and for those the renderer reports rather than accepts: a restated group-owned key is ignored (no override semantics) and warned about; a wizard
groupis refused and warned about; an unresolvable key renders nothing and is reported once, namingform-section-group-unknown. The parse door itself is pinned in the test file, because "enforcement lives upstream" is a claim about upstream, and a claim nothing checks is how a door quietly stops closing.fieldGroup/groupKeyfold togroup". OnFormSectionSchemathey do not fold: they are unrecognized keys, refused with a did-you-mean hint. A renderer-side fold would therefore have been a second, more permissive contract. Pinned so the correction survives.The types half — and which instrument can see it
ObjectFormSectionhad the analogous omissionrecord:detailshad (objectui#8583), but a narrower one: exactly one missing key (group) plus afieldsthat was REQUIRED, and nothing declared that the spec refuses. Enumerated against the installed schema,FormSectionSchemaacceptsname, label, description, collapsible, collapsed, columns, pane, visibleWhen, visibleOn, fields, group; the type declared all of those butgroupandvisibleOn. (visibleOnis the deprecated ADR-0089 alias and its absence from the authoring type looks deliberate; it is left alone and reported, not widened.)⭐ The instrument was established with a lit control before either reading was trusted. With
group?: stringremoved from the interface — mutation proven on disk,1 -> 0occurrences, blob hash moved:vitest(2 files, 43 tests)pnpm --filter @object-ui/types type-checkTS2353x3, naminggrouponObjectFormSectionand on thekeyof ObjectFormSectionkey censusSo the type-level legs are
type-checklegs, and a greenvitestsays nothing about them. Restored and proven restored (blob hash equal to HEAD's,git diff HEADempty).The same instrument caught a consequence I would otherwise have shipped: making
fieldsoptional broke five call sites inObjectForm.tsx(TS2322, the per-layout config types arefields-required), andvitestwas green on all 3736 tests at that moment.Ablation — run, not predicted, and it changed the tests twice
Every leg: mutation proven on disk in both directions (before/after occurrence counts plus a moved blob hash, with a void-leg abort if the blob did not move), classified per-test from vitest's JSON reporter, restored by
git checkout HEAD -- pathand proven restored by blob hash and an emptygit diff HEAD.SECTION_GROUP_RENDERS_ITS_OWN_MEMBERS,SECTION_GROUP_LABEL_IS_THE_GROUPS_OWN,UNKNOWN_GROUP_..., all four container rows, 2 resolver units (9 red / 22)SIBLING_SECTION_SURVIVES,FORM_IS_NOT_BLANKED,SECTION_WITHOUT_MEMBERS_IS_BOUNDED, every spec-door row?? []) reverted, resolution keptSECTION_WITHOUT_MEMBERS_IS_BOUNDEDonly (1 / 22)WIZARD_STEP_GROUP_IS_REFUSED_AND_REPORTEDonlydescription/visibleWhencarry removedSIBLING_SECTION_SURVIVESandFORM_IS_NOT_BLANKEDboth pass under the caricature, exactly as the card's correction from objectui#8497 predicted. The file says out loud which legs are which so nobody re-reads the weak one as the strong one. The discriminating leg isSECTION_GROUP_RENDERS_ITS_OWN_MEMBERS: two sections referencing different groups, asserted as an ordered structural outline of headings and concrete field names, so a resolver that answers the same thing for every input cannot satisfy both.The ablation changed the tests twice.
contact_info, the first declared group — and stayed GREEN under the caricature, because the first group is exactly what a constant resolver returns. They now reference the second group (billing) and go red. A leg a caricature satisfies by accident tests nothing.groupresolved first, a{ group }section is never memberless by the time the loop sees it, so every group leg stayed green with the tolerant read reverted.SECTION_WITHOUT_MEMBERS_IS_BOUNDEDwas added specifically for it, and the third row above is the proof that it is the only leg that catches it. This is the same discovery fix(plugin-detail): implementsections[].group, bound a section's blast radius, and resolve enumerated field labels through the declaredlabel#8584 recorded about its own two guards.Harness-kill leg. Every rendered leg navigates through
requireLiveForm, whose anchor is a section no leg asserts anything about, and whose failure text (HARNESS DEAD: expected exactly 1 ...) is textually unlike every content assertion. It fires in both directions — zero anchors and duplicated anchors — so a caricature cannot satisfy the harness by adding a section. No positional navigation anywhere.Verification
packages/plugin-form+packages/types(final HEAD3c57b9c2f)@object-ui/typestype-check(3 programs)@object-ui/plugin-formtype-check(2 programs)apps/consoletouched test filenode scripts/check-changeset-presence.mjsnode scripts/check-changeset-fixed.mjsnode scripts/check-control-bytes.mjseslintover the 8 changed files,--format jsonno-explicit-any/react-hooksrules on untouched linesOn the eslint narrowing, so it reads as a measurement and not as a skipped run. Population is the 8 files this PR changes, counted from the JSON reporter's own array length. The repo's
eslint.config.jssets noparserOptions.projectand noprojectService, so type-aware linting is not enabled and this diff cannot move the verdict on any file it does not touch. The repo-wide run is CI's.Everything else in the affected set is CI's:
@object-ui/typesis a root dependency, soturbo ls --affectednames essentially the whole monorepo, and running that locally would be re-running CI.Fixture triage
One pre-existing file needed a prose correction rather than an assertion change:
apps/console/src/__tests__/public-block-binding-reach.test.tsxdocumented itssectionssample by asserting "this repo's own type says the same:ObjectFormSection.fieldsis REQUIRED" and "ObjectForm.tsx:1166readssection.fields.map(...)off each entry unguarded". Both halves moved with this PR. The docblock is rewritten, not deleted: the fixture's reasoning is unchanged (a bare string is still not a section, and{}is still refused — the quoted spec message is re-measured against 17.3.0), only its grounds, and the two moved halves are called out explicitly so the paragraph cannot be read as still-true. That file's 16 tests pass unchanged.Out of scope, filed
objectui#8641 —
apps/console's ownFormPage.buildSectionsis a third form-section consumer with the same gap:sec.fields ?? []on a{ group }section yields a section with no label and no fields, silently. It does not go throughObjectForm's dispatcher and the resolver here is package-internal, so fixing it is a real design choice (export the resolver / call the assembler directly / record the divergence), not a copy-paste. Filed with the discriminating axis attached, and with the honest note that it was established by reading the source rather than by a DOM measurement.Risk and rollback
The one behaviour change reaching a path that does not author
group:deriveFieldGroupSectionsnow carries the group'sdescriptionandvisibleWhenonto the section it derives, which the no-sections field-group fallback also uses.descriptionis additive.visibleWhenis not purely additive — an object that declares afieldGroups[].visibleWhenand relies on the fallback would now have that predicate honoured, and a predicate resolving false hides the section. That is the ADR-0089 contract and the key the spec names as group-owned, so carrying it is the correct direction; it is called out because it is the only line in this PR that can change an existing form. Rollback is the two added lines infieldGroups.ts, and the resolver unit leg pins them.Everything else is gated on an authored
group: no such section, no fetch, same array reference out of the resolver, no new path.🤖 Generated with Claude Code
https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
Generated by Claude Code