Repository navigation
fix(app-shell): switching a report to joined clears the container binding it hides (objectui#10746) - #10762
Conversation
…ding it hides
Studio's report inspector committed `{ type: 'joined' }` alone and, in the same
render, hid its dataset / values / rows / columns / chart controls while the
spec's own `reportForm` hid the whole "Dataset binding" section (`order`
included) through `visibleWhen: "data.type != 'joined'"`. A report bound first
and switched second kept every one of those keys invisibly; `ReportSchema`'s
joined arm refuses a container `dataset` / `rows` / `columns` / `values`
(objectstack PR #20160) and has always refused a container `order`, so the
save was refused at a path no Properties-tab control could reach.
The type picker now drops `dataset`, `values`, `rows`, `columns`, `chart` and
`order` in the same patch that commits `type: 'joined'`, as `undefined`-valued
keys (the spelling `commitChart` and the sibling inspectors already clear
with); only keys the draft carries are named. `runtimeFilter` and `drilldown`
are kept (the joined branch reads both), switching between non-joined types
keeps the binding, switching away from `joined` restores nothing, and `blocks`
is never touched.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014mXUNuFomfj24w7s1pZzhN
✅ 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
|
Contract reviewServed-tier: ① Derived judgmentsHosts and the mechanism.
The first PUT is clean on all three paths. The clear does not survive the metadata-admin refresh. Completeness of the cleared set. Spec at objectstack main The Pins. Re-run in a detached worktree at head with Edges. joined to non-joined restores nothing: pinned and stated in changeset and PR body. Undo claim TRUE: Type-check ② Semver level
Published text against head: spec-version claims TRUE (installed and npm latest 17.4.0; os-main ③ Boundary flags
Implemented-by: VERDICT: FAIL What stops it, both wording, no code change owed to this card:
Everything else in ①–③ passes: mechanism correct for the committed patch on every host, cleared set complete against the spec at main, |
…e inspector hosts Round-1 contract review on the PR: wording only. The changeset now states the limit of the clear — it holds for the patch and the save that follows it, while the metadata-admin editor's draft-over-`layered.effective` rebuild (on load, after each save and after publish) brings a PUBLISHED binding's keys back into the draft until publish and does not repair a report already saved `joined` with stale keys (objectui#10765, the host merge, not this card's). The pin file's merge helper comment names the third host, `StudioDesignSurface`, beside `ResourceEditPage` and `ReportConfigPanel`. Prose and a comment only; no source line changes. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014mXUNuFomfj24w7s1pZzhN
✅ 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
|
Contract reviewServed-tier: ① Derived judgmentsThe delta Reach sentence. Changeset :25-31 and body H2 "Reach" bullet are, whitespace-normalised, identical to each other and to round 1's prescribed text up to the final clause, where "filed separately." became "objectui#10765." True at head: Three-host sentence. Body H2 begins with round 1's prescribed sentence verbatim ("in all three hosts … Leftover two-host phrasing. None in the replacement body (no "both hosts", "two hosts", "Neither"), the changeset, the pin file or the component's comments (:93-96 "The host spreads the patch", :357-359 names Round 2 lines and figures. "no source line changed — the diff's only non-changeset lines are the comment lines … Round-1 record read in full; its two requirements are met literally. Live body on GitHub is byte-identical to ② Semver level
③ Boundary flags
Implemented-by: VERDICT: PASS |
1 participant
Fixes #10746
Clause-②: no — a Studio inspector's patch on a type switch; no declared surface moves
What changed
packages/app-shell/src/views/metadata-admin/inspectors/ReportDefaultInspector.tsx: the Report type picker's commit is nowcommitType. When the picked type isjoined, the patch that commitstype: 'joined'also carriesdataset,values,rows,columns,chartandorderasundefined-valued keys — only those the draft actually holds (draft[key] !== undefined). Any other type commits{ type }alone, as before. The list lives inJOINED_CONTAINER_CLEARED_KEYSwith the reasoning beside it;blocks,runtimeFilteranddrilldownare never named.orderis a sixth key, beyond the card's five, under the role file's bounded in-place exemption (same defect class, mechanical, same file, same gate family, no other claim on the file): the spec's ownreportFormhidesorderin the same "Dataset binding" section (visibleWhen: "data.type != 'joined'"), and the INSTALLED spec already refuses a containerorderon a joined report ("ajoinedreport orders per block — moveorderontoblocks[]"), so the same invisible-key save refusal exists today fororder. Evidence in H3 below.New pin file
ReportDefaultInspector.joinedClearsBinding-10746.test.tsx(8 tests); changeset.changeset/10746-joined-report-clears-binding.md('@object-ui/app-shell': patch).H1 — reproduction on
origin/main(1422a920ed)The pin file run against the untouched component:
Tests 4 failed | 4 passed (8). The defect pin quotes the onlyonPatchargument the type picker commits today:AssertionError: expected { type: 'joined' } to strictly equal { type: 'joined', …(6) }— the commit is{ type: 'joined' }and nothing else. The 4 pins green on base are the boundary/control pins, which pin what must NOT change.H2 — the clear mechanism
onPatchis a shallow patch in all three hosts:ResourceEditPageapplies it ashandleDraftChange((d) => ({ ...d, ...patch })),ReportConfigPanel.handlePatchas{ ...draftRef.current, ...patch }, andStudioDesignSurface.onPatchassetDraft((d) => ({ ...d, ...patch })); each saves throughclient.save, which isJSON.stringify. None offers a delete sentinel, so the one way to drop a key is anundefined-valued key — the spelling this inspector's owncommitChart(chart: next.type ? next : undefined) and the siblings (ActionDefaultInspector,ObjectDefaultInspector,DatasetDefaultInspector's "Clear all", pinned by its objectui#9372 suite) already use. Measured in the second pin: after the spread the key is an OWN property holdingundefined(Object.hasOwntrue, valueundefined— notnull, not an empty string), andJSON.parse(JSON.stringify(committed))— the shapeclient.saveputs on the wire and the spec parses — has no such property. The spec's refinement itself skipsundefined(if (value === undefined || …) continue;in the joined arm), so even the in-memory draft would not be refused.layered.effective(on load, after each save and after publish), andeffectiveis the published layer, so a report whose PUBLISHED version was bound gets those keys back in the draft after the first draft save until it is published, and a report already savedjoinedwith stale keys is not repaired on load. Both are the host's draft-over-baseline merge, objectui#10765.H3 — the spec check
Installed
@objectstack/specis 17.4.0 (packages/app-shell/node_modules/@objectstack/spec). It PREDATES objectstack-ai/objectstack#20160:grep 'selects per block'over itsdist/gives 0 hits; the controlgrep 'orders per block'gives 3. One-off probe with the installedReportSchema.safeParse(not committed):dataset/values/rows/columns/chart: success — 17.4.0 accepts themorder: refused,customat['order'], "ajoinedreport orders per block — moveorderontoblocks[]."blocks: []: refused at['blocks']runtimeFilter+drilldown: false: successdataset: undefinedas an own key: successSo the pins' parse leg measures the
orderhalf with the installed spec (before the fix: refused at['order']; after: parses), and for the four selection keys the pins assert ABSENCE and cite the rule read at objectstackorigin/mainpackages/spec/src/ui/report.zod.ts:JOINED_CONTAINER_SELECTION_KEYS = ['dataset', 'rows', 'columns', 'values'], onecustomissue per present key atpath: [key], message "ajoinedreport selects per block — moveKEYontoblocks[], or delete it; on the container it selects nothing." (KEYstands for the key's own name.) No checkout on this box holds a built specdistcarrying that refusal, and building one was outside this card.H4 — edges
joined→ non-joined: nothing is restored; the patch is{ type }alone and the author re-binds (pinned).{ type }alone (control, pinned).joinedwithblocks[]present: untouched, same array reference, in both directions (pinned).ResourceEditPage'sUndo2button isdoDiscardDraft(ADR-0034: discard the whole pending draft), not a per-edit undo, andhandleDraftChangekeeps no history. One undo cannot restore the keys; switching the type back does not either.runtimeFilteranddrilldownsurvive the switch (pinned): the joined branch reads both.ReportConfigPanel.onFieldChangesees no phantom clears, and an unbound report's switch stays{ type: 'joined' }(pinned).Pins and ablations
Head
3f53c15776, pin file plus the two existingReportDefaultInspectorsuites:Test Files 3 passed (3) · Tests 37 passed (37).Red on base (
1422a920ed, component untouched, pin file present):4 failed | 4 passed— the defect pin, the own-key/serialised pin, the parse leg, the partially-bound pin. The other 4 are controls and boundaries, green on base by construction.Per-hunk ablations on the committed head, each through objectstack's
scripts/ablation-replace.mjs(anchor must hit exactly once; the blob change is verified on disk; restore provenblob == HEADa9cb7e9candgit diff HEADempty), the prediction written before each run, observed direction = predicted:joinedguard (if (nextType === 'joined')becomesif (typeof nextType === 'string')): predicted CONTROL F red; observed1 failed | 7 passed(CONTROL: non-joined → non-joined keeps the binding).if (draft[key] !== undefined) patch[key] = undefined;becomes unconditional): predicted D and E red; observed2 failed | 6 passed(names only present keys; unbound one-key patch). The first attempt was a NO-OP the tool refused — the replacement text was a substring of the anchor, so its on-disk count could not rise (1 to 1, a rise of 0) — and it restored; the leg was re-run with a distinct replacement.blocksto the cleared list: predicted C and G red; observed2 failed | 6 passed(parse leg: the joined report loses its blocks; CONTROL: blocks untouched).orderfrom the list: predicted A, B and C red; observed3 failed | 5 passed.Assertion spelling: every patch is read with
toStrictEqual, becausetoHaveBeenCalledWith/toEqualtreat anundefined-valued key as absent and would read the defect and the fix alike.Round 2 (
d4945789f6, the contract-review wording round: the changeset's reach sentence and the pin file's three-host comment; no source line changed — the diff's only non-changeset lines are the comment lines shown bygit diff -U0, andReportDefaultInspector.tsxis untouched): pin file re-runTests 8 passed (8).Gates (local, derived by hand from
package.jsonand the workflows — objectui has nodispatch-gates.mjs)turbo run build --filter='@object-ui/app-shell^...' --concurrency=2:Tasks: 28 successful, 28 total, 2m12s, under the verify lock.pnpm --filter @object-ui/app-shell type-check(tsc --noEmit && tsc -p tsconfig.test.json): exit 0;tsc -p tsconfig.test.json --listFileslists the pin file (1 hit among 4673 files).node scripts/check-changeset-presence.mjs: ✅ (round 2: 2 source files of 1 released package changed, 1 changeset declared).node scripts/check-changeset-no-major.mjs: ✅.pnpm check:changeset-claims: ✅.pnpm check:pending-changeset-literals: ✅.pnpm check:control-bytes: ✅ OK (round 2: 8942 tracked text files). The role file's control-byte grep over the changed files: no match.pnpm check:new-line-citations:0 new citation(s)(both rounds).pnpm check:test-path-roots: ✅ OK.pnpm check:vi-mock-specifiers: ✅ OK..tsxfiles: exit 0.pnpm lint(repo-wide), the 8 test shards,Build & E2E,check:i18n-*(not()key was added or changed). Round 1 CI on3f53c157: 40 success, 3 skipped, 0 failed per the contract review.Serial
Round 2:
origin/mainat25c7d584e4(+5 commits over BASE1422a920ed: #10752, #10753, #10708, #10751, #10761); none touches the three files;git merge-tree --write-treeexits 0 (clean, tree738f2bf534). No merge commit was needed.Acceptance notes
chartis cleared per the triage direction and [finding] a joined report'schart— container andblocks[].chart— parses and is never drawn, while the liveness ledger names the joined branch as its reader objectstack#20161 (inert on a joined container); the installed spec does not refuse it, so its clearing is pinned by absence only.git fetch origin mainran against the shared checkout (the role-file recipe) rather than inside the worktree as the order asked;origin/maindid not move (1422a920edbefore and after), only the shared checkout'sFETCH_HEADwas touched. Every later fetch used a private ref inside the worktree.ReportConfigPanelhosts this inspector too, merges with the same spread and emitsonFieldChange(key, undefined, next)once per cleared key — the documented "field changed" signal, correct for a live preview.Generated by Claude Code