Uh oh!
There was an error while loading. Please reload this page.
fix(spec): reach all three persisted view spellings in every view-family conversion - #13054
Conversation
…ngs (red) Reproduces #13031: 16 of 22 cases fail — every view-family conversion walks only the container keys, so a stored ViewItem record or flattened overlay escapes applyConversionsToStoredItem('view', ...) entirely. The 6 green cases are the container legs and the negative shapes.
…ily conversion
Adds mapViewPayloads to conversions/walk.ts — one shared walk that
discriminates the container, the ViewItem record (viewKind/config) and the
flattened overlay by ViewMetadataSchema's own discriminators, and hands each
conversion the list/form payload wherever it lives, labelled by family.
Adopted by all five view-family conversions. Before this, each walked only
the container keys, so a stored ViewItem record or flattened overlay escaped
applyConversionsToStoredItem('view', ...) entirely.
Fixes#13031📓 Docs Drift CheckThis PR changes 1 package(s): 11 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
What this run could not see
Coarse fallback — 126 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 59de31d472d8d09584159369c4d7a4eb147b6e74 && git checkout 59de31d472d8d09584159369c4d7a4eb147b6e74
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 8f10a79f7aef471482fdb4761ae150014a7f355a 13e03bd4a8820e1e5119b733b80d533526cbc59b && git checkout -B drift-repro 8f10a79f7aef471482fdb4761ae150014a7f355a && git merge --no-ff 13e03bd4a8820e1e5119b733b80d533526cbc59b
node scripts/docs-audit/affected-docs.mjs --json 8f10a79f7aef471482fdb4761ae150014a7f355a
|
Uh oh!
There was an error while loading. Please reload this page.
Fixes#13031
ViewMetadataSchemaaccepts three persistedviewbody shapes and all three land insys_metadatarows, but every view-family conversion walked only the container keys. For a stored ViewItem record or a flattened overlay the entire chain replayed byapplyConversionsToStoredItem('view', row)was a no-op — the row kept its historical shape while the conversion layer reported it canonicalized, and the rehydration parse then refused exactly what had never been rewritten. That is the "row that once worked breaks with no author in the loop" casestored.ts's module doc says the replay exists to prevent.Premise, re-measured on the base commit (8cb96ec)
All three halves of the card hold:
ViewMetadataSchema(packages/spec/src/ui/view.zod.ts) publishesVIEW_METADATA_BRANCHES = ['viewItem', 'container', 'listOverlay', 'formOverlay']— the container, the ViewItem record, and the flattened overlay in its list and form arms.applyConversionsToStoredItem('view', item)(packages/spec/src/conversions/stored.ts) replays the full chain,includeRetired: true, over{ views: [item] }.mapCollection(stack, 'views', ...)call sites exist inregistry.ts, and they are exactly the five conversions the card names. Each read onlyview.list/view.form/view.listViews/view.formViews. Enumerated by reading the chain, not from the dispatch list — the grep for view-collection call sites returns those five and nothing else.The gap, reproduced first
The pin file was written and run against the unmodified chain before any implementation:
The 6 green were the container legs and the negative shapes; every ViewItem-record and flattened-overlay case was red.
The walker
mapViewPayloadsinpackages/spec/src/conversions/walk.ts, following theregion-slots.tscentralization precedent in the same directory family: one walk, so reach cannot differ per conversion. It hands each conversion the list/form payload wherever it lives, labelled with its family.Discrimination uses
ViewMetadataSchema's own discriminators, in the union's own member order, so the walk and the parse cannot disagree about what a body is:viewKindpresent andconfigis an object — member 1's discriminated unionconfiglist/form/listViews/formViews— thecontainerHasAViewrefinementviewKindpresent,configabsent, no container slot — theconfig/list/form/listViews/formViewsguards pinned toundefinedon both inline armsA body matching none of the three passes through untouched — this walker never manufactures shape.
The list-or-form question the card left open is not ambiguous, and the schema answers it. Members 3 and 4 are literally
ListViewSchema.extend(flattenedViewOverlayFields()).strip()andFormViewSchema.extend(...), so "the top level is the payload" is what the schema declares rather than something inferred; which family it is comes fromviewKind, which #7741 made REQUIRED on both arms precisely because the object-bound read paths match onobject+viewKind. No key needed a judgement call, so nothing is guessed here.The family label is load-bearing, not informational. These conversions are shape-scoped and two of them strip a key that is inert on one family and live on the other —
ariais retired on a form and live on a list,datathe reverse. A walk that could not tell the two apart would delete live keys.view-inert-keys-removedselects its key set from the label, and a test pins that a list record keeps itsaria.Adoption table — conversion x spelling x pinned-by-test
Every cell is pinned by a case in
packages/spec/src/conversions/view-spelling-walk.test.ts; container cells additionally keep their pre-existing fixture inregistry.ts, unchanged.view-visibleOn-to-visibleWhen(form)view-inert-keys-removed(both families)view-list-passthrough-keys-removed(list)view-export-options-pdf-removed(list)form-view-option-default-removed(form)Plus: a cross-spelling agreement case asserting all four container/record/overlay routes produce the identical payload; an overlay identity case pinning that no key any view conversion strips collides with
name/object/viewKind/label/columnState/isDefault/order/scope/owner(so a later conversion whose key does collide fails loudly rather than deleting a row's binding); an idempotence case; and a copy-on-write case asserting the same row reference comes back when nothing converts.Contract section
Clause-② PATH leg is hit — the diff lives in
packages/spec/src/**.CONTENT reading: the accept set does not move. This is data-at-rest canonicalization catching up to shapes
ViewMetadataSchemahad already ruled on. No schema is edited, no member's strictness changes, no key becomes newly authorable or newly refused. What changes is only which stored bodies the rewrite layer reaches before a parse sees them — strictly more rows arriving canonical, never a body newly accepted or newly rejected. Evidence that the surface really did not move:check:authorable-surface,check:api-surface,check:docs,check:export-originsandcheck:upgrade-guideare all green with no regeneration (all 14 generated artifacts reported up to date), and the container legs, including every notice path, are byte-identical in behaviour — their fixtures inregistry.tsare untouched and still pass.Blast radius: explicitly UNMEASURED
The card asked this to be stated either way, and the honest answer is that nothing bounds the real-deployment population of pre-protocol ViewItem-record or flattened-overlay rows:
viewKindanywhere underexamples/, and the CLImigrate-metae2e covers no ViewItem-record row. So no fixture, seed or test in this repo carries a stored view row in either new spelling, and nothing here can say how many such rows exist in realsys_metadatatables.Sizing this would need a query against real deployments, which is outside a spec change. The fix is worth landing regardless of the count, because its cost is bounded (the container path is provably unchanged) while the failure it removes is silent.
Verification
Union run on the final commit
13e03bd4a.pnpm --filter @objectstack/spec test—Test Files 441 passed | 1 skipped (442),Tests 11711 passed | 1 skipped (11712)pnpm --filter @objectstack/spec typecheck— all three legs green;check:test-typecheck: OK — @objectstack/spec's test layer compiles under packages/spec/tsconfig.test.json. Coverage proven rather than assumed:tsc -p tsconfig.test.json --listFileslists all three edited files, with zero errors attributed to any of them.pnpm --filter @objectstack/spec check:generated—All 14 generated artifacts are up to date.pnpm check:type-check-debt—31 ledger entr(ies) re-measured, 1570 raw tsc error(s) total, none above its recorded number. surplus: nonemetadata-protocol2010 passed / 10 skipped,metadata674 passed,metadata-core260 passed, CLImigrate-meta.e2e14 passedpnpm lint(full repo,eslint . --no-inline-config) — exit 0, no narrowing claimedpnpm check:nul-bytesgreen, plus a direct control-character scan over all four changed files (no matches)node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstackand re-derived on the final tree; all 35 named families run green. Two returnedPREREQUISITE NOT METon first run and are reported as such rather than as passes:check:doc-formula-expressions(needed@objectstack/formulaand@objectstack/lintbuilt — built them, then green) andcheck:dev-prereqs(needed the workspace closure — built it, then green).check:half-statesremains a non-reading: the container's GitHub token is a proxy placeholder, so the gate swept nothing. That is no reading, not a green.Ablation — the new fixtures are load-bearing
Predicted before running: cutting the flattened-overlay leg out of
mapViewPayloadsreds the 7 flattened cases plus the cross-spelling agreement case, and leaves the record and container legs green — 8 red, 14 green.Observed exactly that, on exactly those 8 cases. The mutation was confirmed on disk rather than trusted from an editor exit code: the removed anchor text went 1 occurrence to 0, the injected marker 0 to 1, and the blob hash moved off the HEAD blob (guarded so a zero-match edit aborts the run). Restore was proved the same way, not by an exit code — restored blob hash equal to the HEAD blob,
git diff HEADempty,git status --porcelainempty, marker back to 0 occurrences — and the restored run is green again at 22/22. No rebuild was involved on either leg: these tests import the conversion source directly (./stored.jsresolved tosrc/by vitest), so nodist/sits between the mutation and the reading.Files
packages/spec/src/conversions/walk.ts(the walker),packages/spec/src/conversions/registry.ts(five adoptions),packages/spec/src/conversions/view-spelling-walk.test.ts(new), one changeset. Nothing outside the dispatched surface.Generated by Claude Code