Uh oh!
There was an error while loading. Please reload this page.
fix(metadata): key loader-held items by the row key, not by body.name - #14340
Conversation
…#14205) `readListUncached()` and its no-catch sibling `listForIndex()` merged each loader's answer keyed by `body.name`, admitting an item ONLY when the stored body carried a string `name`. A metadata body is not required to name itself: `register(type, name, data)` takes the key as its ARGUMENT, and `assertMetadataRegisterContract` explicitly allows a document with no `name` of its own. An aggregated `defineView` container is that shape by design — its identity is the target object, carried in the row's `name` COLUMN — so a container written by `register('view', OBJECT, container)` vanished from `list('view')` at the next restart, with `listDiagnosed()` calling the short answer complete. A loader-held item's identity is the key its store holds it under, so the manager asks the loader for that key instead of guessing it from the body: `MetadataLoader` gains an OPTIONAL `loadManyKeyed()` returning (name, body) pairs, implemented by `DatabaseLoader` (row `name` column) and `MemoryLoader` (storage map key). The key travels beside the body and is never folded into it, so nothing synthesises a `name` and the register contract's `data.name` check is untouched. Loaders without the method keep the previous keying verbatim. `MetadataManager.loadMany()` is deliberately unchanged: its `body.name` test is a de-duplication guard, not an admission gate, so it never had this defect. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
📓 Docs Drift CheckThis PR changes 1 package(s): 3 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 2 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 12 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 e8a19d3276837b3de70aca58a646ab578e9b0764 && git checkout e8a19d3276837b3de70aca58a646ab578e9b0764
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 72adb7fcb6b6f48b656047835ed3852f314d9de1 3cb0b0e257bf74b134756ffff96f097cfeed9c9e && git checkout -B drift-repro 72adb7fcb6b6f48b656047835ed3852f314d9de1 && git merge --no-ff 3cb0b0e257bf74b134756ffff96f097cfeed9c9e
node scripts/docs-audit/affected-docs.mjs --json 72adb7fcb6b6f48b656047835ed3852f314d9de1
|
…ve published surface The diff widens the published surface additively: `@objectstack/metadata`'s entry gains a named type (`MetadataKeyedItem`) and `MetadataLoader` gains an optional member (`loadManyKeyed?`). This repo's precedent for additive public-surface widening is `minor`, not `patch` (R12: #14262's `job-handler-data-reach.md` and #14247, both `"@objectstack/runtime": minor`). Front matter only; the changeset body is byte-identical. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
os-musk
commented
Sep 2, 2026
Provenance for the ready flip + auto-merge: contract review PASS in-seat at tier on Generated by Claude Code |
Fixes#14205
All readings below were taken on branch HEAD
f5e55ff20, with a clean working tree.The defect
MetadataManager.readListUncached()— and its no-catch siblinglistForIndex(), which builds the endpoint index — merged each loader's answer into the result set keyed bybody.name, admitting an item ONLY when the stored body carried a stringname.A metadata body is not required to name itself.
register(type, name, data)takes the key as its ARGUMENT, andassertMetadataRegisterContractsays so in as many words: "A document with NOnameof its own is fine — the argument is the key". An aggregateddefineViewcontainer is exactly that shape — no ownnameby design, its identity being the target object, carried in the row'snameCOLUMN — andDatabaseLoader.rowToData()returns the stored body without folding the column into it.So a container written by
register('view', OBJECT, container)lived in the registry for the life of the process and was written tosys_metadata, then disappeared at the next restart: cold registry, only the loader answering, andlist('view')refused the row.listDiagnosed()reported that short answer as complete (degraded: false) because no loader had thrown.The shape chosen, and why
The triage ruling offered two mechanical shapes — "the loader returns (name, body) pairs or
rowToDataexposes the key beside the body". This PR takes the first, additively:MetadataLoadergains an optionalloadManyKeyed()returning(name, body)pairs, plus an exportedMetadataKeyedItemtype so an out-of-package implementor can name the return type.DatabaseLoaderimplements it from the row'snamecolumn;MemoryLoaderfrom its storage map key. Both share one query and one cache entry withloadMany(), so a keyedlist()read does not re-hit the database.MetadataManager.admitLoaderItems()prefers it and falls back to the previousbody.namekeying when a loader does not have it.Why optional rather than a widened
loadMany():MetadataLoaderis exported from this package's public entry and has implementors outside it —packages/objectql'sWritableFixtureLoaderandFixtureLoader(measured, see below). ChangingloadMany()'s return type breaks every one of them; an optional member breaks none.rowToData()is untouched, and the key is carried BESIDE the body rather than folded into it. Nothing synthesises anameinto a body that deliberately has none, so the register contract's refusal of a disagreeingdata.namekeeps meaning what it says. This is pinned directly (never synthesises a name into a body that deliberately has none).Nothing consumers see today changes shape. For any item that went through
register(), adata.namethat exists is required to EQUAL the key, so the keyed merge produces the identical map entry; what is new is only the entries the old gate refused.Per-site verdicts on the three
typeof itemAny.name === 'string'sitesmainreadListUncached()(was line 1101)listForIndex()(was line 1283)apirow now reachesbuildEndpointIndexMetadataManager.loadMany()(was line 2616)ifandresults.push(item)runs anyway. Pinned byCONTROL: loadMany() answered with the nameless body before the repair too, which is GREEN on the pre-repair tree and green under ablationA measured limit on the second site, stated because it is not what one would assume: the repair makes the nameless
apirow REACH the index, but it does not make the route answer.ApiEndpointSchemadeclaresnameREQUIRED, so the row is still excluded — atbuildEndpointIndex's own separate, declared door, and now loudly, aterrorlevel with its consequence. Before,listForIndex()dropped it upstream and the author got silence. The test asserts exactly that transition rather than a served route.Fixture triage
Three cases in
metadata-manager-degraded-list-cache.test.tsandmetadata-manager-list-diagnosed.test.tsspiedMemoryLoader.loadManyas a loader-walk counter. The walk moved toloadManyKeyed, so the spy counted 0 — which reads exactly like a cache hit. Triage verdict: change the spelling, to a counter that is spelling-independent (loaderWalks()sums both plural reads). That repair is direction-independent: those three cases are green in BOTH directions, measured under the ablation below.Reverse verification (ablation)
Mutation: force the
loadManyKeyedbranch off inadmitLoaderItems, which restores the pre-repair admission gate verbatim (the fallback IS the old code). Run with anEXIT INT TERMrestore trap, absolute paths fromgit rev-parse --show-toplevel.No rebuild is involved and none is owed: every test imports
./metadata-manager.jsrelatively, so vitest transformspackages/metadata/srcdirectly — nothing here resolves through a packageexportsentry intodist/.1 -> 0, injected-text0 -> 1,git hash-object042f99b9…(HEAD blob)-> c72fccba…5 failed | 29 passed (34)across four test filesgit checkout HEAD -- ABSOLUTE_PATH, then restored hash042f99b9…equals the HEAD blob andgit diff HEADis empty; deleted-text back to 1, injected-text back to 0Gates run locally, with their own verdict lines
Re-derived on the real change set:
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack(no paths passed; it read the change set from git itself — "9 path(s) vs merge base 909a441"). It named 30 families where the dispatch hint carried 21; the 9 additions all come from the.changeset/file the hint's two-path derivation could not see, and every one of them was run.All green (exit 0):
check:changeset-gate-self-tests·check:cross-package-test-inputs·check:doc-authoring·check:durability-log-level·check:logger-receiver-detach·check:objectql-double-limit·check:objectui-changeset·check:page-declaration-shape·check:pm-half-states·check:published-files·check:slot-lookup·check:test-source-alias·check:type-source-resolution·check:where-matcher·check:dispatcher-error-vocabulary·check-adr-0087-registration.mjs·check-changeset-no-major.mjs·check-ci-filter-parity.mjs·check-comment-mask-adoption.mjs·check-cross-package-test-inputs.mjs·check-empty-changeset.mjs·check-keyed-text-bounds.mjs·check-plugin-teardown-shape.mjs·check-shard-attestation.mjs·check-undeclared-dep-imports.mjs·docs-audit/check-affected-docs.mjs·docs-audit/check-drift-comment.mjs·pm/release-rehearsal-clone.mjs --self-testThree exited 3 = PREREQUISITE NOT MET, which each gate's own text says is NOT a red and NOT a finding. Recorded as NOT MEASURED locally, all three being CI-owned runs:
check-test-completeness.mjs— "grades a savedturbo run testlog, and no log was named … the local reading for this gate is NOT MEASURED"check-dual-build-cjs-loads— "this gate reads built output, and some package has no dist/ … 57 packages. This is NOT a pass: nothing was measured". Its self-test passed 93/93 first. The metadata package's own dual build is green (ESM + CJS +.d.ts+.d.cts,check-dts-emitted: 8/8)pm/check-half-states.mjs— needs repo-scoped REST, which this container is refused (/rate_limitanswers 200 with 15000 left while/repos/…answers 403). Independent confirmation of this seat's read-channel posture.pnpm check:pm-half-statesexited 0Other verdict lines:
pnpm --filter @objectstack/metadata test→Test Files 41 passed (41)·Tests 655 passed (655)pnpm --filter @objectstack/metadata build→DTS Build success·check-dts-emitted: @objectstack/metadata - 8/8 declared declaration file(s) presentturbo run build --filter='...@objectstack/metadata'over 44 packages in scope →Tasks: 68 successful, 68 total. A first attempt with a barepnpm --filter '...@objectstack/metadata' buildfailed onCannot find module '@objectstack/driver-memory'— a dependency of a selected dependent that the prefix filter does not select, not a consequence of this change; turbo, which pulls^build, is greenMetadataLoaderimplementors:pnpm --filter @objectstack/objectql exec vitest runovermetadata-service-roundtrip-conformance.test.ts,metadata-service-getobject-equivalence.test.ts,plugin.integration.test.ts→Tests 136 passed (136). None declaresloadManyKeyed, so all three exercise the fallback path unchangedpnpm lint(eslint . --no-inline-config, the whole repo, no narrowing) → exit 0grep -naPfor the C0 set plus DEL) → no hitsType checking: this package declares no
typecheckscript, soturbo run typechecknever selects it.tsc --noEmit -p packages/metadata/tsconfig.json --listFilesreports 0 errors in all 8 changed files, each confirmed present in the program by--listFiles; its other 89 errors are pre-existing, in untouched test files, under a config this repo does not run. The live type gate for the published surface is the DTS build above, which is green.Clause ② — re-declared UPWARD from the diff
The dispatch declared
no. Reading the diff, it is yes, additively: the published@objectstack/metadataentry gains a named type (MetadataKeyedItem) andMetadataLoadergains an optional member (loadManyKeyed?). Both are verified present in the emitteddist/index.d.ts.No accept/reject behaviour narrows anywhere: the member is optional, so no existing implementor of the published interface has to change, and all 44 dependents plus the two out-of-package implementors are green above.
Scope
Deliberately untouched, each held serial behind this card:
packages/metadata/src/plugin.ts(that is #13912's surface, out of scope here) anddatabase-loader.ts'srowToRecord/recordedAt/createdAt/updatedAtcasts (out of scope: #14037).FilesystemLoaderandRemoteLoaderdeliberately do not implementloadManyKeyedand keep the fallback — see the report on the card for the measured reason for each.Same-day churn accounted for: #13913 landed as PR #14206 on 2026-09-01, and its
metadata-manager-views-by-object-container.test.tsis an oracle consumer of this change. Work was done on the merged tree and that file passes here, unmodified.🤖 Generated with Claude Code
https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
Generated by Claude Code