Skip to content

Commit 3fc2e48

Browse files
os-zhuangclaude
andauthored
fix(spec,rest,cli): validation diagnostics reach the real defect — named view-union branches + invalid_key/invalid_element descent (#7025) (#7042)
* fix(spec,rest,cli): validation diagnostics reach the real defect (#6391, #5389) Sweep #7025 — two cases where a refusal is correct but its DIAGNOSTIC cannot reach the element that actually failed. Both fixes move the diagnostic face only; the acceptance face is pinned unmoved in both directions. #6391 — ViewMetadataSchema's union members are contractual, not positional. Three of its four members were inline expressions with no name, so a consumer diagnosing a failure could only reach a branch by indexing the nested invalid_union `errors[]` BY MEMBER POSITION (objectui#3624 shipped exactly that and had to hold the coupling down with a canary test). The union is now BUILT from a named record — VIEW_METADATA_BRANCHES / VIEW_METADATA_MEMBERS — so "member N is the published schema" is one declaration rather than two that can drift, and diagnoseViewMetadata() returns the failing branch by NAME with that branch's own leaf issues and real field paths. The union is deliberately NOT converted to z.discriminatedUnion: that would move membership (a discriminated union refuses an unknown discriminant outright where this one falls through all four members, and several of these shapes carry no discriminant at all). The dispatch is diagnostic — ViewMetadataSchema remains the only judge of acceptance, and a pin asserts the two never disagree. Measured: a 41-body corpus run through ViewMetadataSchema before and after produces byte-identical verdicts, parse output and issue-code sets (sha256 fca9df8937bbb9f736f11895a6e1ddf23b7fb25d9b13cfa7e67c71c8dfaaf2b2), and that corpus is now a committed pin. #5389 — invalid_key / invalid_element are descended, in all three consumers. Zod hangs a failing record-key / map-element schema's real issues on `issue.issues` — the same shape as invalid_union's `issue.errors`, one property name over. The family had already been fixed three times for `errors` (#4971, #5014, #5341) while none of the three consumers read `issues`, so both codes surfaced as a bare wrapper line with the prescription stranded in the payload. formatZodError/formatZodIssue (spec), zodIssuesToFields (the REST wire) and formatZodErrors (the CLI terminal) now all descend it, additively: the container's own line/entry is unchanged and the leaves follow it. Unlike a union's branches — competing candidates, therefore ranked and capped — a container's issues are the one list the inner schema produced, so every one of them is reported. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FRQdfKG4YpSv1SkujpWmk2 * docs(spec): the acceptance-face corpus is 42 bodies, not 41 Comment-only. The pin table is 19 ACCEPT + 23 REFUSED = 42; the header said 41. No assertion changed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FRQdfKG4YpSv1SkujpWmk2 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent f764691 commit 3fc2e48

10 files changed

Lines changed: 1100 additions & 84 deletions

File tree

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/rest": patch
4+
"@objectstack/cli": patch
5+
---
6+
7+
fix(spec,rest,cli): validation diagnostics reach the real defect — named view-union branches, and `invalid_key` / `invalid_element` descent (#6391, #5389)
8+
9+
Two cases where a refusal fired correctly but its *diagnostic* could not reach the
10+
element that actually failed. Both fixes change the DIAGNOSTIC face only: every
11+
input that parsed before parses after, every input refused before is refused
12+
after, and each refusal keeps its issue codes (ADR-0112 / #6142 — a better
13+
diagnostic never weakens the envelope). Pinned in both directions.
14+
15+
**#6391`ViewMetadataSchema`'s union members are now contractual.** Three of
16+
its four members were inline expressions with no name, so a consumer diagnosing a
17+
failure could only reach a branch by indexing the nested `invalid_union`
18+
`errors[]` **by member position**; objectui shipped exactly that and had to hold
19+
the coupling down with a canary test (objectui#3606 / PR objectui#3624). The
20+
union is now built from a named record:
21+
22+
- `VIEW_METADATA_BRANCHES` — the branch names, in the union's own order;
23+
- `VIEW_METADATA_MEMBERS` — branch name → the schema the union actually holds
24+
(`viewItem` is identically `ViewItemWireSchema`, as before);
25+
- `selectViewMetadataBranch(body)` — which branch a body claims, by the
26+
discriminants the members already declare;
27+
- `diagnoseViewMetadata(body)` — the failing branch **named**, with that branch's
28+
own leaf issues and real field paths, so no consumer needs `errors[i]`.
29+
30+
The union is **not** converted to `z.discriminatedUnion`. That would move the
31+
acceptance face — a discriminated union refuses an unknown discriminant outright
32+
where this one falls through all four members, and several of these shapes carry
33+
no discriminant at all. `ViewMetadataSchema` remains the only judge of
34+
acceptance; the dispatch only explains a verdict it did not make, and a pin
35+
asserts the two never disagree.
36+
37+
**#5389`invalid_key` / `invalid_element` are descended, in all three
38+
consumers.** Zod hangs a failing record-key / map-element schema's real issues on
39+
`issue.issues` — the same shape as `invalid_union`'s `issue.errors`, one property
40+
name over. The family had already been fixed three times for `errors` (#4971,
41+
#5014, #5341) and none of the three consumers read `issues`, so both codes
42+
surfaced as a bare wrapper line with the prescription stranded in the payload.
43+
Now `formatZodError`/`formatZodIssue` (spec), `zodIssuesToFields` (the REST wire)
44+
and `formatZodErrors` (the CLI terminal) all descend it.
45+
46+
Before / after, a `z.record` with a constrained key:
47+
48+
```
49+
✗ fields.First Name: Invalid key in record
50+
```
51+
```
52+
✗ fields.First Name: Invalid key in record
53+
✗ fields.First Name: Invalid identifier. Must be lowercase snake_case …
54+
```
55+
56+
The expansion is strictly additive on every surface: the container's own line
57+
(and, on the wire, its own `{field, code: 'invalid_shape', message}` entry) is
58+
unchanged, and the leaves follow it. Unlike a union's branches — competing
59+
candidates, therefore ranked and capped — a container's `issues` are the one list
60+
the inner schema produced, so every one of them is reported.

packages/cli/src/utils/format.ts

Lines changed: 39 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -208,18 +208,37 @@ export function createTimer() {
208208
// ─── Zod Error Formatting ───────────────────────────────────────────
209209

210210
/**
211-
* How far the branch lines of an expanded union are pushed to sit UNDER the
212-
* `code: message` line this file prints for the union itself.
211+
* How far the nested lines of an expanded issue are pushed to sit UNDER the
212+
* `code: message` line this file prints for the issue itself.
213213
*
214214
* `formatZodIssue` indents its own depth-0 line by 2 spaces and each nested
215215
* level by 2 more; this file's per-issue block is at 4/6. Adding 4 puts the
216-
* first branch level at 8 — one step below the `invalid_union: Invalid input`
216+
* first nested level at 8 — one step below the `invalid_union: Invalid input`
217217
* line it explains — and keeps every deeper level nested relative to it.
218218
*/
219-
const UNION_BRANCH_REINDENT = ' ';
219+
const NESTED_ISSUE_REINDENT = ' ';
220220

221221
/**
222-
* The lines that explain an `invalid_union`, or nothing at all.
222+
* [#5389] The issue codes whose real diagnosis is nested one level down, and
223+
* which `formatZodIssue` therefore renders as more than one line.
224+
*
225+
* `invalid_union` puts each candidate branch on `issue.errors[]`; `invalid_key`
226+
* / `invalid_element` put the key/element schema's own issues on
227+
* `issue.issues[]`. Same defect, two property names — and the gate below has to
228+
* name both, or the terminal keeps printing `invalid_key: Invalid key in
229+
* record` with the prescription stranded in the payload.
230+
*
231+
* Kept in step with `CONTAINER_ISSUE_CODES` in `@objectstack/spec`'s
232+
* `error-map.zod.ts`, which is where the descent itself lives.
233+
*/
234+
const EXPANDABLE_ISSUE_CODES: ReadonlySet<string> = new Set([
235+
'invalid_union',
236+
'invalid_key',
237+
'invalid_element',
238+
]);
239+
240+
/**
241+
* The lines that explain an expandable issue, or nothing at all.
223242
*
224243
* Zod folds every branch of a failed union into ONE issue whose own `message`
225244
* is the literal `"Invalid input"`; each branch's real rejection sits in
@@ -241,18 +260,24 @@ const UNION_BRANCH_REINDENT = ' ';
241260
* so had to re-implement the ranking — the terminal needs exactly the STRING
242261
* that spec already exports, so here the reuse is a plain import.
243262
*
244-
* Line 0 of that render is the union's own verdict, which the caller has
263+
* [#5389] The same import now also covers `invalid_key` / `invalid_element`,
264+
* whose diagnosis hangs on `issue.issues[]` instead. Widening the gate is the
265+
* WHOLE of this file's share of that fix — the descent is spec's, so the
266+
* terminal inherits it the moment it stops refusing to ask.
267+
*
268+
* Line 0 of that render is the issue's own verdict, which the caller has
245269
* already printed in this file's own idiom; only the explanation is returned,
246270
* so the change is strictly ADDITIVE — nothing that printed before #5341 stops
247-
* printing. A non-union issue renders as a single line, hence never reaches
248-
* here and could not add one anyway.
271+
* printing. An issue with nothing nested renders as a single line, hence never
272+
* reaches here and could not add one anyway.
249273
*/
250-
function unionBranchLines(issue: unknown): string[] {
251-
if ((issue as { code?: unknown } | null)?.code !== 'invalid_union') return [];
274+
function nestedIssueLines(issue: unknown): string[] {
275+
const code = (issue as { code?: unknown } | null)?.code;
276+
if (typeof code !== 'string' || !EXPANDABLE_ISSUE_CODES.has(code)) return [];
252277
return formatZodIssue(issue as Parameters<typeof formatZodIssue>[0])
253278
.split('\n')
254279
.slice(1)
255-
.map((line) => `${UNION_BRANCH_REINDENT}${line}`);
280+
.map((line) => `${NESTED_ISSUE_REINDENT}${line}`);
256281
}
257282

258283
export function formatZodErrors(error: ZodError) {
@@ -291,8 +316,9 @@ export function formatZodErrors(error: ZodError) {
291316
console.log(chalk.dim(` received: ${chalk.red((issue as any).received)}`));
292317
}
293318

294-
// [#5341] …and, for a union, the branch that actually explains it.
295-
for (const line of unionBranchLines(issue)) {
319+
// [#5341] …and, for a union — [#5389] or a record key / map element —
320+
// the nested issues that actually explain it.
321+
for (const line of nestedIssueLines(issue)) {
296322
console.log(chalk.dim(line));
297323
}
298324
}

packages/cli/test/format-zod-union.test.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,3 +224,42 @@ describe('[#5341] `os validate` delivers a union branch prescription', () => {
224224
expect(JSON.stringify(payload.errors[0].errors)).toContain('`direction` → `order`');
225225
}, 120_000);
226226
});
227+
228+
/**
229+
* [#5389] The terminal's share of the 4th/5th family members.
230+
*
231+
* `invalid_key` / `invalid_element` hang the key/element schema's real
232+
* complaint on `issue.issues[]` rather than on `invalid_union`'s
233+
* `issue.errors[]`. The descent itself is `@objectstack/spec`'s — reused, not
234+
* re-derived, exactly as #5341 reused it for the union — so this file's whole
235+
* share of the fix is that its gate stopped saying `code !== 'invalid_union'`
236+
* and started naming all three expandable codes.
237+
*
238+
* Strictly additive, same as #5341: the `code: message` line the terminal
239+
* already printed for the container is untouched; only the explanation is new.
240+
*/
241+
describe('[#5389] formatZodErrors expands invalid_key / invalid_element', () => {
242+
const SnakeKey = z
243+
.string()
244+
.regex(/^[a-z][a-z0-9_]*$/, "Invalid identifier. Must be lowercase snake_case (e.g. 'first_name').");
245+
246+
it('prints the key schema prose under the container line', () => {
247+
const schema = z.object({ fields: z.record(SnakeKey, z.object({ type: z.string() })) });
248+
const out = render(schema.safeParse({ fields: { 'First Name': { type: 'text' } } }).error!);
249+
expect(out).toContain('invalid_key: Invalid key in record');
250+
expect(out).toContain("Invalid identifier. Must be lowercase snake_case (e.g. 'first_name').");
251+
});
252+
253+
it('prints the element schema prose for a map', () => {
254+
const schema = z.map(z.object({ id: z.string() }), z.number().min(5, 'too small'));
255+
const out = render(schema.safeParse(new Map([[{ id: 'a' }, 1]])).error!);
256+
expect(out).toContain('invalid_element');
257+
expect(out).toContain('too small');
258+
});
259+
260+
it('adds nothing to an ordinary issue', () => {
261+
// The conservative half: only the three expandable codes may grow lines.
262+
const before = render(z.object({ a: z.string() }).safeParse({}).error!);
263+
expect(before.split('\n').filter((l) => l.includes('✗'))).toHaveLength(1);
264+
});
265+
});

packages/rest/src/rest-server.ts

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -212,8 +212,8 @@ function valueAtPath(input: unknown, path: unknown): unknown {
212212
}
213213

214214
/**
215-
* How many levels of nested `invalid_union` are expanded below a top-level
216-
* issue, and how many equally-informative branches are emitted at one level.
215+
* How many levels of nested issues are expanded below a top-level issue, and
216+
* how many equally-informative union branches are emitted at one level.
217217
*
218218
* Both bounds — and the whole selection policy below — are the ones
219219
* `formatZodError` landed for the CLI/spec side of this defect (#4971,
@@ -223,9 +223,22 @@ function valueAtPath(input: unknown, path: unknown): unknown {
223223
* the same, or one mistake gets two different prescriptions depending on whether
224224
* the author published from the terminal or POSTed to the API (#5014).
225225
*/
226-
const UNION_EXPANSION_DEPTH_LIMIT = 3;
226+
const NESTED_EXPANSION_DEPTH_LIMIT = 3;
227227
const UNION_BRANCH_EMIT_LIMIT = 3;
228228

229+
/**
230+
* [#5389] The issue codes that hang their real diagnosis on `issue.issues`
231+
* rather than on `invalid_union`'s `issue.errors`.
232+
*
233+
* `invalid_key` is raised when `z.record(K, V)`'s KEY schema rejects a key (and
234+
* by `z.map` for a non-`PropertyKey` key); `invalid_element` when `z.map`'s
235+
* VALUE schema rejects the value under such a key. Both carry a bare wrapper
236+
* message ("Invalid key in record") with everything the client needs one level
237+
* down — the same defect as #5014, one property name over. Kept in step with
238+
* `CONTAINER_ISSUE_CODES` in `spec/src/shared/error-map.zod.ts`.
239+
*/
240+
const CONTAINER_ISSUE_CODES: ReadonlySet<string> = new Set(['invalid_key', 'invalid_element']);
241+
229242
/** A Zod issue path, normalised to the array Zod always produces. */
230243
function issuePathOf(issue: any): Array<string | number> {
231244
return Array.isArray(issue?.path) ? issue.path : [];
@@ -308,6 +321,14 @@ function selectUnionBranches(branches: readonly (readonly any[])[]): readonly (r
308321
* the branches that explain it, with `field` resolved against the union's own
309322
* path — branch paths are relative to it.
310323
*
324+
* [#5389] An `invalid_key` / `invalid_element` behaves the same way one property
325+
* name over: its own entry (zod's `"Invalid key in record"`, also
326+
* `invalid_shape`) followed by the entries on `issue.issues`, whose paths are
327+
* likewise relative. The one difference from a union: those issues are not
328+
* competing candidates, so they are NOT ranked or capped — every one of them is
329+
* a true statement about the value, and dropping any would be dropping a real
330+
* diagnosis rather than declining to guess.
331+
*
311332
* The union's entry is kept rather than replaced: it is the only entry naming
312333
* the slot the client sent, existing clients already read it, and when every
313334
* branch is uninformative it is still the whole answer. So the expansion is
@@ -343,7 +364,11 @@ function collectIssueFields(
343364
const branches: readonly (readonly any[])[] = issue?.code === 'invalid_union' && Array.isArray(issue?.errors)
344365
? issue.errors.filter((branch: unknown): branch is any[] => Array.isArray(branch))
345366
: [];
346-
const expandable = branches.length > 0 && depth < UNION_EXPANSION_DEPTH_LIMIT;
367+
const contained: readonly any[] = CONTAINER_ISSUE_CODES.has(issue?.code) && Array.isArray(issue?.issues)
368+
? issue.issues
369+
: [];
370+
const expandable = (branches.length > 0 || contained.length > 0)
371+
&& depth < NESTED_EXPANSION_DEPTH_LIMIT;
347372

348373
const entry = {
349374
field,
@@ -361,10 +386,17 @@ function collectIssueFields(
361386
out.push(entry);
362387
if (!expandable) return;
363388

364-
for (const branch of selectUnionBranches(branches)) {
365-
for (const nested of branch) {
366-
collectIssueFields(nested, path, depth + 1, seen, input, inputProvided, out);
389+
if (branches.length > 0) {
390+
for (const branch of selectUnionBranches(branches)) {
391+
for (const nested of branch) {
392+
collectIssueFields(nested, path, depth + 1, seen, input, inputProvided, out);
393+
}
367394
}
395+
return;
396+
}
397+
398+
for (const nested of contained) {
399+
collectIssueFields(nested, path, depth + 1, seen, input, inputProvided, out);
368400
}
369401
}
370402

packages/rest/src/zod-union-fields.test.ts

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,3 +238,85 @@ describe('zodIssuesToFields — invalid_union expansion (#5014)', () => {
238238
});
239239
});
240240
});
241+
242+
/**
243+
* [#5389] `invalid_key` / `invalid_element` — the same defect, one property
244+
* name over, on the same wire.
245+
*
246+
* `invalid_union` buries a branch's real complaint in `issue.errors[]`; these
247+
* two bury the key/element schema's real complaint in `issue.issues[]`. Before
248+
* this, a client POSTing a record with a bad KEY got exactly one `fields[]`
249+
* entry — `{field: 'fields.First Name', code: 'invalid_shape', message:
250+
* 'Invalid key in record'}` — with the sentence naming the rule produced and
251+
* dropped, which is #5014 verbatim.
252+
*
253+
* The expansion is strictly ADDITIVE, same contract as #5014: the container's
254+
* own entry stays (it is the only one naming the slot, and existing clients
255+
* read it) and the leaf entries follow it. ADR-0114's `{field, code, message}`
256+
* shape is unchanged, and the container entry keeps mapping to `invalid_shape`.
257+
*
258+
* Every fixture drives a REAL `safeParse`, for the reason the file's header
259+
* gives: `issue.issues` is Zod's internal shape.
260+
*/
261+
describe('zodIssuesToFields — invalid_key / invalid_element descent (#5389)', () => {
262+
const fieldsFor = (schema: z.ZodType, value: unknown) => {
263+
const r = schema.safeParse(value);
264+
expect(r.success, 'the fixture must actually fail to parse').toBe(false);
265+
return zodIssuesToFields((r as { error: { issues: unknown[] } }).error.issues, value);
266+
};
267+
268+
const SnakeKey = z
269+
.string()
270+
.regex(/^[a-z][a-z0-9_]*$/, "Invalid identifier. Must be lowercase snake_case (e.g. 'first_name').");
271+
272+
it('delivers the KEY schema prescription beside the container entry', () => {
273+
const schema = z.object({ fields: z.record(SnakeKey, z.object({ type: z.string() })) });
274+
const fields = fieldsFor(schema, { fields: { 'First Name': { type: 'text' } } });
275+
276+
expect(fields).toEqual([
277+
// Unchanged — the entry clients read today.
278+
{ field: 'fields.First Name', code: 'invalid_shape', message: 'Invalid key in record' },
279+
// …and the leaf that says WHY, with the container's path resolved.
280+
{
281+
field: 'fields.First Name',
282+
code: 'invalid_format',
283+
message: "Invalid identifier. Must be lowercase snake_case (e.g. 'first_name').",
284+
},
285+
]);
286+
});
287+
288+
it('emits EVERY nested issue — a container has no branches to rank', () => {
289+
// Deliberate divergence from the union descent: `errors[]` holds
290+
// competing candidates, so it is ranked and capped; `issues[]` is the
291+
// one list the element schema produced, and each entry is a real field.
292+
const schema = z.map(
293+
z.object({ id: z.string() }),
294+
z.object({ a: z.string(), b: z.string(), c: z.string() }),
295+
);
296+
const fields = fieldsFor(schema, new Map([[{ id: 'x' }, {}]]));
297+
expect(fields[0]).toMatchObject({ code: 'invalid_shape' });
298+
expect(fields.map((f) => f.field)).toEqual(['', 'a', 'b', 'c']);
299+
});
300+
301+
it('keeps the catalog code an ADR-0114 member on every entry', () => {
302+
const schema = z.object({ fields: z.record(SnakeKey, z.unknown()) });
303+
const codes = FieldErrorCode.options as readonly string[];
304+
for (const entry of fieldsFor(schema, { fields: { 'Bad Key': 1 } })) {
305+
expect(codes).toContain(entry.code);
306+
}
307+
});
308+
309+
it('leaves an ordinary issue exactly as it was', () => {
310+
const schema = z.object({ a: z.string() });
311+
expect(fieldsFor(schema, {})).toEqual([
312+
{ field: 'a', code: 'required', message: 'Invalid input: expected string, received undefined' },
313+
]);
314+
});
315+
316+
it('still tolerates a container issue with no nested list', () => {
317+
expect(zodIssuesToFields([{ code: 'invalid_key', path: ['m', 'k'], message: 'Invalid key in record' }]))
318+
.toEqual([{ field: 'm.k', code: 'invalid_shape', message: 'Invalid key in record' }]);
319+
expect(zodIssuesToFields([{ code: 'invalid_element', path: ['m'], message: 'Invalid value', issues: 'nope' }]))
320+
.toEqual([{ field: 'm', code: 'invalid_shape', message: 'Invalid value' }]);
321+
});
322+
});

packages/spec/api-surface/ui.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,8 @@
329329
"VIEW_CONSOLE_ROW_DECORATIONS (const)",
330330
"VIEW_FILTER_OPERATORS (const)",
331331
"VIEW_FILTER_OPERATOR_ALIASES (const)",
332+
"VIEW_METADATA_BRANCHES (const)",
333+
"VIEW_METADATA_MEMBERS (const)",
332334
"VIEW_WRITE_PATH_IDENTITY_KEYS (const)",
333335
"View (type)",
334336
"ViewData (type)",
@@ -348,6 +350,8 @@
348350
"ViewKind (type)",
349351
"ViewKindSchema (const)",
350352
"ViewMetadata (type)",
353+
"ViewMetadataBranch (type)",
354+
"ViewMetadataDiagnosis (type)",
351355
"ViewMetadataParsed (type)",
352356
"ViewMetadataSchema (const)",
353357
"ViewParsed (type)",
@@ -382,6 +386,7 @@
382386
"defineTheme (function)",
383387
"defineView (function)",
384388
"defineViewItem (function)",
389+
"diagnoseViewMetadata (function)",
385390
"expandViewContainer (function)",
386391
"expandViewContainerWithDiagnostics (function)",
387392
"isAggregatedViewContainer (function)",
@@ -393,6 +398,7 @@
393398
"reportForm (const)",
394399
"reportSelectionOrder (function)",
395400
"resolveI18nLabel (function)",
401+
"selectViewMetadataBranch (function)",
396402
"stripViewConsoleDecorations (function)",
397403
"validateActionParams (function)",
398404
"viewForm (const)"

0 commit comments

Comments
 (0)