diff --git a/.changeset/value-shape-detail-prefers-unrecognized-keys.md b/.changeset/value-shape-detail-prefers-unrecognized-keys.md new file mode 100644 index 0000000000..be2c73cfb9 --- /dev/null +++ b/.changeset/value-shape-detail-prefers-unrecognized-keys.md @@ -0,0 +1,13 @@ +--- +"@objectstack/objectql": patch +--- + +`os migrate value-shapes` now prescribes the key rename on a legacy `{latitude, longitude}` location, instead of reporting the missing-pair type error. + +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. + +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. + +⛔ 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. + +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. diff --git a/packages/objectql/src/validation/record-validator.ts b/packages/objectql/src/validation/record-validator.ts index 1d33afc116..08f474fd46 100644 --- a/packages/objectql/src/validation/record-validator.ts +++ b/packages/objectql/src/validation/record-validator.ts @@ -487,6 +487,47 @@ export function coerceBooleanFields>( return (copy ?? row) as T; } +/** + * The one parse issue an author can act on, out of everything zod reported for + * a value-shape rejection. + * + * NOT `issues[0]`. 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. A `location` stored as `{latitude, longitude}` — the + * exact legacy shape `scan-value-shapes` names in its own header as one the + * scan exists to find — reports: + * + * [0] invalid_type lat Invalid input: expected number, received undefined + * [1] invalid_type lng Invalid input: expected number, received undefined + * [2] unrecognized_keys ... Did you mean `latitude` -> `lat`, `longitude` -> `lng`? ... + * + * The rename IS the prescription, and edit distance cannot reach it + * (`latitude` -> `lat`), which is exactly why `LocationValueSchema` curates an + * `aliases` map. Reading positionally builds that hint and then discards it, + * leaving an operator running `os migrate value-shapes` to derive the rename + * themselves while the identically-shaped `address` case is handed it. + * + * The preference is a NO-OP for every other class rather than merely harmless + * to it, which is what makes it safe as a blanket rule: of the sixteen types + * these readers cover, only `location` and `address` are backed by a + * `strictObject`, so only they can emit `unrecognized_keys` at all. The + * reference and file-reference classes are strings, `composite` / `record` / + * `repeater` / `vector` are open records and arrays, `json` is `z.unknown()`, + * and the one deliberately loose object shape (`FileValueSchema`) never refuses + * a key. For the other fourteen this cannot change a single character; both + * classes it does reach curate the alias map that makes the undeclared key the + * more actionable half of the rejection. + * + * Shared by both readers for the reason `valueShapeViolation` is exported + * rather than re-derived one layer up: two readings of the same rejection + * drifting by one clause is how one path prescribes the rename and the other + * hands out `expected number, received undefined`. + */ +function valueShapeDetail(error: { issues: ReadonlyArray<{ code: string; message: string }> }): string { + const { issues } = error; + return (issues.find((i) => i.code === 'unrecognized_keys') ?? issues[0])?.message ?? 'invalid value shape'; +} + function validateOne( name: string, def: FieldDef, @@ -792,7 +833,7 @@ function validateOne( if (REFERENCE_VALUE_TYPES.has(t) || FILE_REFERENCE_TYPES.has(t) || STRUCTURED_JSON_TYPES.has(t)) { const parsed = shapeSchemaFor(def).safeParse(value); if (!parsed.success) { - const detail = parsed.error.issues[0]?.message ?? 'invalid value shape'; + const detail = valueShapeDetail(parsed.error); const isMedia = FILE_REFERENCE_TYPES.has(t); if (isMedia ? mediaStrictEffective(mediaStrict) : valueShapeStrictEffective(valueStrict)) { return fail('invalid_type', { type: t, detail }, 'invalid_value_shape'); @@ -931,7 +972,9 @@ export function mediaPostureSetByEnv(): boolean { * not recognise, which is precisely the borrowed-evidence failure the ADR's * addendum forbids one layer up. * - * Returns the first parse issue's message, or `null` when the value conforms. + * Returns the actionable parse issue's message (see `valueShapeDetail` — the + * undeclared-key one when present, which is the half carrying the rename), + * or `null` when the value conforms. * A value that is missing (per `isMissing`) is never a violation: absence is * the `required` check's business, not the shape contract's. */ @@ -943,7 +986,7 @@ export function valueShapeViolation(def: FieldDef, value: unknown): string | nul } const parsed = shapeSchemaFor(def).safeParse(value); if (parsed.success) return null; - return parsed.error.issues[0]?.message ?? 'invalid value shape'; + return valueShapeDetail(parsed.error); } /** @@ -1009,7 +1052,11 @@ export interface AdmittedValueShapeViolation { field: string; /** The declared field type, so a report can group by what went wrong. */ type: string; - /** The first parse issue — the prescription an author acts on. */ + /** + * The prescription an author acts on — `valueShapeDetail`'s reading of the + * rejection, the same string the warn-first log line and the + * `os migrate value-shapes` finding carry. Not positional. + */ detail: string; } diff --git a/packages/objectql/src/validation/scan-value-shapes.test.ts b/packages/objectql/src/validation/scan-value-shapes.test.ts index 1aa9495ba7..ee806e7886 100644 --- a/packages/objectql/src/validation/scan-value-shapes.test.ts +++ b/packages/objectql/src/validation/scan-value-shapes.test.ts @@ -176,6 +176,27 @@ describe('scanValueShapes (ADR-0104 D1 / #3438)', () => { ).not.toThrow(); }); + it('#15490: an undeclared key outranks a same-value member error — on address too, not just location', async () => { + // The sweep this fix owes measured every class the two readers cover, and + // found only `location` and `address` able to emit `unrecognized_keys` at + // all (the rest are strings, open records, arrays, or `z.unknown()`). It + // also refuted the reason `address` was believed immune: every member being + // OPTIONAL rules out a MISSING-member `invalid_type`, but says nothing about + // a WRONG-TYPED declared one — which still sorts ahead of the object-level + // issue. So the defect reaches this class too; the contrast case that made + // it look location-only just happened to carry no type error. + const engine = makeEngine({ + contact: [{ id: 'c1', addr: { street: 5, postal_code: '98101' } }], + }); + const report = await scanValueShapes(engine, silent); + + expect(report.blocking).toBe(1); + const addr = report.findings.find((f) => f.field === 'addr')!; + // Positionally this rejection reads `[invalid_type(street), unrecognized_keys]`. + expect(addr.detail).toContain('`postal_code` \u2192 `postalCode`'); + expect(addr.detail).not.toContain('expected string, received number'); + }); + it('the scan counts exactly what strict mode rejects — one predicate, not two', async () => { // The anti-drift property: every value the scan flags must also be a write // rejection under the strict gate, and every value it passes must write. @@ -186,6 +207,25 @@ describe('scanValueShapes (ADR-0104 D1 / #3438)', () => { const report = await scanValueShapes(engine, silent); expect(report.blocking).toBe(1); + // #15490: the fixture above was already the renamed pair, but the pin + // asserted only the COUNT — so the operator-facing prescription was free to + // be the wrong half of the rejection and every gate stayed green. zod sorts + // the two missing-member `invalid_type` issues (`lat`, `lng`) ahead of the + // object-level `unrecognized_keys` one, so a positional `issues[0]` read + // reported `expected number, received undefined` and threw away the rename + // the schema curates an `aliases` map to produce. Assert the PRESCRIPTION, + // not just that something was flagged: `detail` is documented as "the + // prescription an author acts on". + const geo = report.findings.find((f) => f.field === 'geo')!; + expect(geo.detail).toContain('`latitude` \u2192 `lat`'); + expect(geo.detail).toContain('`longitude` \u2192 `lng`'); + expect(geo.detail).toContain('this location value'); + // The half that was being shown instead — naming it keeps the pin able to + // see a regression back to the positional read. + expect(geo.detail).not.toContain('expected number, received undefined'); + // Customer-facing refusal text carries no internal issue id. + expect(geo.detail).not.toMatch(/#\d+/); + expect(() => validateRecord(OBJECTS.contact, { ...flagged }, 'update', { valueShapeStrict: true }), ).toThrow(ValidationError); diff --git a/packages/objectql/src/validation/scan-value-shapes.ts b/packages/objectql/src/validation/scan-value-shapes.ts index aa0db9e06d..0293f6c1e7 100644 --- a/packages/objectql/src/validation/scan-value-shapes.ts +++ b/packages/objectql/src/validation/scan-value-shapes.ts @@ -48,7 +48,14 @@ export interface ValueShapeFinding { count: number; /** A few offending record ids, so an operator can go and look. */ sampleRecordIds: string[]; - /** The first parse issue seen — the prescription an author acts on. */ + /** + * The prescription an author acts on: the undeclared-key issue when the + * rejection carries one (it names the key AND the rename its schema + * curates), else the first issue seen. Deliberately not positional — + * zod sorts per-member issues ahead of the object-level one, so on the + * renamed `{latitude, longitude}` location this header names, the + * actionable message is last. + */ detail: string; }