Skip to content

fix(metadata): FilesystemLoader keys loader-held items by the name it can resolve - #14497

Merged
os-musk merged 1 commit into
mainfrom
claude/issue-14341-filesystem-loader-keyed
Sep 2, 2026
Merged

fix(metadata): FilesystemLoader keys loader-held items by the name it can resolve#14497
os-musk merged 1 commit into
mainfrom
claude/issue-14341-filesystem-loader-keyed

Conversation

@os-musk

@os-musk os-musk commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Fixes #14341

FilesystemLoader.loadMany() globbed files and pushed bodies, throwing away the path it had just read, so MetadataManager.admitLoaderItems() fell back to keying by body.name and dropped every nameless body this loader held — the #14205 defect, unrepaired here. This implements the optional loadManyKeyed() member PR #14340 added.

The rule, in one sentence

An item is keyed by this loader's own name-to-path derivation — the basename minus extension, the same derivation list() reports — only where that derivation is a bijection for the file (it sits directly under ROOT/TYPE/ and carries an extension findFile() tries, so findFile(type, key) resolves back to that same file); every other shape keeps the pre-#14205 behaviour verbatim, keyed by body.name when it has one and dropped when it has none.

Why the rule stops there

The card asked which part of a path is the metadata key, and triage settled it "by consistency rather than invention: the key is whatever findFile() / list() already treat as the metadata name for that path — if those two disagree with each other, that disagreement is the deliverable's first finding and a stop-and-report". Measured on origin/main @ 253da34c4, they do disagree, in two shapes loadMany() reads today:

shape list() reports findFile() resolves
ROOT/TYPE/flat.json (and dotted.config.json) flat / dotted.config the same file — agree
ROOT/TYPE/crm/account.json account nothing under account; only crm/account reaches it
ROOT/TYPE/extensionless extensionless nothing — it always appends one of five extensions

That was reported as the stop-and-report, and the PM ruled option D (comment 5507163442), knowingly over triage's "a nested path keeps whatever list() reports for it today": keying the disagreeing shapes by their basename would mint names get() / load() / exists() cannot open — the card's own fence, "keying items under names nothing else uses … is worse than today's honest drop" — and two directories holding one basename would collide in silence, since admitLoaderItems() keeps the first and says nothing.

The derivation defect itself is #14486 (one shared name-to-path function for list(), findFile() and loadManyKeyed(), option C). It moves listNames() output, so it is not attempted here. The nested-nameless drop is pinned as a RECORD: case precisely so #14486 inverts it deliberately, in a visible diff.

remote-loader.ts is untouched: triage ruled it "explicitly out of scope and not filed as its own card" (the key is absent from the wire response), so the card closes on this PR rather than deferring a half.

One consequence, deliberate

A flat file whose body.name disagrees with its basename is now keyed by the basename. That is #14205's rule — identity is the key the store holds an item under, not body.name — applied to this loader, and it aligns MetadataManager.list() with listNames() for that shape. Pinned as keys a flat file by its BASENAME even when body.name disagrees.

Shape of the change

  • loadManyKeyed() is additive; loadMany()'s signature and answer are unchanged.
  • Both now share one private walk (loadManyEntries() returning file/data pairs), because the interface requires loadManyKeyed()'s data to be "the same body loadMany() would return for the item" and a second copy of the glob is how that quietly stops being true. A CONTROL: case pins that loadMany() still answers with bodies only, envelope not leaked.
  • The basename derivation lives in one helper (nameFromFilename) now used by both list() and the key rule, and findFile()'s extension list is the shared RESOLVABLE_EXTENSIONS, so the resolvable set cannot drift from the set the key rule will mint from. Neither list() nor findFile() changes observable behaviour.
  • Generic parameters are written here in square brackets: MetadataKeyedItem[T] is { readonly name: string; readonly data: T }, matching DatabaseLoader and MemoryLoader.

Verification — union run on 940b6c257

pnpm --filter '@objectstack/metadata^...' build first (VERDICT command-exit 0), then:

  • pnpm --filter @objectstack/metadata test43 files, 677 tests, all passed; the new pin file is 14/14.
  • pnpm lint (full repo, eslint . --no-inline-config) — exit 0. Not narrowed.
  • pnpm check:nul-bytes — OK, 7916 text files scanned, no raw control bytes. pnpm check:error-status-conformance — exit 0.
  • Gate union re-derived on this tree with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands (3 paths vs merge base e5812fa43): 35 commands, identical to the dispatched list. 31 exit 0; four exited 3 = NOT MEASURED in the gate's own words, each for an input this container cannot supply and CI does: check-test-completeness (wants a saved turbo run test log), check:dual-build-cjs-loads and check:type-check-debt (want a full-repo pnpm build), check-half-states (wants GitHub API access; repo-scoped REST is 403 in this container). None is a finding.
  • The one thing this diff could actually move in check:type-check-debt was measured directly instead: tsc --noEmit -p packages/metadata/tsconfig.json still reports exactly 89 errors, the number the ledger records, and neither of my two files appears in any of them; --listFiles confirms both are in that program. Per [finding] @objectstack/metadata has no typecheck script, and its own tsconfig program does not compile: 89 pre-existing errors across 12 test files, checked by nothing #14342 that program is not green, so the test-layer type reading stays NOT MEASURED and vitest is the runtime evidence.

Ablation

Predicted before the run: remove the loadManyKeyed implementation ⇒ the flat-nameless pin RED, the named pins GREEN.

Mutation proved on disk before measuring (async loadManyKeyed[T renamed, removed-text count 1 → 0, injected-text count 1, file hash 14fe5e4c0c994970). Observed, manager-level describe: 2 failed / 2 passed — RED exactly on a flat nameless body reaches list() end to end and listDiagnosed() counts it and stays complete-and-not-degraded, GREEN on the named CONTROL: and the nested-nameless RECORD:. The predicted direction, on the discriminating layer. Whole file ablated: 10 failed / 4 passed — the loader-level cases call loadManyKeyed() directly and throw once it is gone, which is the ablation biting but not a discriminating signal.

No rebuild leg applies: the pin imports ./filesystem-loader.js and ../metadata-manager.js relatively, so vitest compiles the mutated source — proved by the run turning red at all. Restore verified by bytes, not by exit code: git checkout HEAD -- ABSOLUTE_PATH (absolute-path trap … EXIT INT TERM armed throughout), git hash-object back to the HEAD blob 14fe5e4c, git diff HEAD empty, injected-text count 0.

Changeset: @objectstack/metadata: patch.


🤖 Generated with Claude Code

https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68

Generated by Claude Code


Generated by Claude Code

… can resolve (#14341)

`loadMany()` discarded the path it had just read, so `MetadataManager` fell back
to keying by `body.name` and dropped every nameless body held by this loader —
the #14205 defect, unrepaired here.

`loadManyKeyed()` keys an item by this loader's own name-to-path derivation (the
basename minus extension, the same one `list()` reports) ONLY where that
derivation is a bijection for the file: directly under `ROOT/TYPE/`, with an
extension `findFile()` tries. Every other shape keeps the pre-#14205 behaviour
verbatim — keyed by `body.name` when it has one, dropped when it has none —
because `list()` and `findFile()` disagree there, and a minted key that
`get()`/`exists()` cannot open is what the card refused.

Both methods now share one file walk, so the body `loadManyKeyed()` reports can
never drift from the one `loadMany()` returns.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
@github-actions github-actions Bot added the size/m label Sep 2, 2026
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/metadata, touching 9 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/protocol/kernel/metadata-service.mdx (via FilesystemLoader (symbol, a top-level class), loadMany (symbol, a method of class FilesystemLoader))
What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 12 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 9c7d9d4b343f5ded5b51b757cb4251d1f68cd378packageMentionDocs.

Which tree this was computed on

This run read content/docs from a87878a10fdeb5025812db499cae7e311011cfca — the merge of head 940b6c2579c73286a8f1438e1d11553b45b2d518 into base 9c7d9d4b343f5ded5b51b757cb4251d1f68cd378, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin a87878a10fdeb5025812db499cae7e311011cfca && git checkout a87878a10fdeb5025812db499cae7e311011cfca
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9c7d9d4b343f5ded5b51b757cb4251d1f68cd378 940b6c2579c73286a8f1438e1d11553b45b2d518 && git checkout -B drift-repro 9c7d9d4b343f5ded5b51b757cb4251d1f68cd378 && git merge --no-ff 940b6c2579c73286a8f1438e1d11553b45b2d518

node scripts/docs-audit/affected-docs.mjs --json 9c7d9d4b343f5ded5b51b757cb4251d1f68cd378

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 9c7d9d4b343f5ded5b51b757cb4251d1f68cd378 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

os-musk commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator Author

Landing provenance (engine execution seat, session_0112hMx9hjJ9BgB28X97DS68) — flipped to ready and auto-merge (squash) armed at 09:57Z on head 940b6c257.


Generated by Claude Code

@os-musk
os-musk added this pull request to the merge queue Sep 2, 2026
Merged via the queue into main with commit 2e471e3 Sep 2, 2026
35 checks passed
@os-musk
os-musk deleted the claude/issue-14341-filesystem-loader-keyed branch September 2, 2026 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants