Skip to content

refactor(objects): say collapse: 'collapsed', not the deprecated defaultExpanded alias - #1843

Merged
os-steve merged 1 commit into
mainfrom
claude/issue-1835-fieldgroup-collapse-canonical
Sep 10, 2026
Merged

os-steve merged 1 commit into
mainfrom
claude/issue-1835-fieldgroup-collapse-canonical

Conversation

@claude

@claude claude Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Description

Every fieldGroups collapse hint in src/objects/*.object.ts was authored with the
ADR-0085-deprecated defaultExpanded: false alias — 25 sites across 13 objects, and
zero use of the canonical collapse enum anywhere in the repo. All 25 now write
collapse: 'collapsed'.

This is a spelling change on the authoring surface. No group was added, removed,
reordered or re-labelled, and no group changed whether it starts open or closed.

Type of Change

  • Code refactoring

Related Issues

Fixes #1835

Changes Made

  • 25 rewrites of defaultExpanded: false to collapse: 'collapsed' in the 13
    src/objects/*.object.ts files that carried the alias: account campaign case
    contact contract event forecast knowledge_article lead opportunity
    product quote task.
  • One empty-frontmatter changeset declaring that this PR releases nothing.
  • Nothing else. git diff --stat against the base is 13 object files at 25 insertions /
    25 deletions, plus the changeset.

No code comment needed updating: no comment anywhere in the repo taught the retired
spelling (grep -rniE 'expand|collaps' over src/objects/*.object.ts returns only
whitespace-collapsing prose, unrelated to this key).

The equality proof

The card's claim is equality, so equality is what is measured — green CI would not
have shown it.

1. The mechanism, read off the installed dist

In the pinned @objectstack/spec@17.4.0, deriveFieldGroupLayout (ADR-0085 §5) is the
single place a group's collapse behaviour is resolved for the form and
synthesized-detail renderers. Its readGroup reads the canonical key first:

let collapse = "none";
if (grp.collapse === "expanded" || grp.collapse === "collapsed" || grp.collapse === "none") {
  collapse = grp.collapse;
} else if (typeof grp.collapsible === "boolean" || typeof grp.collapsed === "boolean") {
  collapse = grp.collapsed === true ? "collapsed" : grp.collapsible === true ? "expanded" : "none";
} else if (typeof grp.defaultExpanded === "boolean") {
  collapse = grp.defaultExpanded ? "expanded" : "collapsed";
}

ObjectSchema.parse / safeParse normalize the same way ahead of the base parse, via
normalizeSemanticRoleAliases, which short-circuits on if (grp.collapse != null) return g;.
So an authored collapse: 'collapsed' lands on exactly the value defaultExpanded: false
was being folded into — it does not merely produce an equivalent, it takes the same branch's
output.

2. The derived layout is byte-identical

Every object was parsed through the pinned ObjectSchema (each file already calls
ObjectSchema.create, which parses) and run through deriveFieldGroupLayout, on both
revisions. Coverage is all 18 objects that declare fieldGroups, not just the exemplar:
83 derived sections — 81 declared groups plus the two synthesized ungrouped sections on
crm_case and crm_task — of which 25 are collapsed and 58 none.

$ diff before.json after.json ; echo "DIFF_EXIT=$?"
DIFF_EXIT=0
$ sha256sum before.json after.json
c58c2130b768732e408f584ab06a653d8b9051e3f43deb098ef626df672c59d3  before.json
c58c2130b768732e408f584ab06a653d8b9051e3f43deb098ef626df672c59d3  after.json

The crm_contact.mailing_address exemplar the finding used in the browser derives
{ key: 'mailing_address', label: 'Mailing Address', icon: 'map-pin', collapse: 'collapsed', fields: [mailing_street, mailing_city, mailing_state, mailing_postal_code, mailing_country] }
on both sides, identically.

3. The probe is load-bearing — negative control

A probe that reported equality because it never read the value would be worthless, so it
was proved able to fail. One site was mutated to collapse: 'expanded' after the
implementation was committed, the mutation was verified on disk by content (not by the
editor's exit code, which is 0 on zero matches), and the probe was re-run:

HEAD_BLOB=41f5d440a3702a106c78468bbba80404684f840c
mutation_injected_count=1  original_line_remaining=0
MUT_BLOB=1a281f40d6abbfda9a383c09a020ab56529e48ef
NEGATIVE_CONTROL_DIFF_EXIT=1  (want NON-zero: the probe must SEE the mutation)
398c398
<         "collapse": "collapsed"
---
>         "collapse": "expanded"
450c450
<         "collapse": "collapsed",
---
>         "collapse": "expanded",
RESTORED_BLOB=41f5d440a3702a106c78468bbba80404684f840c
git_diff_HEAD_after_restore=''
NEGATIVE_CONTROL: PASS

The restore leg is git checkout HEAD -- path (never the bare form, which would take the
mutation back out of the index) and is proved by the blob hash matching HEAD and by
git diff HEAD being empty — not by a clean-up step's exit code.

4. The built artifact — the strongest reading

pnpm build was run on both revisions and the two dist/objectstack.json artifacts
(2.0 MB each) were compared key-sorted. The entire difference is 25 lines:

$ grep -E '^[<>]' artifact.diff | sed 's/^. *//' | sort | uniq -c
     25 "defaultExpanded": false,

Every "collapse" value is present and identical on both sides — 25 collapsed,
56 none, 0 expanded. So the shipped metadata does change in one respect and only one:
the now-redundant deprecated alias key is no longer emitted. Nothing a renderer reads
moved.

That is also why the changeset frontmatter is empty rather than a patch: nothing new
reaches a user.

Why the key is rewritten and never deleted

collapse defaults to 'none' — always open, no toggle — not to 'expanded'. A
group that declares no collapse key is therefore not the same as defaultExpanded: false,
and "tidying" the key away would have been a real UX change disguised as a simplification.
Every site is rewritten; none is dropped.

Testing

  • Unit tests pass
  • Linting passes
  • Build succeeds
  • New tests added — not applicable, see below

pnpm verify is green end to end on f36ce0d, and each stage was additionally run on its
own so the exit codes are separable rather than hidden behind the chain's &&:

stage exit wall
pnpm validate 0 4s
pnpm typecheck 0 4s
pnpm lint 0 4s
pnpm lint:i18n-gate 0 3s
pnpm hygiene 0 1s
pnpm hygiene:tokens 0 0s
pnpm build 0 3s (locked)
pnpm test 0 113s (locked)
pnpm verify (whole chain) 0 135s (locked)

Test counts are unchanged, measured by running the suite on the base commit 62e23f5 and
on f36ce0d:

before (62e23f5)  Test Files  165 passed (165)   Tests  3454 passed | 1 skipped (3455)
after  (f36ce0d)  Test Files  165 passed (165)   Tests  3454 passed | 1 skipped (3455)

No test was added. Nothing in this diff is a behaviour to pin: the canonical spelling is
already enforced by the platform's own schema, and a repo-local guard forbidding an alias
the platform still accepts would be exactly the gate farm AGENTS.md Scope rule 3 rules out.
The 8 author-time build warnings are unchanged from the base and none of them concerns
field groups.

Not browser-verified. The finding's open question was whether the pinned console honours
collapse on the synthesized detail path as well as the alias; that is answered above at
the layer which decides it — both spellings resolve through the same readGroup branch
into the same derived value, and the built artifact confirms no renderer input changed.

Checklist

  • I have added a changeset — empty-frontmatter, the sanctioned "releases nothing"
    declaration documented in .github/workflows/changeset-check.yml:33, precedent test(i18n): point the #802 term guard's pack rule at the real zh-CN tree #1778
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My changes generate no new warnings
  • New and existing unit tests pass locally with my changes

Additional Notes

Out of scope and deliberately untouched: FormSectionSchema's own collapsible /
collapsed keys on form view sections in src/views/*.view.ts (a different schema,
not aliases of anything), and defaultExpandedDepth (a different key, on a tree
component). Zero occurrences of defaultExpanded remain anywhere in the repo outside
node_modules.


🤖 Generated with Claude Code

https://claude.ai/code/session_01DuzfS5chho38Yx1jxx9DEj


Generated by Claude Code

…efaultExpanded` alias

Every `fieldGroups` collapse hint in `src/objects/*.object.ts` was authored with
the ADR-0085-deprecated `defaultExpanded: false` alias — 25 sites across 13
objects, and zero use of the canonical `collapse` enum anywhere in the repo.
Rewrite all 25 to `collapse: 'collapsed'`.

The rendered output cannot move, and that is measured rather than assumed.
`deriveFieldGroupLayout`'s `readGroup` (ADR-0085 §5) resolves the canonical
`collapse` first and only then falls back to `collapsible`/`collapsed` and
`defaultExpanded`; `ObjectSchema.parse` normalizes the same way ahead of the
base parse, short-circuiting on `grp.collapse != null`. Two measurements on the
pinned @objectstack/spec@17.4.0:

- the derived layout for all 18 objects that declare `fieldGroups` — 83
  sections, 25 `collapsed` and 58 `none` — serializes to the same SHA-256
  before and after;
- the built `dist/objectstack.json` differs in exactly 25 lines, every one of
  them the removal of a now-redundant `"defaultExpanded": false`. All
  `"collapse"` values are identical on both sides.

The key is rewritten and never deleted: `collapse` defaults to `'none'` (always
open, no toggle), not `'expanded'`, so dropping it would be a real UX change
rather than a simplification.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DuzfS5chho38Yx1jxx9DEj
@vercel

vercel Bot commented Sep 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
hotcrm Ignored Ignored Sep 10, 2026 4:42am UTC

Request Review

@github-actions github-actions Bot added the metadata Declarative metadata — schema, security posture, UI surfaces label Sep 10, 2026
@os-steve
os-steve added this pull request to the merge queue Sep 10, 2026

Copy link
Copy Markdown
Collaborator

PM review → accepted, auto-merge armed

Checked against the branch and the diff, ⛔ not against the report's self-description. CI 9 of 9 green on f36ce0d.

The diff is the card and nothing else. 14 files: the 13 named object files at 25 insertions / 25 deletions, plus the changeset. I read every hunk — each one is the alias swapped for the canonical key on the same line, with the label, icon, key and array position untouched. Counted per file it comes to exactly 25 (account 2 · campaign 1 · case 2 · contact 3 · contract 1 · event 1 · forecast 1 · knowledge_article 1 · lead 5 · opportunity 2 · product 1 · quote 2 · task 3).

The equality claim is proved at three independent layers, and the middle one is the reason I believe the other two.

  1. Mechanism — readGroup reads canonical first, aliases second; normalizeSemanticRoleAliases short-circuits on grp.collapse != null. Same branch, not merely an equivalent one.
  2. ⭐ Negative control — the probe was mutated to collapse: 'expanded' after the implementation was committed, the mutation proved on disk by content rather than by the editor's exit code, and NEGATIVE_CONTROL_DIFF_EXIT=1 with the diff naming both the parsed and the derived reading. Without this leg, DIFF_EXIT=0 would have been worth nothing — a probe that reads nothing also reports equality. This is the leg that most reports skip.
  3. Built artifact — the entire difference across a 2 MB dist/objectstack.json is 25 removed "defaultExpanded": false, lines, with "collapse" identical on both sides. That is the strongest available statement of "nothing a renderer reads moved", and it is also why the empty-frontmatter changeset is honest here: the artifact does change, in exactly one way, and that way reaches no user.

Coverage was widened beyond the dispatch, correctly. I asked for the crm_contact.mailing_address exemplar; the run covered all 18 objects that declare fieldGroups — 83 derived sections including the two synthesized ungrouped ones on crm_case and crm_task — and hashed the lot. Broader than asked, at no cost to reviewability.

The trap was respected. 25 rewrites, 0 deletions. The PR body states the reason in the right form (collapse defaults to 'none', which is always-open-no-toggle and is not what the alias meant), so the next author meets the reasoning rather than rediscovering it.

The two "nothing to do here" calls are both right, and both are stated with their evidence rather than as assertions: no comment taught the retired spelling (grep -rniE 'expand|collaps' over the object files returns only unrelated whitespace prose), and no test was added — a repo-local guard forbidding an alias the platform still accepts is precisely the gate farm AGENTS.md Scope rule 3 rules out. ⭐ Declining to add a test and saying which rule makes that the right answer is the part that keeps this repo's suite honest.

Auto-merge armed (squash). #1835 closes on merge; nothing further needed from you.


Generated by Claude Code

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

Labels

metadata Declarative metadata — schema, security posture, UI surfaces

Projects

None yet

2 participants