Skip to content

fix(metadata): getViewsByObject() expands aggregated view containers instead of answering empty - #14206

Merged
os-support-ai merged 4 commits into
mainfrom
claude/issue-13913-getviewsbyobject-container-expansion
Sep 1, 2026
Merged

fix(metadata): getViewsByObject() expands aggregated view containers instead of answering empty#14206
os-support-ai merged 4 commits into
mainfrom
claude/issue-13913-getviewsbyobject-container-expansion

Conversation

@claude

@claude claude Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Fixes #13913

The defect

MetadataManager.getViewsByObject() reads this.list('view') — the manager's
own loader-based store (this.registry plus every registered loader). That
is a completely different store from the sys_metadata rows the REST route
(GET /meta/view?object=) reads through
ObjectStackProtocolImplementation.getMetaItems.

#13407 taught getMetaItems to expand a runtime-authored aggregated
defineView container inline. This exit never calls getMetaItems and had no
equivalent step, so the very container #13407/#13929 made visible on the wire
still answered empty here — for every internal/SDK caller that uses this
entry point rather than the route.

What was NOT done, and why

The repair

getViewsByObject() expands aggregated containers inline, per read, and
keeps the filter as-is — what changes is that the filter now also sees each
container's expansion, whose items carry the viewKind + object pair it has
always tested.

Registry-free, mirroring #13407's choice at the other exit and for the same
reason: MetadataManager's registry is process-wide, so a read must not graft
rows into it. Already-present names win, so a container whose expanded ViewItems
were registered by a source registrar (the ObjectQL boot loop, the artifact/HMR
loader) still answers with those registered, fully-enriched items and gains
nothing new.

The cross-package reuse question, measured

The card asked whether expandRuntimeViewContainer could be shared from
packages/metadata-protocol, or whether packages/metadata needs a local
equivalent. Neither, as posed — the reusable substance already sits below
both packages. Three readings:

Reading Result
Dependency direction @objectstack/metadata-protocol depends on @objectstack/metadata (its package.json dependencies). An import the other way inverts an existing edge and closes a cycle.
Is the helper exported? No — expandRuntimeViewContainer is a private method on ObjectStackProtocolImplementation (protocol.ts), reachable only through this, and it calls two further private members.
Would promoting it widen a public surface? Yes — and it would still be unreachable from here, because of the edge direction. So the widening would buy nothing.

A shared export from metadata-protocol is the wrong direction and is not
taken.
But this is not duplication either: the canonical expansion
primitives (isAggregatedViewContainer, expandViewContainer) live in
@objectstack/spec, one level below both packages, and this package already
imports them (plugin.ts). They are reused unchanged.

What is genuinely local is the ~6-line object-derivation chain — and that is
the part that has silently drifted three ways: the ObjectQL boot loop keys off
the registration name, plugin.ts walks two levels
(list.data.objectform.data.object), and protocol.ts walks four since
#13407 (objectlist.data.objectform.data.objectname). This PR
gives packages/metadata one spelling of it,
deriveViewContainerObject() in the new view-container-expansion.ts, so the
drift has a single place to be repaired rather than a third private copy to fall
behind.

📌 For #13912's implementer: that card's surface (plugin.ts, 2 sites) still
carries the pre-#13407 two-deep chain, which is exactly why a container with
a top-level object and no list.data.object is dropped there. Import
deriveViewContainerObject from ./view-container-expansion.js — same package,
relative import, no public-surface change — rather than adding a fourth copy.

Clause-② : no. The new module is internal: it is not re-exported from
packages/metadata/src/index.ts, so @objectstack/metadata's published surface
is unchanged, and no accept set moves.

Tests

New pin: packages/metadata/src/metadata-manager-views-by-object-container.test.ts
(7 cases). Every assertion drives getViewsByObject() itself — a pin written
against GET /meta/view?object= would have gone green on main without
touching this bug, since #13929 already repaired that exit. The fixture is the
card's own shape: a top-level object and no list.data.object.

Both admission routes are driven: a container registered in the in-memory
registry, and a container arriving from a DatabaseLoader over a mock
sys_metadata store.

pnpm --filter @objectstack/metadata exec vitest run --maxWorkers=2
  Test Files  40 passed (40)
       Tests  645 passed (645)

Ablation 1 — revert the repair (direction re-predicted, then measured)

Run on the committed tree; the mutation was confirmed on disk by marker
counts and a git hash-object comparison against the HEAD blob before any
reading was taken, and the restore was proven by the file's hash returning to
the HEAD blob with git diff HEAD empty. No rebuild is involved: the pin
imports ./metadata-manager.js relatively, so vitest resolves it to the
package's own source rather than through dist — independently evidenced by the
very first pin run, which observed the new expansion behaviour with
@objectstack/metadata itself never built.

Reverting getViewsByObject to its pre-#13913 body: 4 red / 3 green.

Case Ablated
answers with the container EXPANSION, not empty RED (expected 0 to be greater than 0)
derives the binding from the top-level object when list.data.object is absent RED
reaches a container that arrived through a LOADER, not only the registry RED
contributes only the names the store does not already hold RED
answers with EXPANDED items and never the container itself (#7163) green — vacuously, see below
CONTROL: a non-container ViewItem still resolves unchanged green
CONTROL: a registered expanded item is returned BY IDENTITY, never shadowed green

The first ablation corrected the test. The dedupe case was originally one
control asserting both the full answer and the registered item's identity; it
went red, because pre-fix the answer is that registered item alone. A control
that goes red under ablation is not a control, so it was split: the set
assertion became an ordinary case, and the both-directions half is the object-
identity assertion. The prediction above is the re-prediction made after that
split, and it matched exactly.

Ablation 2 — apply the forbidden one-line "fix"

The #7163 case is green under Ablation 1 only vacuously (the pre-fix answer is
empty, so nothing can be a container). It guards a different mutation, so it was
ablated against that one: dropping v.viewKind && from the filter — the
tempting one-liner — makes the container answer as a view.

Measured: 5 red, including answers with EXPANDED items and never the container itself (#7163). The guard is known to fire, not assumed to.

Gates

Derived at the actual diff with
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands
(the script takes its own change set from the merge base; no hand-built path
list was fed to it) and run with exit codes captured by redirect before any
pipe.

32 of 35 green. 3 are PREREQUISITE NOT MET — recorded as NOT MEASURED, never as a
pass
, on each gate's own instruction; all three need a whole-repo pnpm build,
which is CI's run, and each says in its own words that it is "NOT a pass and NOT
a finding". Verdicts are quoted from the lines the gates print, not from a bare
$?.

Gate Exit Reading
the other 32 0 green
node scripts/check-test-completeness.mjs 3 NOT MEASURED — takes a saved turbo run test log; the family names it with no argument. Its own text: "running the family locally, record this gate as NOT MEASURED".
pnpm check:dual-build-cjs-loads 3 NOT MEASURED — "PREREQUISITE NOT MET — this gate reads built output, and some package has no dist/." 61 packages unbuilt.
pnpm check:type-check-debt 3 NOT MEASURED — --re-measure refuses without the built closure: "measuring now would not fail, it would silently measure a DIFFERENT WORLD". Its sibling pnpm check:type-check-coverage ran green (exit 0).

Note check:engine-double-contract, check:cross-package-test-inputs and
check:test-source-alias are all green — the new test file adds no cross-package
read and no aliased import.

The type-check ratchet, measured directly

@objectstack/metadata has no typecheck script; it is a shrink-only DEBT
ledger entry frozen at 89 raw errors. Its dependency closure is built here,
so the number was measured rather than deferred:

pnpm --filter @objectstack/metadata exec tsc --noEmit -p tsconfig.json
  89 errors  (ledger: 89 — unmoved)
  errors in the three files this PR touches: 0

And the files were proven to be in the program rather than silently excluded —
tsc --noEmit --listFiles names all three:
view-container-expansion.ts, metadata-manager-views-by-object-container.test.ts,
metadata-manager.ts.

Repo-wide lint

Run in full, not narrowed:

pnpm exec eslint . --no-inline-config --format json
  5644 files linted · 0 errors · 0 warnings   (exit 0)

All three touched files appear in that run at 0/0.

Mergeability

Re-checked at the end against a freshly fetched origin/main (c41b42e8):
git merge-tree --write-tree origin/main HEAD returns a clean tree, exit 0 — no
conflicts
, so nothing was merged in. The three upstream commits since the base
touch driver-sql, lint, and one docblock in packages/spec/src/shared/identifiers.zod.ts;
none overlaps this diff behaviourally. The gate family was re-derived after that
fetch and is byte-identical to the family run above.

Adjacent defect found and filed, NOT fixed here

readListUncached() drops every loader-held item whose stored body has no
top-level name — which an aggregated view container has none of, by design. So
a container persisted through register() is invisible to list('view')
entirely after a restart, before this exit ever sees it. Measured with a positive
control and filed unassigned as #14205. Out of scope: different function,
different failure path, and repairing it changes list() for every metadata type.


Generated by Claude Code

@github-actions github-actions Bot added the size/m label Sep 1, 2026
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

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

  • content/docs/concepts/metadata-lifecycle.mdx (via MetadataManager (symbol, a top-level class))
  • content/docs/plugins/adding-a-metadata-type.mdx (via MetadataManager (symbol, a top-level class))
  • content/docs/protocol/kernel/metadata-service.mdx (via MetadataManager (symbol, a top-level class))

2 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/implementation-status.mdx (via MetadataManager (symbol, a top-level class))
  • content/docs/releases/v17.mdx (via MetadataManager (symbol, a top-level class))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

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 c41b42e8db6efdc7091e9c320c0598cd30c7777dpackageMentionDocs.

Which tree this was computed on

This run read content/docs from 76764bb12eb24c3326829918c002402033184cbc — the merge of head 523d6e4affbe94d27e93724ffc6618c9795c3b87 into base c41b42e8db6efdc7091e9c320c0598cd30c7777d, 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 76764bb12eb24c3326829918c002402033184cbc && git checkout 76764bb12eb24c3326829918c002402033184cbc
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin c41b42e8db6efdc7091e9c320c0598cd30c7777d 523d6e4affbe94d27e93724ffc6618c9795c3b87 && git checkout -B drift-repro c41b42e8db6efdc7091e9c320c0598cd30c7777d && git merge --no-ff 523d6e4affbe94d27e93724ffc6618c9795c3b87

node scripts/docs-audit/affected-docs.mjs --json c41b42e8db6efdc7091e9c320c0598cd30c7777d

⚠️ 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 c41b42e8db6efdc7091e9c320c0598cd30c7777d → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Sep 1, 2026
@os-support-ai
os-support-ai marked this pull request as ready for review September 1, 2026 12:52
@os-support-ai
os-support-ai added this pull request to the merge queue Sep 1, 2026
Merged via the queue into main with commit f7be03f Sep 1, 2026
34 checks passed
@os-support-ai
os-support-ai deleted the claude/issue-13913-getviewsbyobject-container-expansion branch September 1, 2026 13:15
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

Development

Successfully merging this pull request may close these issues.

metadata-manager: getViewsByObject() reads a separate backing store never touched by the #13407 runtime-container-expansion fix

2 participants