Repository navigation
fix(plugin-detail): implement sections[].group, bound a section's blast radius, and resolve enumerated field labels through the declared label - #8584
Merged
os-justin merged 2 commits intoSep 8, 2026
Conversation
…t's declared label
An enumerated `sections[].fields` list resolved its labels ONLY through the
i18n bundle, so every app with no translations configured rendered raw
snake_case field names on its detail pages while the object's own `label:`
sat unread one lookup away — and nothing reported it.
`DetailSection` renders `fieldLabel(objectName, field.name, field.label ||
field.name)`: an i18n lookup whose third argument is the fallback, i.e. the
ladder is `bundle key -> field.label -> field.name`. A spec-shaped section
enumerates bare strings, which `normaliseField` turned into `{ name }` with
no `label`, so rung 2 was missing entirely and the ladder collapsed to
`bundle key -> field name`.
`withDeclaredLabel` fills that rung from the bound object definition. This is
not a new source: it is the same read the GROUP-derived body has always
performed (`toField` in `synth/buildDefaultPageSchema.ts`, `label: f.label ||
name`), which is precisely why the platform's synthesized default record page
showed declared labels while a hand-authored enumeration of the same fields
did not. An authored entry `label` still wins, and an app WITH a full bundle
is untouched because rung 1 resolves before rung 2 is read.
`pickLocalized` rather than a raw read, so an inline `{ en, 'zh-CN' }` label
cannot reach the DOM as `[object Object]`.
Pins all three rungs, not just the first and the last: bundle key wins; with
no bundle the declared label wins; a field declaring no label still falls
back to its own name.
Four existing fixtures navigated rows by the raw field NAME, which is the
behaviour this commit changes, so their row identifiers move to the declared
label their own object schemas already carried. Their subjects — the title
dedupe, the empty-section heuristic, the `hideEmpty` retirement — are
untouched; only how a row is named moved. Two comments that documented the
missing rung as if it were the contract are rewritten rather than deleted, and
the one negative assertion among them was re-pointed so it cannot pass
vacuously against a string the page no longer renders.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
…n's blast radius
An authored `{ group: 'parties' }` on `record:details` threw
`Cannot read properties of undefined (reading 'name')` and took every sibling
section down with it — one authored key blanking an entire record page.
`@objectstack/spec` 17.3.0 declares the group-reference form in full
(`RecordDetailsProps.sections[].group`, objectstack#13855): it inherits the
object's `fieldGroups` entry with that key, members and presentation both,
from `deriveFieldGroupLayout` (ADR-0085 §5). This renderer read the key
nowhere and its props description told authors it "does nothing", which was
false in the worst direction. `git log -S` on the crashing expression names
one commit, `e997708419c1d311c2126c93762578d330cb48cf` (2026-05-01), months
before the key it fires on was declared — so the throw was never a deliberate
refusal of anything. The declaration was right and the runtime had simply
never honoured it, so `group` is implemented rather than made a no-op.
Resolution goes through `deriveFieldGroupDetailSections`, the SAME adapter the
synthesized default record page already uses — deliberately, not a second
derivation: that is what makes the two bodies agree, which is what let this
ship (the synthesized page showed declared labels, the enumerated section did
not). The spread order is the spec's precedence: a group-reference section may
carry only `columns` / `showBorder` / `headerColor`, so the group owns its
presentation and the page owns its layout, with no key having two sources.
A `group` naming no declared group renders nothing and is reported once on the
console. Existence is not a parse question — the spec assigns it to
`@objectstack/lint`'s `page-section-group-unknown` — and `deriveFieldGroupLayout`
already drops a declared group nothing references, so dropping is the contract.
Vanishing without a word is not.
Blast-radius containment is two layers because one cannot do it. The crash
fired inside a `useMemo` in `DetailView`'s own body, ABOVE the section loop and
outside every per-section subtree, so a boundary alone could never have caught
it: `sectionFieldEntries` guards that path. Each section is additionally
wrapped in the existing `SchemaErrorBoundary`, pushing its granularity from
per-component to per-section, so any OTHER throw shows in the broken section's
own place while its siblings keep rendering their fields and their labels.
`@object-ui/types` is the same contract's type half: `RecordDetailsComponentProps`
required `fields` and declared no `group`, so the shape this commit implements
did not COMPILE for a TypeScript author — the same declared-versus-enforced
disagreement pointed the other way. `fields` becomes optional and `group` is
declared, exactly as the spec has them. The pin is compile-time first; writing
it also measured that the section type declares no `columns` either (nor
`icon` / `description` / `showBorder` / `headerColor` / `defaultCollapsed`),
a PRE-EXISTING divergence left alone here and filed instead.
The `inputs` description is part of the fix, not a footnote: it now documents
`group` as the reference form it is, and `fields` as required only when `group`
does not supply the members.
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 13:33
os-justin
enabled auto-merge
September 8, 2026 13:33
This was referenced Sep 8, 2026
os-justin
deleted the
claude/issue-8497-detail-section-group-and-labels
branch
September 8, 2026 14:02
This was referenced Sep 8, 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 #8497
Two defects, two commits. Base pinned at
e411c3e58; all readings below are against that commit, not against the movingorigin/main.Defect 1 — an authored
sections[].groupthrew and blanked the pageReproduced before touching anything, on
e411c3e58:The card's framing of the evidence needs one correction
The card reads the spec's docblock as predicting a silent no-op, quoting
field-group-layout.ts— "the reference surface would accept a key the declaring surface refuses, so the reference could never resolve and the section would render nothing with nothing reported." Read in place, that sentence is a counterfactual: it is the rationale for declaringFIELD_GROUP_KEY_PATTERNonce, i.e. the harm that single-sourcing the key grammar prevents. It is not a statement about what an authoredgroupdoes.What the spec actually declares (installed pin
@objectstack/spec17.3.0,packages/spec/src/ui/component.zod.ts) is a fully specified, working capability:So this is a two-face disagreement, not three, and it resolves cleanly: the spec declares
groupas implemented; objectui's own props description said "authoring it does nothing on this renderer"; the runtime threw.Which side was intended — measured, not assumed
git log -Son the crashing expression names exactly one commit:e997708419c1d311c2126c93762578d330cb48cfThat is months before objectstack#13855 declared
group(2026-08-31). The throw was never a deliberate refusal of the key —DetailViewmapped every section throughs.fieldsunguarded,flatMapkept the resultingundefinedas an element (it flattens arrays;undefinedis not one), and the next line read.nameoff it. The declaration was right and the runtime had never honoured it, which is the verdict class the brief pointed at. Hence: implementgroup, do not make it a no-op.Resolution goes through
deriveFieldGroupDetailSections— the same adapter the synthesized default record page already uses, deliberately not a second derivation. That is what makes acceptance 4 true by construction rather than by coincidence.Blast-radius containment is two layers, because one cannot do it
The crash fires inside a
useMemoinDetailView's own body — above the section loop, outside every per-section subtree. A React error boundary catches what throws while rendering its subtree, so a per-section boundary alone could never have caught this one.sectionFieldEntries— one tolerant spelling of "every field entry named by every section", guarding the path that actually fired.SchemaErrorBoundary, pushing its granularity from per-component to per-section, bounding every other way a section can throw.An unresolvable
grouprenders nothing and is reported once on the console. Existence is not a parse question — the spec assigns it to@objectstack/lint'spage-section-group-unknown, andderiveFieldGroupLayoutalready drops a declared group nothing references — so dropping is the contract; vanishing without a word is not.Defect 2 — enumerated
fieldsreached only the i18n bundleDetailSectionrendersfieldLabel(objectName, field.name, field.label || field.name)— an i18n lookup whose third argument is the fallback. The ladder isbundle key -> field.label -> field.name, and a spec-shaped section enumerates bare strings, which became{ name }with nolabel. Rung 2 was missing entirely.withDeclaredLabelfills it from the bound object definition. Not a new source: it is the same read the group-derived body has always done (toField,label: f.label || name) — which is exactly why the platform's synthesized page showed declared labels while a hand-authored enumeration of the same fields did not.The caricature was run, not predicted — and it changed this PR twice
Caricature: swallow the throw and render nothing (the page stops crashing, the section silently disappears). Mutation proven on disk in both directions, restore proven by state (
git diff HEADempty and each blob hash equal to HEAD's).SECTION_GROUP_RENDERS_MEMBERS,BODIES_AGREESIBLINGS_SURVIVE,ONE_BAD_SECTION_IS_BOUNDED,HARNESS_KILL, all rung legsflatMapSECTION_WITHOUT_MEMBERS_IS_BOUNDED(with the originalCannot read properties of undefined (reading 'name'))ONE_BAD_SECTION_IS_BOUNDED,SECTION_WITHOUT_MEMBERS_IS_BOUNDEDRUNG_2_DECLARED_LABEL,RUNG_1_BUNDLE_WINS,BODIES_AGREE+ the three containment legs (they assert the sibling's label)RUNG_3_FIELD_NAME,AUTHORED_LABEL_WINS,SECTION_GROUP_RENDERS_MEMBERS,HARNESS_KILLgroupimplemented, a{ group }section is resolved into one carryingfieldsbeforeDetailViewsees it — so every group leg passed with the tolerant reader reverted.SECTION_WITHOUT_MEMBERS_IS_BOUNDEDwas added specifically to pin it, and it is the only leg that does.Each discriminating assertion was confirmed to execute in its own leg, by line number, not by assumption — e.g.
RUNG_1_BUNDLE_WINSreddens on its third assertion (the field the bundle does not cover still gets its declared label), proving the first two ran and passed rather than short-circuiting it.Harness-kill leg. Every leg navigates by
requireLiveBody, whose anchor is a section no leg is 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.Fixture triage — 9 pre-existing tests, three dispositions considered
Four files navigated rows by the raw field name, which is the behaviour defect 2 changes. All four are "change the spelling": their own object schemas already declared the labels, and their subjects (title dedupe, empty-section heuristic,
hideEmptyretirement) are untouched. Two comments that documented the missing rung as if it were the contract are rewritten rather than deleted, and the one negative assertion among them was re-pointed so it cannot pass vacuously against a string the page no longer renders. Scanned all four for leftovers with a lit control.Verification
packages/plugin-detailfull suitepackages/typesfull suiteplugin-detailtype-check(both programs)--listFilesproves the pin file is in the test program andrecord-details.tsxin the src program, so neither is NOT MEASUREDpackages/typestype-check(three programs)eslint .(no narrowing needed)check:control-byteschr().grep -naP '[\x{200b}]'fails outright on this container and, left of an&&, renders as a clean passcheck:changeset-presence/check:changeset-no-majorDeclared narrowings.
check:sdui-registration-pinsexits 2 with its own "this is not a pass — build the console first" message; left to CI, because the diff adds and removes no registration (only a description string). The remainingturbo --affectedpackages (calendar, gantt, kanban, runner, console, examples, site) are affected through the workspace graph, not through the code touched.objectui#7051 is open, unassigned, no linked PR, labelled
pm:on-hold, and is precisely this work: "Consume the spec section group-reference form …record:detailssections and viewform.sectionsresolvegroup:keys viaderiveFieldGroupLayout".record:detailshalf.form.sectionshalf is still open: measured on this branch,plugin-formreadsfield.group(field-to-group membership) but no form-section renderer readssection.group.@objectstack/*version carrying it" — was last measured ⛔ NO on 2026-09-03. It is satisfied today: the installed pin is@objectstack/spec17.3.0 and it declaresgroup: SectionGroupKeySchemaonRecordDetailsProps.sections[].⛔ Deliberately not using a closing keyword for #7051 here: this PR closes only one of its two halves. Re-measure and re-queue it for the rest.
Out of scope, filed
RecordDetailsComponentProps.sections[]omits six member keys the spec declares (columns,icon,description,showBorder,defaultCollapsed,headerColor) and declares one it refuses (collapsed). This PR moves only the two members it needs (fieldsto optional,groupadded). Found by the compile-time type pin — and notevitestwas green on the offending literal whiletscwas red.Not done
Per the card's boundaries: no browser re-drive against a current
@objectstack/console(the card's own measurements are the original reporter's), andhideEmptyis untouched.🤖 Generated with Claude Code
https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
Generated by Claude Code