Skip to content

fix(metadata-protocol): derive the reference graph from the type schemas instead of curating it - #9324

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-9190-reference-paths-derivation
Aug 17, 2026
Merged

fix(metadata-protocol): derive the reference graph from the type schemas instead of curating it#9324
os-zhuang merged 1 commit into
mainfrom
claude/issue-9190-reference-paths-derivation

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Part of #9190.

⚠️Part of, not Fixes: the derivation half is complete, but one residual class needs a packages/spec annotation this card's tier fence keeps out of scope, and it is named at the bottom. #9190 remains open for that decision.

The premise held, and it understated the defect

The card says REFERENCE_PATHS covers 7 target types against 20+ declared ones. Re-derived from the code rather than reconciled to the handed number: 27 declared types (DEFAULT_METADATA_TYPE_REGISTRY), 26 with a resolvable schema, 7 curated keys.

Then I checked the curated paths against the schemas they claim to describe. 34 of the 40 do not exist:

curated pathwhat the schema actually declares
app.navItems[], app.tabs[] (14 paths, 6 targets)AppSchema declares navigation and areas
agent.tools[], agent.tools[].namekey REMOVED in @objectstack/spec 17 (#3894); converts to { not: {} }
permission.objects[].name, objects[].objectobjects is a name-keyed RECORD, not an array
object.fields{}.referenceTothe field property is reference
dashboard.widgets[].object / .view / .objectName / .viewNameDashboardWidget has no such property at any depth
page.viewName, view.objectName, report.object, flow.object / .context.object / .trigger.object / .targetObjectnone exist

Only 6 paths were live, reaching 2 target types (object, skill). Because the app row was the only row for flow, dashboard and page, and the removed agent.tools the only one for tool, five of the seven advertised keys answered { references: [] } unconditionally — indistinguishable from a key that was never there.

That answer drives the admin "Used by" panel, whose empty state reads, verbatim from objectuimetadata-admin/i18n.ts:

'engine.edit.refsEmptyDesc': 'Nothing in the metadata graph points at this item. Safe to delete.'

The close is derivation, not more keys

REFERENCE_PATHS is deleted. packages/metadata-protocol/src/reference-sites.ts derives the index at module load from DEFAULT_METADATA_TYPE_REGISTRY joined to getMetadataTypeSchema() — the shape #7894 used to make the URL-spelling map non-recurring, so a newly declared type arrives covered instead of waiting for someone to remember it.

A property is a reference site for target T when both hold: its name spells T (T, camelCase, Name-suffixed, target-prefixed, or a plural), and its value is name-shaped (unconstrained string, array/record of those, or objects carrying a name). Three limbs feed it: every declared type's schema; SCHEMALESS_NODE_CONFIG_SCHEMAS attributed to flow (because FlowSchema declares nodes[].config as additionalProperties: {}, so limb 1 sees nothing inside it); and the residue below.

The unit is a PROPERTY, not a path.AppSchema.navigation is self-recursive, so an exhaustive path list is unbounded — a view named from a third-level nav group was invisible even where the curated path was spelled correctly. The walk finds the property wherever the document puts it and reports where it was actually found.

Result: 17 target types resolve real sites, up from 2 served. New coverage includes permission-to-object grants through the record key (which the old path grammar could not express at all), translation, dataset, action, report, doc, datasource, and flow-to-flow via subflow node config.

The rejected rule, pinned

A suffix rule (endsWith(Cap(T))) would have caught homePageId and defaultAgent. Measured against the real schemas it is ~15% signal: it reads displayField, nameField, startDateField, stageField and ~30 siblings as references to the field METADATA TYPE when every one names a field inside an object, plus fieldMapping/inputMapping as mapping references and tabPosition as a position reference. Rejected, and pinned in reference-sites.derivation.test.ts so it cannot return by accident.

Fixture triage — the list and the fixtures were confirming each other

Four tests failed, every one on a fixture that pinned a phantom path. Re-spelled to the schemas, with the reason recorded in each:

referenceTo was dropped rather than tolerated: a consumer that accepts both spellings is how the wrong one survives.

What is still not computable, and where it now lives

The ruling requires that "no references" stop meaning "not computable". After derivation, an empty answer is a derived statement for 26 of 27 declared types. The residual honest gap moved off the response and into the build, so no wire change was needed:

  • unwalkableSourceTypes is pinned to exactly ['external_catalog'] (resolves no schema, ADR-0062/0088). If that set grows, a source type stopped being readable and every panel silently got shorter — now a red test.
  • SEMANTIC_REFERENCE_SITES holds exactly one entry: FieldSchema.reference, which names an object in prose ("Target object name (snake_case) for lookup/master_detail fields") and nowhere machine-readable. It is carried only because dropping it regresses the highest-value edge in the graph, and pinned at one. Other measured members of the class — AppSchema.homePageId, AppSchema.defaultAgent — are deliberately excluded so the incompleteness stays visible rather than looking handled.

⚠️Decision for the maintainer, not taken here. The durable close for that class is a producer-side annotation, e.g. .meta({ xRefTo: 'object' }) on the property — the channel flow-node-expression-paths.ts already uses for xExpression, and the "declared = enforced" side of ADR-0049. That is a packages/spec / *.zod.ts change and sits behind this card's tier fence, so it is flagged, not written.

Tier fence

Not tripped. No packages/spec edit, no *.zod.ts, no error-code ledger row, no new refusal, no response-shape change — response shape, status codes and error envelope are byte-identical. path and kind now describe where the reference was found rather than which table row matched.

Serial

Changed ranges in protocol.ts from git diff --unified=0: line 80 (import), 2955-3069 (registry ⇒ walker), 17843-17997 (the method). No overlap with PR #9173 (11398-11496 / 11650-11666) or #9196's JSDoc region.

Verification — all at 4e5989b9a, the final commit

pnpm --filter @objectstack/metadata-protocol test Test Files 118 passed (118)
pnpm --filter @objectstack/objectql test Test Files 213 passed (213)
pnpm --filter @objectstack/rest test Test Files 122 passed (122)

Gate families derived from the actual changed paths via scripts/pm/dispatch-gates.mjs, all PASS: check:cross-package-test-inputs, check:durability-log-level, check:filter-alias-parity, check:objectui-changeset, check:query-options-erasure, check:engine-double-contract, check:where-matcher, check:type-check-coverage, check-empty-changeset, check-changeset-no-major, check-adr-0087-registration, check-nul-bytes.

Ratchet family at the final head, after the full closure build:

check-type-check-coverage --re-measure: OK — 33 ledger entr(ies) re-measured in 301.1s,
1926 raw tsc error(s) total, none above its recorded number.
surplus: none — every entry sits exactly at its measurement.

Reverse verification — direction predicted in writing first

Predicted: ablating the shape half of the rule (accept any name-spelled property regardless of its value schema) goes red on exactly the anti-guesser pins and nothing else. Observed, exactly that:

× an ENUM-constrained value that shares a type name is NOT a reference
AssertionError: expected true to be false
Test Files 1 failed | 117 passed (118)

chartConfig.xAxis.position ('left' | 'right') and flow.nodes[].position ({x, y}) were both admitted as position references. Fix restored and proved byte-identical to the committed state (diff against git show HEAD:..., empty).


Generated by Claude Code

…mas (#9190)
`findReferencesToMeta` — the admin "Used by" panel behind
`GET /api/v1/meta/:type/:name/references` — was driven by a hand-curated
table of 7 target types and 40 dotted paths. Measured against the schemas
it claimed to describe, 34 of the 40 named properties no metadata type
declares, leaving 5 of its 7 target keys answering `{ references: [] }`
unconditionally while appearing covered. The panel's empty state reads
"Nothing in the metadata graph points at this item. Safe to delete."
Coverage is now derived at boot from `DEFAULT_METADATA_TYPE_REGISTRY` and
each type's schema, in the shape #7894 used for the URL-spelling map, so a
newly declared type arrives covered. The unit of derivation is a PROPERTY,
not a path: recursive containers (app navigation) make an exhaustive path
list unbounded, and the walk reports where the name was actually found.
No wire change: response shape, status codes and error envelope untouched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NTKPDRoynY8i3HmdSFUxFj
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

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

  • content/docs/api/client-sdk.mdx(via getReferences (sdk), meta.getReferences (sdk))
  • content/docs/concepts/metadata-lifecycle.mdx(via ObjectStackProtocolImplementation (symbol))
  • content/docs/getting-started/build-with-claude-code.mdx(via viewName (literal))
  • content/docs/ui/apps.mdx(via viewName (literal))

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

  • content/docs/releases/v12.mdx(via viewName (literal))
  • content/docs/releases/v16.mdx(via ObjectStackProtocolImplementation (symbol))
  • content/docs/releases/v17.mdx(via ObjectStackProtocolImplementation (symbol))

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
  • 1 anchor(s) matched too much of the corpus to be a work list: objectName (literal, 29 pages)
  • 10 name(s) were too generic to anchor anything (single lowercase words)

Coarse fallback — 5 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 origin/mainpackageMentionDocs.

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 origin/main → pass the list as args.docs.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 17, 2026
@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

PM review — the card was filed cautiously and the defect was far worse than filed

Not flipping: CI started 13:50Z. Holding until each job's own conclusion is green. ⚠️ Note the queue now runs the full suite as a required check (maintainer ruled #9283 Option A at ~11:59Z), so expect groups to take materially longer than earlier today — that is the intended cost, not a stall.

⭐ The headline: the registry was not incomplete, it was largely fictional

#9190 was filed with a careful hedge — "the behaviour is documented in the method's own TSDoc, so this may well be triaged as working-as-declared." Measured: 34 of the 40 curated paths do not exist against the schemas they claim to describe. Only 6 were live, reaching 2 target types. Five of the seven advertised keys answered {references: []} unconditionally — indistinguishable from a key that was never there, which is precisely the "a miss and a fault are different facts" collapse (ADR-0110 D3) the card was about, except worse: the miss was dressed as coverage.

And the harm is not abstract. You quoted the consuming string verbatim:

'engine.edit.refsEmptyDesc': 'Nothing in the metadata graph points at this item. Safe to delete.'

A lookup that could not answer was rendering "Safe to delete." That is the sentence that makes this a defect rather than a documentation gap, and finding it in objectui's i18n rather than assuming it was the right instinct.

⭐ The insight that makes the old design unfixable, not just unfixed

The unit is a PROPERTY, not a path.AppSchema.navigation is self-recursive, so an exhaustive path list is unbounded.

That is the load-bearing observation and it retroactively justifies the ⛔ in my brief. Adding keys by hand was never merely tedious — for a self-recursive schema it cannot be completed even in principle, so a view named from a third-level nav group was invisible even where the curated path was spelled correctly. Any close that stayed inside the path grammar would have shipped a new incompleteness with a fresh coat of paint.

The fixtures were confirming the fiction — that is the ninth instance

Four tests failed, every one pinning a phantom path. ⭐ "the list and the fixtures were confirming each other." A hand-written list validated by fixtures written from the same hand is a closed loop that cannot detect its own error, and it is why this survived so long with tests passing.

That makes nine instances this shift of a derived or hand-curated artefact consumed as authoritative and wrong: #9171 (twice — a missing path→gate edge, and a gate no path can express), #9182, #9192, the Part-of gate blind to Fixes, #8990's census (12 measured where there were 57), #9282, #9283, my own hand-written ledger control on #6367, and this — the most severe, because here the test suite was part of the loop.

What the review credits beyond that

  • referenceTo dropped rather than tolerated, with the reason stated: "a consumer that accepts both spellings is how the wrong one survives." Correct, and the harder choice.
  • The rejected rule is pinned, not just rejected. A suffix rule measures ~15% signal — it reads displayField / nameField / stageField and ~30 siblings as references to the fieldmetadata type when each names a field inside an object. Pinning it in reference-sites.derivation.test.ts means the next author cannot re-derive it as a "small improvement".
  • The honest gap moved into the BUILD instead of onto the wire.unwalkableSourceTypes pinned to exactly ['external_catalog']; SEMANTIC_REFERENCE_SITES pinned at exactly one. And the part I most want on the record: AppSchema.homePageId and defaultAgent are deliberately excluded so the incompleteness stays visible rather than looking handled. That is the opposite of the instinct that created this card.
  • Reverse verification predicted in writing first, ablating the shape half → red on exactly the anti-guesser pins and nothing else (chartConfig.xAxis.position, flow.nodes[].position both wrongly admitted). One failing file, 117 passing — a discriminating result, not a blanket one.
  • The Three read-side /meta verbs (history, audit, references) never reach canonicalizeMetaRequestType — they still derive their key from the MANIFEST map #9157 residue pin moved deliberately and got stronger: it asserted translation answers [] and called that a later card's problem. This is that card, DocSchema.translations is a real site, and the pin now proves the same fold on a non-trivial result.
  • Part of, not Fixes — correct. The residual needs a packages/spec annotation your tier fence keeps out, and you flagged it rather than writing it.

The one thing now owed to the maintainer

The durable close for SEMANTIC_REFERENCE_SITES — a producer-side .meta({ xRefTo: 'object' }) annotation, the channel flow-node-expression-paths.ts already uses for xExpression, and the "declared = enforced" side of ADR-0049. ⛔ Correctly not written: it is packages/spec / *.zod.ts, i.e. clause-② territory and a claude-fable-5 floor, not this dispatch's tier. I will carry it to the maintainer as a decision rather than let it sit inside a PR body.

Holding for CI.


Generated by Claude Code

@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

PM rulings on the two open questions, and the two findings are filed

Q2 — the wire-shape clause: A. Ruled, and your reading is the intended one.

My brief said an acceptable partial "must distinguish 'no references' from 'not computable'." You satisfied that without a wire change, by moving the discriminator off the response and into the build: after derivation an empty answer is a derived statement for 26 of 27 declared types, and the one genuinely-uncomputable type is named in unwalkableSourceTypes and pinned by a test so the set cannot grow silently.

That is the clause's intent met exactly. ⭐ The clause was never about a response field — it was about an empty array having stopped meaning two things, and it has. Doing it inside the fence rather than by widening the fence is the better answer, not a lesser one.

Not routing B. You measured that the panel renders From type / From name / Path and never reads kind, so a response-level coverage discriminator has no consumer today. Adding a wire field with no reader is how a surface accretes. If a client later needs to render the distinction, that is a card at that point, with the client as its evidence.

Q1 — the semantic-reference class: not mine. Carried to the maintainer, unchanged.

⛔ I am not ruling this and I am not sizing it. It needs packages/spec / *.zod.ts edits — clause ② and a claude-fable-5 floor — so it is outside both your tier and mine to decide.

What I am doing is making sure it does not evaporate inside a PR body: I am carrying it to the maintainer as a decision, with your recommendation intact (A — a producer-side .meta({ xRefTo: 'object' }) annotation, the channel flow-node-expression-paths.ts already uses for xExpression, and the declared-equals-enforced side of ADR-0049), including your own honest caveat that it "wants its own card with a spec-seat tier judgement rather than riding here."

⭐ And the reason your interim handling is right regardless of which way it is ruled: you carried exactly one bridge entry, pinned it at one, and deliberately excludedAppSchema.homePageId and defaultAgent even though you had measured them — so the incompleteness stays visible rather than looking handled. A hand-maintained map that grows quietly to three is how this card's original defect was born. Keeping it at one, visibly, is what makes the decision still real for whoever takes it.

Your two unfiled findings — filed, with the dedup search you were blocked from running

You were right to report rather than file blind or drop them when the API rate limit blocked the duplicate search. Both are now filed:

Two smaller things

Holding the flip for CI, which now runs the full suite as a required queue check (maintainer ruled #9283 Option A at ~11:59Z) — so expect it to take materially longer than earlier today.


Generated by Claude Code

@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

docs-drift advisory — checked, no doc change belongs in this PR

content/docs/api/client-sdk.mdx (getReferences / meta.getReferences) — outcome 1: it documents only the call.

The entire mention is one line in a code sample:

constrefs=awaitclient.meta.getReferences('object','account');

No sample response, no field table, no prose describing path or kind. Verified it is the only place in the docs tree that mentions the method at all (git grep -l getReferences -- content/ returns exactly this file), and that nothing anywhere under content/docs/** describes the path / kind values of a reference row. So the semantic shift this PR makes — path / kind now describing where the reference was found rather than which table row matched — is documented nowhere and cannot have gone stale.

The other three: dismissals confirmed, and one is stronger than "noise".

  • metadata-lifecycle.mdx — checked; it carries no reference / dependency / "used by" content at all. The ObjectStackProtocolImplementation symbol anchor is the whole class, exactly as predicted.
  • build-with-claude-code.mdx — the literal viewName does not appear in the file in the current tree. The advisory row is anchored on something no longer there; nothing to do here, and worth knowing the anchor itself is stale.
  • ui/apps.mdx — ⭐ not merely clean, corroborating. It teaches app navigation entries as { id, type, label, objectName, icon, viewName } under navigation, which is precisely the shape this PR's derivation now covers (app.objectName, app.viewName are derived sites). It is independent, docs-side evidence for the core measurement: the curated table's app.navItems[] / app.tabs[] were never the real spelling, and navigation[] always was. The page was right while the code's reference table was wrong.

viewName was removed as a curated path on page, never as a schema property — it remains real on app navigation, which is why a literal-string anchor cannot separate the two cases.

⛔ The three content/docs/releases/** rows were not opened and not touched.

No commit results from this round, so the head stays 4e5989b9a and the gate/ratchet evidence in the PR body still describes the tree at HEAD.


Generated by Claude Code

@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

Dogfood Regression Gate went red — diagnosed as infrastructure, re-run queued, and here is why that is not a shrug

⚠️The aggregate job failed while all three of its shards passed. That inversion is worth naming: the usual discipline here is never read the roll-up, read each job's own conclusion — this is the same rule running the other way, where the roll-up is red and every shard underneath it is green.

The diagnosis

Job 95410591524, run 32036890755. It died in "Prepare all required actions" — before checkout, before install, before a single test body ran:

Failed to download action 'actions/download-artifact@v8' …
Error: Response status code does not indicate success: 429 (Too Many Requests)
Back off 23.891 seconds before retry.
Failed to download action … 429 (Too Many Requests)
Back off 11.654 seconds before retry.
##[error]Response status code does not indicate success: 429 (Too Many Requests).
##[error]Failed to download archive … after 3 attempts.

The runner could not fetch the action itself. Nothing in this diff was ever executed by that job.

Why a re-run is the correct fix here, and only here

The standing rule is that "flaky" is not a diagnosis — a re-run is legitimate only when the job died before any test body ran (checkout, dependency install, lost runner); anything else gets root-caused. This died earlier than that: at action download, three attempts, 429 each time. There is no test result to interpret because no test was reached.

⭐ The distinction that matters: the three shards (1/3), (2/3), (3/3) all completed success — 13:57:48, 13:59:13, 13:59:40 — so the work this gate exists to do was done and passed. Only the job that collects them failed, and it failed at the package manager rather than at the assertion. Re-running restores the collection; it does not paper over a verdict.

Corroborating context

This is consistent with a degraded GitHub rather than anything local: the issues API returned 504 twice while I was reading #9283 this hour; a pull_request.closed webhook for #9284 arrived 89 minutes late; and the os-dev agent on this very card was blocked from filing two findings by "API rate limit already exceeded". Same window, same symptom class.

If the re-run reds again at a real assertion, this note does not cover it — that would be a genuine failure and gets root-caused, not re-run a second time.

Still holding the flip until the re-run is green.


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review August 17, 2026 14:20
@os-zhuang
os-zhuang added this pull request to the merge queueAug 17, 2026
Merged via the queue into main with commit a4c11adAug 17, 2026
39 of 40 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-9190-reference-paths-derivation branch August 17, 2026 14:38
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/xlteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@os-zhuang@claude