Skip to content

Commit 290dfc5

Browse files
committed
fix(objectql): the value-shape detail prescribes the key rename, not the missing-pair type error
A value-shape rejection was read positionally (`parsed.error.issues[0]`) at both sites that produce the operator-facing detail: the write path's warn-first / strict branch, and the exported `valueShapeViolation` the `os migrate value-shapes` scan imports. zod reports per-member issues before the object-level `unrecognized_keys` one, so on a value whose keys were RENAMED the actionable message sorts last and was discarded — a `location` stored as `{latitude, longitude}` reported `expected number, received undefined` while `LocationValueSchema`'s curated `aliases` map had already built the rename prescription that edit distance cannot reach. Both readers now share one helper that prefers the undeclared-key issue when the rejection carries one. Scoped by a sweep of every class the two readers cover: only `location` and `address` are backed by a key-closed object schema, so only they can emit `unrecognized_keys` at all and the preference is a no-op for the other fourteen types. The sweep also refuted the reason `address` looked immune — all-optional members rule out a missing-member type error, not a wrong-typed declared one — so a pin now covers that class too. No verdict moves: the same values are flagged and the same writes rejected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
1 parent c463d03 commit 290dfc5

4 files changed

Lines changed: 112 additions & 5 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
"@objectstack/objectql": patch
3+
---
4+
5+
`os migrate value-shapes` now prescribes the key rename on a legacy `{latitude, longitude}` location, instead of reporting the missing-pair type error.
6+
7+
A value-shape rejection was read positionally — `parse.error.issues[0]` — at both places the value-shape detail is produced: the write path's warn-first / strict branch, and the exported `valueShapeViolation` the scan imports. zod reports per-member issues before the object-level `unrecognized_keys` one, so on a value whose keys were **renamed** the actionable message sorts last and was discarded. A `location` stored as `{latitude, longitude}` — the exact legacy shape the scan's own header names as one it exists to find — reported `Invalid input: expected number, received undefined`, leaving an operator to derive a rename that edit distance cannot reach (`latitude` -> `lat`), while `LocationValueSchema` had built the prescription and thrown it away.
8+
9+
Both readers now prefer the undeclared-key issue when the rejection carries one, through a single shared helper — two readings of the same rejection drifting by one clause is how one path prescribes the rename and the other does not. The affected strings are the `os migrate value-shapes` finding `detail`, the warn-first `[value-shape]` log line, and the `invalid_value_shape` error's `detail` under strict enforcement.
10+
11+
⛔ No verdict moves. The same values are flagged, the same writes are rejected or admitted, and the deployment gate opens on exactly the same evidence — only the operator-facing text changes.
12+
13+
Scoped by measurement rather than by assumption: of the sixteen types these readers cover, only `location` and `address` are backed by a key-closed object schema, so only they can emit `unrecognized_keys` at all — for the other fourteen the preference cannot change a single character. Both classes it does reach curate the alias map that makes the undeclared key the more actionable half. The defect reaches `address` as well as `location`: every address member being optional rules out a *missing*-member type error, but not a *wrong-typed* declared one, which still sorts ahead of the undeclared-key issue.

packages/objectql/src/validation/record-validator.ts

Lines changed: 51 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,47 @@ export function coerceBooleanFields<T extends Record<string, unknown>>(
487487
return (copy ?? row) as T;
488488
}
489489

490+
/**
491+
* The one parse issue an author can act on, out of everything zod reported for
492+
* a value-shape rejection.
493+
*
494+
* NOT `issues[0]`. zod reports per-member issues before the object-level
495+
* `unrecognized_keys` one, so on a value whose keys were RENAMED the actionable
496+
* message sorts LAST. A `location` stored as `{latitude, longitude}` — the
497+
* exact legacy shape `scan-value-shapes` names in its own header as one the
498+
* scan exists to find — reports:
499+
*
500+
* [0] invalid_type lat Invalid input: expected number, received undefined
501+
* [1] invalid_type lng Invalid input: expected number, received undefined
502+
* [2] unrecognized_keys ... Did you mean `latitude` -> `lat`, `longitude` -> `lng`? ...
503+
*
504+
* The rename IS the prescription, and edit distance cannot reach it
505+
* (`latitude` -> `lat`), which is exactly why `LocationValueSchema` curates an
506+
* `aliases` map. Reading positionally builds that hint and then discards it,
507+
* leaving an operator running `os migrate value-shapes` to derive the rename
508+
* themselves while the identically-shaped `address` case is handed it.
509+
*
510+
* The preference is a NO-OP for every other class rather than merely harmless
511+
* to it, which is what makes it safe as a blanket rule: of the sixteen types
512+
* these readers cover, only `location` and `address` are backed by a
513+
* `strictObject`, so only they can emit `unrecognized_keys` at all. The
514+
* reference and file-reference classes are strings, `composite` / `record` /
515+
* `repeater` / `vector` are open records and arrays, `json` is `z.unknown()`,
516+
* and the one deliberately loose object shape (`FileValueSchema`) never refuses
517+
* a key. For the other fourteen this cannot change a single character; both
518+
* classes it does reach curate the alias map that makes the undeclared key the
519+
* more actionable half of the rejection.
520+
*
521+
* Shared by both readers for the reason `valueShapeViolation` is exported
522+
* rather than re-derived one layer up: two readings of the same rejection
523+
* drifting by one clause is how one path prescribes the rename and the other
524+
* hands out `expected number, received undefined`.
525+
*/
526+
function valueShapeDetail(error: { issues: ReadonlyArray<{ code: string; message: string }> }): string {
527+
const { issues } = error;
528+
return (issues.find((i) => i.code === 'unrecognized_keys') ?? issues[0])?.message ?? 'invalid value shape';
529+
}
530+
490531
function validateOne(
491532
name: string,
492533
def: FieldDef,
@@ -792,7 +833,7 @@ function validateOne(
792833
if (REFERENCE_VALUE_TYPES.has(t) || FILE_REFERENCE_TYPES.has(t) || STRUCTURED_JSON_TYPES.has(t)) {
793834
const parsed = shapeSchemaFor(def).safeParse(value);
794835
if (!parsed.success) {
795-
const detail = parsed.error.issues[0]?.message ?? 'invalid value shape';
836+
const detail = valueShapeDetail(parsed.error);
796837
const isMedia = FILE_REFERENCE_TYPES.has(t);
797838
if (isMedia ? mediaStrictEffective(mediaStrict) : valueShapeStrictEffective(valueStrict)) {
798839
return fail('invalid_type', { type: t, detail }, 'invalid_value_shape');
@@ -931,7 +972,9 @@ export function mediaPostureSetByEnv(): boolean {
931972
* not recognise, which is precisely the borrowed-evidence failure the ADR's
932973
* addendum forbids one layer up.
933974
*
934-
* Returns the first parse issue's message, or `null` when the value conforms.
975+
* Returns the actionable parse issue's message (see `valueShapeDetail` — the
976+
* undeclared-key one when present, which is the half carrying the rename),
977+
* or `null` when the value conforms.
935978
* A value that is missing (per `isMissing`) is never a violation: absence is
936979
* the `required` check's business, not the shape contract's.
937980
*/
@@ -943,7 +986,7 @@ export function valueShapeViolation(def: FieldDef, value: unknown): string | nul
943986
}
944987
const parsed = shapeSchemaFor(def).safeParse(value);
945988
if (parsed.success) return null;
946-
return parsed.error.issues[0]?.message ?? 'invalid value shape';
989+
return valueShapeDetail(parsed.error);
947990
}
948991

949992
/**
@@ -1009,7 +1052,11 @@ export interface AdmittedValueShapeViolation {
10091052
field: string;
10101053
/** The declared field type, so a report can group by what went wrong. */
10111054
type: string;
1012-
/** The first parse issue — the prescription an author acts on. */
1055+
/**
1056+
* The prescription an author acts on — `valueShapeDetail`'s reading of the
1057+
* rejection, the same string the warn-first log line and the
1058+
* `os migrate value-shapes` finding carry. Not positional.
1059+
*/
10131060
detail: string;
10141061
}
10151062

packages/objectql/src/validation/scan-value-shapes.test.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,27 @@ describe('scanValueShapes (ADR-0104 D1 / #3438)', () => {
176176
).not.toThrow();
177177
});
178178

179+
it('#15490: an undeclared key outranks a same-value member error — on address too, not just location', async () => {
180+
// The sweep this fix owes measured every class the two readers cover, and
181+
// found only `location` and `address` able to emit `unrecognized_keys` at
182+
// all (the rest are strings, open records, arrays, or `z.unknown()`). It
183+
// also refuted the reason `address` was believed immune: every member being
184+
// OPTIONAL rules out a MISSING-member `invalid_type`, but says nothing about
185+
// a WRONG-TYPED declared one — which still sorts ahead of the object-level
186+
// issue. So the defect reaches this class too; the contrast case that made
187+
// it look location-only just happened to carry no type error.
188+
const engine = makeEngine({
189+
contact: [{ id: 'c1', addr: { street: 5, postal_code: '98101' } }],
190+
});
191+
const report = await scanValueShapes(engine, silent);
192+
193+
expect(report.blocking).toBe(1);
194+
const addr = report.findings.find((f) => f.field === 'addr')!;
195+
// Positionally this rejection reads `[invalid_type(street), unrecognized_keys]`.
196+
expect(addr.detail).toContain('`postal_code` \u2192 `postalCode`');
197+
expect(addr.detail).not.toContain('expected string, received number');
198+
});
199+
179200
it('the scan counts exactly what strict mode rejects — one predicate, not two', async () => {
180201
// The anti-drift property: every value the scan flags must also be a write
181202
// rejection under the strict gate, and every value it passes must write.
@@ -186,6 +207,25 @@ describe('scanValueShapes (ADR-0104 D1 / #3438)', () => {
186207
const report = await scanValueShapes(engine, silent);
187208
expect(report.blocking).toBe(1);
188209

210+
// #15490: the fixture above was already the renamed pair, but the pin
211+
// asserted only the COUNT — so the operator-facing prescription was free to
212+
// be the wrong half of the rejection and every gate stayed green. zod sorts
213+
// the two missing-member `invalid_type` issues (`lat`, `lng`) ahead of the
214+
// object-level `unrecognized_keys` one, so a positional `issues[0]` read
215+
// reported `expected number, received undefined` and threw away the rename
216+
// the schema curates an `aliases` map to produce. Assert the PRESCRIPTION,
217+
// not just that something was flagged: `detail` is documented as "the
218+
// prescription an author acts on".
219+
const geo = report.findings.find((f) => f.field === 'geo')!;
220+
expect(geo.detail).toContain('`latitude` \u2192 `lat`');
221+
expect(geo.detail).toContain('`longitude` \u2192 `lng`');
222+
expect(geo.detail).toContain('this location value');
223+
// The half that was being shown instead — naming it keeps the pin able to
224+
// see a regression back to the positional read.
225+
expect(geo.detail).not.toContain('expected number, received undefined');
226+
// Customer-facing refusal text carries no internal issue id.
227+
expect(geo.detail).not.toMatch(/#\d+/);
228+
189229
expect(() =>
190230
validateRecord(OBJECTS.contact, { ...flagged }, 'update', { valueShapeStrict: true }),
191231
).toThrow(ValidationError);

packages/objectql/src/validation/scan-value-shapes.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,14 @@ export interface ValueShapeFinding {
4848
count: number;
4949
/** A few offending record ids, so an operator can go and look. */
5050
sampleRecordIds: string[];
51-
/** The first parse issue seen — the prescription an author acts on. */
51+
/**
52+
* The prescription an author acts on: the undeclared-key issue when the
53+
* rejection carries one (it names the key AND the rename its schema
54+
* curates), else the first issue seen. Deliberately not positional —
55+
* zod sorts per-member issues ahead of the object-level one, so on the
56+
* renamed `{latitude, longitude}` location this header names, the
57+
* actionable message is last.
58+
*/
5259
detail: string;
5360
}
5461

0 commit comments

Comments
 (0)