fix(objectql): the value-shape detail prescribes the key rename, not the missing-pair type error - #16078
Conversation
…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
📓 Docs Drift CheckThis PR changes 1 package(s): 1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 16 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 9f904f92363123fb50f7540b7c627517cb79fc01 && git checkout 9f904f92363123fb50f7540b7c627517cb79fc01
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin c2520416cff04575dd3def789dd04a5745e3912b 290dfc53f88953aeb6efeb965476d19ca750962f && git checkout -B drift-repro c2520416cff04575dd3def789dd04a5745e3912b && git merge --no-ff 290dfc53f88953aeb6efeb965476d19ca750962f
node scripts/docs-audit/affected-docs.mjs --json c2520416cff04575dd3def789dd04a5745e3912b
|
Docs Drift Check verdict — a positive hit, read properlyRead at this PR's head
|
| page's promise | on base c463d03e0 |
at 290dfc53f |
|---|---|---|
location: undeclared key latitude "refused by name" |
scan reported Invalid input: expected number, received undefined — names no key |
names latitude -> lat, longitude -> lng |
address: postal_code "fails with a rename to postalCode" |
true only when no member is also wrong-typed | true in both cases |
The notable part: one of these lines was ALREADY falsified on origin/main, by the base code, not by this diff
The address sentence promises postal_code "fails with a rename to postalCode" without qualification. Measured on base, that held only for the fixture shape the page happens to show. Add a wrong-typed declared member and the promise broke:
address { street: 5, postal_code: '98101' } (base c463d03e0)
detail -> "Invalid input: expected string, received number" <- not a rename; the page's sentence was false here
This is the sweep's finding reaching the docs: all-optional members rule out a missing-member invalid_type, but not a wrong-typed declared one, which still sorted ahead of the undeclared-key issue. This PR closes that gap, so the page's unqualified sentence becomes true for the first time. That case is pinned in scan-value-shapes.test.ts.
⇒ No docs edit is owed, and none was made. The page states the rule my layer implements; the PR makes the implementation match the already-written rule. Editing it would be widening the PR for no gain.
content/docs/releases/v17.mdx — UNTOUCHED, and correct. Not edited, no follow-up owed.
Read at L776-795 and L1344 (read-only, release-owned). Its unrecognized_keys anchor is the wire error catalog: "Routes that validate with Zod stopped leaking Zod's issue codes: ... unrecognized_keys becomes unknown_field, and ... a missing required property now reports required instead of the invalid_type Zod uses for it". That is a different layer — it maps Zod issue codes to public FieldErrorCode values on EnhancedApiError.fields. This PR changes neither which Zod issues exist nor any code mapping; it changes which issue's message text becomes the objectql value-shape detail. The invalid_value_shape code itself is unchanged. L1344 (ui as an unrecognized_keys error) is metadata authoring, also unrelated. Nothing there is wrong, so there is no separate docs issue to raise.
Scope
This was a read. Nothing in the dispatch moved: the sweep still came first and still gated the blanket rule (16 classes measured, 2 can emit unrecognized_keys at all), both call sites are fixed (record-validator.ts:795 and :946, re-derived), and the detail assertion in scan-value-shapes.test.ts is in and ablation-proven.
Generated by Claude Code
Fixes #15490
An operator running
os migrate value-shapesover real legacylocationdata was toldInvalid input: expected number, received undefinedand left to derive a rename that edit distance cannot reach — whileLocationValueSchemahad already built the prescription (latitude->lat) and thrown it away.All evidence below is from commit
290dfc53f.The defect
A value-shape rejection was read positionally,
parsed.error.issues[0], at two sites — the card names one:record-validator.ts:795const detail = parsed.error.issues[0]?.message ?? 'invalid value shape';— the write path's warn-first / strict branchrecord-validator.ts:946return parsed.error.issues[0]?.message ?? 'invalid value shape';— inside the exportedvalueShapeViolation, which the scan importszod reports per-member issues before the object-level
unrecognized_keysone, so on a value whose keys were renamed the actionable message sorts last. Fixing only the second site would have left the first emitting the identical wrong prescription — which is exactly the drift the comment at:840already worries about, so both now share one reader.Deliverable 1 — the sweep, done first, and it changed what shipped
The card fenced its own suggestion: confirm the positional read does not mislead the other structured-JSON classes before changing it. Every class the two readers cover was measured through the real
valueSchemaFor, with a battery per class (missing members, undeclared keys, both at once, wrong base type):unrecognized_keys?invalid_type?issues[0]when it doesjsoncompositerepeaterrecordlocationinvalid_typeaddressinvalid_typevectorlookup/master_detail/user/treefile/image/avatar/video/audio16 classes measured; 2 can emit
unrecognized_keysat all. Onlylocationandaddressare backed by a key-closed object schema. The reference and file-reference classes are strings,composite/record/repeater/vectorare open records and arrays,jsonisz.unknown(), and the one deliberately loose object shape (FileValueSchema) never refuses a key.So preferring
unrecognized_keysis a no-op for the other fourteen — it cannot change a single character of their diagnosis, which is what makes it safe as a blanket rule rather than merely untested against them. And both classes it does reach curate analiasesmap, so the undeclared key is the half carrying the prescription in both.The sweep refuted the reason
addresslooked immuneThe triage read was that
addressdeclares every member optional, so noinvalid_typecan sort ahead of the undeclared-key issue. Measured, that holds only for the missing-member route. A wrong-typed declared member still emitsinvalid_type, and it still sorts first:So the defect reaches
addresstoo; the contrast case that made it look location-only just happened to carry no type error. That case is now pinned.End-to-end, through the real exported
scanValueShapes{latitude, longitude}locationInvalid input: expected number, received undefined— no renamelatitude->lat,longitude->lng{postal_code}address (the contrast that already worked)blocking=0, passedInvalid input: expected string, received numberpostal_code->postalCodeblockingcounts are identical before and after in every case.Finishing the pin
scan-value-shapes.test.tsalready used the exact value{ geo: { latitude: 1, longitude: 2 } }and asserted onlyreport.blocking === 1— neverdetail. The fixture was there; only the assertion was missing, which is why no gate could see this. It now asserts the prescription, and names the string that was being shown instead so a regression to the positional read is visible.Ablation (one shell; mutation proven on disk before measuring, restore proven after):
The 7 tests that passed in both legs are every
blockingcount and every accept/reject assertion — reverting the fix moved no verdict, which is the same fact from the other direction.No verdict moves
The same values are flagged, the same writes rejected or admitted, and the deployment gate opens on the same evidence. The helper is only reached after the
parsed.successbranch, so the accept/reject decision never consults it. Only operator-facing text changes: the scan finding'sdetail, the warn-first[value-shape]log line, and theinvalid_value_shapeerror'sdetailunder strict enforcement.Clause-2:
no— both limbs measured separately on@objectstack/objectql's built surface.c463d03e0, rebuilt, diffeddist/index.d.ts. With the content-hash chunk filename normalized (util-D3s8yRSa->util-mkEsVIjj, which moves because implementation bytes changed), the only remaining difference is the JSDoc onValueShapeFinding.detail. Declared-symbol sets are identical, 116 vs 116.valueShapeDetailis module-private and appears 0 times in the emitted.d.ts. The base rebuild was byte-identical to a snapshot taken before any edit, which confirms the base artifact. Restore proven byte-exact, and a final rebuild putdist/back to the head artifact.Verification (all at
290dfc53f)pnpm --filter @objectstack/objectql exec vitest run --maxWorkers=2 src/validation/scan-value-shapes.test.ts src/validation/record-validator.test.ts— Test Files 2 passed (2), Tests 104 passed (104)pnpm --filter @objectstack/objectql typecheck— exit 0, all three legs;check:test-typecheck: OK — 44 file(s) / 242 error(s) / 69 pinned signature(s) held(ledger unmoved)pnpm lint(whole repo,eslint . --no-inline-config) — exit 0node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, provenance line checked: "gate list derived from the tree of 'objectstack-ai/objectstack' at commit 290dfc5". All 57 run: 55 exit 0; the 2 non-zero are both exit 3 = PREREQUISITE NOT MET, which each gate documents as "NOT a pass and NOT a finding":check:dual-build-cjs-loads— reads built output for 54 packages with nodist/; its own--self-testpassed 93 casescheck:type-check-debt— needs the whole workspace build closure. The targeted answer is the packagetypecheckabove, green with its debt ledger unmovedCI runs both with the full build.
Out of scope
packages/spec/src/data/default-value-shape.ts:120carries a third positionalissues[0]read producing adetailstring. It is a different function on thedefaultValueauthoring path rather than the stored-value path this card governs, so it is not touched here. Filed separately as #16077, with the measurement.Generated by Claude Code