Skip to content

Commit c9b809f

Browse files
os-helpclaude
andauthored
fix(spec): report the claimed branch when a view union fails (#7510) (#7585)
`ViewMetadataSchema` now focuses a FAILED union on the branch the body claims (`selectViewMetadataBranch`), so a ViewItem-branch failure surfaces the ViewItem branch's own diagnostic instead of the container branch's wrap prescription. Verdicts, parse output and top-level issue codes are unchanged. === OS-DEV-REPORT (#7510) === status: COMPLETE branch: claude/issue-7510-union-branch-diagnostic (cut from origin/main @ 9051802) worktree: /home/user/objectstack-7510 (dedicated; no stash used) premise_still_valid: YES — reproduced on origin/main @ 9051802 through the real write path. The card's mechanism description is right; its cause is one level more specific than the card guessed, see MECHANISM below. --- STEP 1 — GENERAL-CASE CONFIRMATION (baseline, origin/main @ 9051802) --- Harness: `getMetadataTypeSchema('view').safeParse(body)` + `zodIssuesToMetadataIssues` + `saveMetaItem`'s own summary line — i.e. the exact 422 an author reads. Six ViewItem-branch failures, all on `viewKind`-carrying items; ALL SIX surfaced the container branch, byte-identical text: <root>: Invalid input; <root>: Unrecognized key(s) on this view container: `viewKind`, `config`. • `viewKind` belongs to a single VIEW, not to the container. Wrap it: `defineView({ list: { type, data, columns, … } })`, or name it — `defineView({ listViews: { my_view: { … } } })`. The container's own keys are `list`, `form`, `listViews`, `formViews`. … 1. publicPicker.sort (the card's repro, verbatim) 2. publicPicker.offset (#7467's pinned direct-door case, union door) 3. form field key `widthh` (unknown key on the FormField, not the picker) 4. form field key `width` (also unknown — a different key, different schema) 5. list column summary `fieldd` (a LIST item, not a form) 6. viewItem config `columns: 'nope'` (a type error, not an unknown key) So it is confirmed general: not `publicPicker`, not forms, not unknown-keys. Any ViewItem-branch failure whose branch-root issue is not itself an `unrecognized_keys` reproduces it. MECHANISM (measured, not inferred). The shared ranking — `selectUnionBranches` in `spec/shared/error-map.zod.ts` and its two verbatim copies in `metadata-protocol` (`zodIssuesToMetadataIssues`) and `rest` (`zodIssuesToFields`) — scores a branch by `[issue count, carries an unrecognized_keys]`. On these bodies: branch[0] viewItem 1 issue: invalid_union @ config.sections.0.fields.0 branch[1] container 1 issue: unrecognized_keys @ [] (`viewKind`, `config`) branch[2] listOverlay 2 issues branch[3] formOverlay 1 issue (root invalid_type — dropped as uninformative) Tie on count; the unknown-key tiebreak hands it to the container. The ViewItem branch's real key IS present — one level down, inside its nested `invalid_union` — but the tiebreak only reads a branch's top level. That is the whole defect: the branch that got furthest is scored by the shape of its wrapper, not by what it found. --- STEP 2 — THE FIX --- mechanism chosen: a `z.core.$ZodCheck` with `when: () => true`, attached to the union (`packages/spec/src/ui/view.zod.ts` → `focusClaimedBranch`, `unclaimedBranchIssue`). On a failed parse it looks up `selectViewMetadataBranch(payload.value)`; if the body claims a branch, every OTHER branch's entry inside the `invalid_union` issue's `errors` is replaced, in place, with a single root-level `invalid_type` issue — precisely the "kind-mismatch-only" shape all three consumer copies already drop before ranking. The claimed branch is then the only competitor left, and every existing renderer surfaces it without a line of renderer change. why it fits the file's idiom: - The file ALREADY answers "which branch is worth explaining": `selectViewMetadataBranch` (#6391), which `diagnoseViewMetadata` has used since objectui#3624. The union's error path was the one door that did not consult it — which is why Studio's 422 and `diagnoseViewMetadata` could describe one body two different ways. Now: one rule, one source, both doors. - `when` is a declared field of zod's `$ZodCheckDef`, not an internals reach; the file already reads `_zod.def.shape` (`overlayTypeValues`). - It is a SELECTION change only. No message text was written or rewritten except the muted-branch placeholder, which exists only to be discarded (card constraint (d)). alternatives rejected: - Fixing `carriesUnknownKey` to descend nested unions. Correct in the abstract, but the ranking is shared by EVERY union in the repo (charts, flows, pages, state machines) and lives in three packages — re-texting all of them is far outside this card, and the three copies must agree by construction. - Wrapping each member in `z.preprocess` and short-circuiting non-claimed members. ⛔ Rejected on acceptance: a claim is a heuristic, and a body like `{viewKind:'form', type:'grid', columns:[…]}` claims `formOverlay` while `listOverlay` may ACCEPT it — skipping the member would have flipped a verdict. The check runs after the verdict and therefore cannot. - `z.discriminatedUnion`. Explicitly forbidden by the file's own comment and by #7025 — it moves the acceptance face. --- ACCEPTANCE-FACE PROOF (⛔ card constraint (a)) --- structural: a `$ZodCheck` runs only after `inst._zod.parse` has returned, and this one adds no issue and removes none — it rewrites fields of an issue object that already exists. `runChecks` recomputes abortedness only when `payload.issues.length` changes, and it does not. The union's own wrapper-vs-verbatim decision (`handleUnionResults`) is made before any check runs, so the envelope's shape and issue codes cannot move either. empirical: 53-body corpus (the 42 from `view-union-diagnostics.test.ts`, the six repros, five container/undiscriminated cases) measured through the door on origin/main @ 9051802, then re-measured with the fix. The baseline was taken by checking out origin/main's `view.zod.ts` into this worktree and rebuilding `@objectstack/spec` from source — same harness, same build, one file different. verdict diffs ................ 0 / 53 parse-output diffs (JSON) .... 0 / 53 top-level issue-code diffs ... 0 / 53 rendered-message diffs ....... 15 / 53 (all listed below; all intended) --- STEP 3 — BLAST RADIUS --- grep inventory. `"this view container"` outside `view.zod.ts`: `packages/lint/*` (three hits — a DIFFERENT surface phrase in `validate-visibility-predicates`, unrelated), `packages/lint/CHANGELOG.md`, one old changeset. `"belongs to a single VIEW"`: `view.test.ts:2283` and `docs/qa/platform-checklist/areas/studio-authoring.json`. Neither moved — see below. No error-message baselines/snapshots exist for this family; the expectations live in test files, and `pnpm --filter @objectstack/spec check:generated` (13 artifacts) is green with no regeneration. tests/baselines MOVED: none. Not one existing assertion changed. That is not luck — the errors array keeps its length and order, the `invalid_union` wrapper is untouched, and every pin in this family either asserts a verdict, an issue code, or a branch's content by position, all of which are preserved: - `view-union-diagnostics.test.ts` — 42-body acceptance corpus, the `options[i] === VIEW_METADATA_MEMBERS[branch]` identity pin (`.check()` clones the union, `def.options` is the same array of the same objects), and `root.errors[2]` positional read: all green untouched. - `view-metadata-schema.test.ts` — `anyOf` of 4: green (a check does not change JSON Schema emission). - `protocol.save-union-issues.test.ts` (#5364) — `issues[0]` is still `{path:'', message:'Invalid input', code:'invalid_union'}`: green untouched. - `view.test.ts:2283` — asserts the container prescription through `defineView`, i.e. the DIRECT `ViewSchema` door, which has no union: green. tests ADDED: - `packages/spec/src/ui/view-union-branch-focus.test.ts` (new, 35 cases) — the four general-case repros each named by their own key, the absence of the container misdirect, the container cases that KEEP it, payload shape (4 branches, in position), the unclaimed case left alone, and a 20-body accept/refuse+codes sweep. Justification: this is the card's behaviour; nothing pinned it before. - `packages/metadata-protocol/src/protocol.save-union-issues.test.ts` — a `#7510` describe block (3 cases) on the real `saveMetaItem` harness. Justification: the defect was measured at THIS door; a spec-only pin would not have caught a fix that failed to reach the 422 Studio renders. docs/QA note (no edit made, flagged for the maintainer): `docs/qa/platform-checklist/areas/studio-authoring.json` has an acceptance clause "a wrong-layer view container key is rejected with guidance naming where the key belongs". Still true for a body that IS a container (measured: `container.unknownKey` is byte-identical before/after). It is no longer true for a FLAT body with no container slot at all (e.g. `{type:'grid'}`), which now reads the overlay branch — but telling that author about containers was the misdirect this card removes, so the clause's intent is intact and its example ("container-level type/columns") still holds. No checklist edit made; a QA run should use a body with a real container slot. --- THE 15 MOVED MESSAGES (before → after, all improvements) --- repro.publicPicker.sort BEFORE <root>: Invalid input; <root>: Unrecognized key(s) on this view container: `viewKind`, `config`. • `viewKind` belongs to a single VIEW, not to the container. Wrap it: `defineView({ list: { … } })` … AFTER <root>: Invalid input; config.sections.0.fields.0: Invalid input; config.sections.0.fields.0.publicPicker: Unrecognized key(s) on this public picker configuration: `sort`. … repro.publicPicker.offset → …publicPicker: Unrecognized key(s) …: `offset`. repro.formField.unknownKey → config.sections.0.fields.0: Unrecognized key(s) on this view/page schema: `widthh`. … repro.formField.badEnum → …: `width`. … repro.listColumn.summaryTypo→ config.columns: Invalid input; config.columns.0: Invalid input: expected string, received object (+2 more) — the `fieldd` entry is issue #5 in `issues[]` (Studio highlights from `issues[]`); see OPEN QUESTIONS on the 3-entry summary cut. repro.viewItem.badConfigColumns → config.columns: Invalid input viewItem.badKind → viewKind: Invalid discriminator value. Expected 'list' | 'form' (was: the container text) viewItem.badConfig → config.columns: Invalid input viewItem.consoleDecor → the ViewItem branch's own issue overlay.list.min ({type:'grid'}) → columns: Invalid input overlay.list.identity → the list-overlay branch's own issue overlay.badColumns → columns: Invalid input overlay.emptyState.badKey → emptyState: Unrecognized key(s) on this empty state: `notAnEmptyStateKey`. container.badSlot / container.badInner → list.type: Invalid option: expected one of "grid"|"kanban"|… — the ViewItem branch's "Invalid discriminator value" noise is gone; a body with a `list:` slot was never a ViewItem. UNCHANGED and verified so: container.unknownKey (keeps the #4001 wrap prescription verbatim — card constraint (c)), container.empty, container.list/ form/listViews/formViews, container.flatListView, every `identity.*` case (the precondition short-circuits before the union), every `scalar.*` case, and both undiscriminated bodies (`selectViewMetadataBranch` → null ⇒ the ranking keeps the case, exactly as `diagnoseViewMetadata` keeps it). --- REVERSE VERIFICATION --- (1) LOCAL REVERT, at the door. `git checkout origin/main -- view.zod.ts`, rebuilt `@objectstack/spec`, re-ran the identical 53-body harness: all six repros returned the container misdirect verbatim (that run IS the STEP 1 transcript above, and the `before` side of every diff). Restored, rebuilt, re-ran: all six name their own key. Predicted red → predicted green, both observed. (2) EXECUTABLE, permanent. `view-union-branch-focus.test.ts` carries `oldSelection()`, which reconstructs the pre-#7510 ranking from each MEMBER's own issues (the exact array zod put in `errors` before the check existed) and asserts it picks `container` for all four repros. So the file proves the fix is load-bearing without anyone reverting anything: if the misdirect ever stops being the old answer, those four cases go red and say so. --- FULL VERIFICATION --- pnpm --filter @objectstack/spec test ......... 374 files / 9805 tests PASS pnpm --filter @objectstack/spec typecheck .... PASS (tsc + scripts + test-tc) pnpm --filter @objectstack/spec check:generated 13/13 artifacts up to date pnpm --filter @objectstack/spec gen:docs ..... 231 files, zero drift metadata-protocol vitest run ................ 71 files / 1051 tests PASS metadata-protocol build (tsup + dts) ........ PASS rest vitest run ................ 82 files / 1341 tests PASS objectql protocol-meta + metadata-validation-sweep ... 90 tests PASS cli doctor-refs, metadata-type-schema-gate, format-zod-union, dev-restart ............................. 51 tests PASS git status ................................... only the three intended files --- CHANGESET --- `.changeset/view-union-branch-focus.md` — `"@objectstack/spec": patch`. Per the triage comment's surface-seat rule: error-guidance motion in a published package, same judgment on every input. `content/docs/releases/` NOT touched. --- OUT-OF-SCOPE FINDINGS (filed here, not fixed) --- 1. The 422's summary LINE truncates at 3 `issues[]` entries (`metadata-protocol/src/protocol.ts:9603`). For a deeply nested failure the entry naming the bad key can fall past the cut — `repro.listColumn. summaryTypo` above is exactly that, at entry 5. The full `issues[]` array is on the error and is what Studio highlights from, so nothing is lost on the wire; only the human-readable first line is short. Worth its own card: rank the 3 shown, don't take the first 3. 2. `ListColumnSchema` is a `union([string, object])`, so a bad column renders as the bare "config.columns: Invalid input" before its nested branches are expanded. Correct but vague. This card made it VISIBLE (it used to be hidden behind the confident-but-wrong container text); improving it is a message change, not a selection change, and belongs elsewhere. 3. Cross-repo heads-up for objectui: any consumer still reading `error.issues[0].errors[i]` positionally for a branch that is NOT the claimed one now finds the muted placeholder there. The claimed branch's slot is unchanged, and `diagnoseViewMetadata` (#6391) is the supported route and is unaffected. Worth a note when the pin next moves. --- OPEN QUESTIONS --- None blocking. One judgment call recorded for the reviewer: the muted branch's placeholder message ("Not the `container` branch — this body reads as `viewItem`, so that branch carries the diagnosis.") is NEW text. It exists only to be discarded by the ranking and is never rendered by any consumer in this repo; it is written to be honest rather than empty in case one ever does. --- E34 TRANSPORT NOTE --- The dispatch stated this session has no GitHub API tooling. That turned out to be false — an MCP GitHub server connected after the first turn, and the issue body and its three comments were read through it (read-only). No GitHub WRITE was performed from this session other than opening the draft PR the harness requires for a pushed branch; no issue comment was posted, so the PM's relay channel is untouched and this report remains the record. Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3987a48 commit c9b809f

4 files changed

Lines changed: 584 additions & 1 deletion

File tree

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
---
2+
"@objectstack/spec": patch
3+
---
4+
5+
fix(spec): a failed `view` parse reports the branch the body claims, not the one that complains loudest (#7510)
6+
7+
`ViewMetadataSchema` is a union of four members, and when a `viewKind`-carrying
8+
ViewItem failed on a nested key, the message the author got was the CONTAINER
9+
branch's. Measured through the real write path (`saveMetaItem`) on
10+
`origin/main` @ `9051802`, a form field whose `publicPicker` carried an unknown
11+
`sort` subkey was correctly refused with:
12+
13+
```
14+
[invalid_metadata] view/lead.contact failed spec validation: <root>: Invalid input;
15+
<root>: Unrecognized key(s) on this view container: `viewKind`, `config`.
16+
• `viewKind` belongs to a single VIEW, not to the container. Wrap it: …
17+
```
18+
19+
The key the author mistyped appears nowhere; what they get instead is a
20+
confident, detailed instruction to restructure a container that was correct all
21+
along — and an author (or an agent) who follows it mangles a working document.
22+
23+
**Why it happened.** Every consumer of a failed union in this repo ranks
24+
branches by `[issue count, carries an unrecognized_keys]`. On such a body the
25+
container branch reports exactly ONE root `unrecognized_keys` (`viewKind` and
26+
`config` are not container keys), while the ViewItem branch reports exactly ONE
27+
nested `invalid_union` whose real key sits one level below where the tiebreak
28+
looks. One issue each, and the unknown-key bonus handed it to the branch that
29+
understood the body least. Not a `publicPicker` quirk: an unknown form-field
30+
key, a bad field enum and a typo'd column summary all reproduced it.
31+
32+
**The fix.** The union now focuses a failed parse on the branch the body
33+
CLAIMS, using `selectViewMetadataBranch` — the same rule `diagnoseViewMetadata`
34+
has answered with since #6391, so Studio's 422 and a consumer's diagnosis can no
35+
longer describe one body two ways. Branches that are not the claimed one are
36+
replaced, in place, by the "wrong kind at the root" issue shape that every
37+
ranking already drops, so the claimed branch is what remains to be rendered.
38+
39+
The card's repro now reads:
40+
41+
```
42+
config.sections.0.fields.0.publicPicker:
43+
Unrecognized key(s) on this public picker configuration: `sort`.
44+
```
45+
46+
**No acceptance change**, by construction: the focusing is a `$ZodCheck` that
47+
runs after the union has reached its verdict and can only rewrite an existing
48+
issue's `errors` — it adds no issue and removes none. Verdicts, parse output and
49+
top-level issue codes are byte-identical across the 42-body corpus in
50+
`view-union-diagnostics.test.ts` plus 11 more measured for this change. The
51+
`errors` array keeps its length and order for positional consumers, the
52+
`invalid_union` envelope is untouched, a body with no discriminant is left to
53+
the ranking exactly as before, and a genuine container failure still reads the
54+
#4001 wrap prescription verbatim.

packages/metadata-protocol/src/protocol.save-union-issues.test.ts

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,95 @@ describe('#5364 saveMetaItem 422 expands union branches', () => {
174174
});
175175
});
176176

177+
/**
178+
* #7510 — the 422 for a ViewItem body names the key the AUTHOR typed.
179+
*
180+
* #5364 (above) got a field name onto the wire at all. It did not settle WHICH
181+
* branch's field name, and for a `viewKind`-carrying item the shared ranking
182+
* picked the container branch: the author was told to restructure a correct
183+
* container, in detail, with confidence, while the subkey they actually
184+
* mistyped went unmentioned. `ViewMetadataSchema` now focuses a failed union on
185+
* the branch the body claims (`spec/ui/view.zod.ts` → `focusClaimedBranch`), so
186+
* the expansion below has the right branch to expand.
187+
*
188+
* Pinned HERE and not only in spec because this is the door the defect was
189+
* measured through — `saveMetaItem`'s 422 is what Studio renders, and a spec
190+
* fix that did not reach this envelope would be a fix nobody sees.
191+
*/
192+
describe('#7510 the 422 for a broken ViewItem names its own key, not the container\'s', () => {
193+
/** The card's repro: a form ViewItem whose field's `publicPicker` carries `sort`. */
194+
const pickerReproView = () => ({
195+
name: 'lead.contact',
196+
object: 'lead',
197+
viewKind: 'form',
198+
label: 'Contact us',
199+
config: {
200+
type: 'simple',
201+
data: { provider: 'object', object: 'lead' },
202+
sections: [{
203+
label: 'About you',
204+
fields: [{ field: 'owner', publicPicker: { displayFields: ['name'], sort: [{ field: 'email', order: 'desc' }] } }],
205+
}],
206+
},
207+
});
208+
209+
it('the issue\'s body: `sort` reaches the author, the container prescription does not', async () => {
210+
const { protocol, rows } = makeProtocol();
211+
212+
const err = await rejection(save(protocol, pickerReproView(), 'lead.contact'));
213+
214+
expect(err.code).toBe('INVALID_METADATA');
215+
expect(err.status).toBe(422);
216+
// Load-bearing: the verdict is unchanged — refused, nothing persisted.
217+
// #7510 moves which refusal is explained, never whether it is one.
218+
expect(rows.size).toBe(0);
219+
220+
// The union's own entry is still entry 0, unmoved (#5364's additive
221+
// contract) — focusing happens inside its `errors`, not around it.
222+
expect(err.issues[0]).toEqual({ path: '', message: 'Invalid input', code: 'invalid_union' });
223+
224+
const unknownKey = err.issues.find((i: any) => i.code === 'unrecognized_keys');
225+
expect(unknownKey).toBeDefined();
226+
expect(unknownKey.message).toContain('`sort`');
227+
expect(unknownKey.path).toBe('config.sections.0.fields.0.publicPicker');
228+
229+
// ⛔ The measured misdirect, gone from the whole envelope: on
230+
// `origin/main` @ `9051802` this message was the container branch's.
231+
expect(err.message).not.toContain('belongs to a single VIEW, not to the container');
232+
expect(JSON.stringify(err.issues)).not.toContain('this view container');
233+
});
234+
235+
it('the same item minus the bad subkey still saves', async () => {
236+
const { protocol, rows } = makeProtocol();
237+
const item: any = pickerReproView();
238+
delete item.config.sections[0].fields[0].publicPicker.sort;
239+
240+
const result = await save(protocol, item, 'lead.contact');
241+
242+
expect(result.success).toBe(true);
243+
expect(rows.size).toBe(1);
244+
});
245+
246+
it('a genuine container failure keeps the container prescription', async () => {
247+
// Constraint (c) of the card: the #4001 guidance text is good, and a
248+
// body that really is a container with wrong-layer keys still reads it.
249+
const { protocol, rows } = makeProtocol();
250+
251+
const err = await rejection(save(protocol, {
252+
name: 'lead',
253+
object: 'lead',
254+
list: { type: 'grid', columns: [{ field: 'title' }] },
255+
type: 'grid',
256+
columns: [{ field: 'title' }],
257+
}, 'lead'));
258+
259+
expect(err.status).toBe(422);
260+
expect(rows.size).toBe(0);
261+
expect(err.message).toContain('Unrecognized key(s) on this view container');
262+
expect(err.message).toContain('belongs to a single VIEW, not to the container');
263+
});
264+
});
265+
177266
describe('#5364 zodIssuesToMetadataIssues — the shared ranking, verbatim', () => {
178267
const union = (errors: unknown[][], path: unknown[] = []) =>
179268
({ code: 'invalid_union', message: 'Invalid input', path, errors });

0 commit comments

Comments
 (0)