Skip to content

fix(objectql): buildSummaryIndex reports the skip when a roll-up's reference carrier is unreadable - #19293

Merged
huangyiirene merged 3 commits into
mainfrom
claude/issue-19082-summary-index-loud-skip
Sep 20, 2026
Merged

huangyiirene merged 3 commits into
mainfrom
claude/issue-19082-summary-index-loud-skip

Conversation

@huangyiirene

Copy link
Copy Markdown
Collaborator

Fixes #19082

Clause-②: no

A diagnostic added to an internal, private index neither loosens an accept set nor widens a published surface. No schema changed; buildSummaryIndex is private and nothing about its signature, its return shape or its resolution rule moved.

The premise, re-taken by symbol

Triage said it had not re-taken the reading and asked the executor to. packages/objectql/src/engine.ts took a lander after the card's reading ref 221dabb72a675ad4e (#19080, the ten-residual-readers round) — so the site was re-located by symbol, never by the card's line numbers.

It still resolves by carrier equality. buildSummaryIndex is at engine.ts:9009; the comparison the card quotes at :9033 is byte-identical and now sits at the same line, and the silent continue at :9039 is unchanged. #19080 routed planCascadeAtomicity and cascadeDeleteRelations through the arbiter and left this third site alone. premise_still_valid: true.

The defect

The child-to-parent foreign key is resolved by scanning the child object's master_detail / lookup fields for one whose reference names the parent. That comparison read the carrier raw, so a carrier no reader can read — a non-string, where FieldSchema.reference declares an optional string — compared false against every name, fkField stayed unset, and

if (!fkField) continue; // can't resolve the relationship — skip

dropped a declared summary field out of both indexes. recomputeSummaries() then had nothing to do after every insert / update / delete of the child, so the parent's stored summary value kept whatever it held while each of those writes reported success, and nothing anywhere said so. It is the second way this one function invents "nothing to recompute"; the first, its registry read, was closed as #9154.

The boundary this card asked to reopen — and where it now stands

PR #18503 recorded this site in its C2 list and the #18550 round left it there deliberately. That boundary stands: the resolution rule is untouched. Loosening the comparison would trade a silent stall for a mis-matched foreign key, which is more expensive — a roll-up quietly aggregating the wrong children reads exactly like a correct one, while a roll-up that stopped moving is at least visible to anyone who looks at the value. What ends here is only the silence, which triage named as the half available today:

  • the carrier is read through the one arbiter, referenceCarrierOf — the accessor Refuse an unreadable reference carrier at the ten residual readers (ruling E item 2 residue) #19080 routed the two cascade seams through;
  • its refusal is caught rather than propagated, because this is a scan looking for the FK across every relation field: a propagating refusal on one unreadable field would hide a readable sibling that really is the foreign key, turning a roll-up that works today into a hard failure of every write to that child. Pinned (§5 of the new test);
  • the skip reports itself at error, once per index build. A persisted summary that silently stops tracking its children while every write keeps reporting success is the durability class by AGENTS.md's own question, and the line carries both halves it owes: the consequence (which field will not recompute, and that the system keeps looking healthy) and the fix (spell the carrier as the target object's name, or name the FK with summaryOperations.relationshipField);
  • absence is untouched. undefined, null and '' mean "this field names no target", which is legal; they skip silently exactly as before. Every readable carrier resolves exactly as before.

The decision and its reasoning are recorded on the card and in the function's own docblock, so the next reader of the skip branch finds them instead of re-filing.

Reachability — measured, and deliberately not inflated

The card recorded this as not established, and it is now measured on this tree rather than argued. One probe, three doors, each with a readable-carrier control that passes:

door shape { object: 'bad' } on a master_detail control reference: 'bad'
ObjectSchema.safeParse (the contract door) REFUSEDfields.bad.reference: invalid_type accepted
getMetadataTypeSchema('object') — what saveMetaItem resolves for a stored /meta write REFUSED accepted
registry.registerObject — the choke point every metadata door funnels through ACCEPTED, carrier stored verbatim as {"object":"bad"}; referenceCarrierOf on the registered field throws accepted

So: not a live outage — the live authoring and stored-write doors refuse this shape today — and not unreachable either. The registry takes it raw, which is the population engine.ts's own #9689 note already names for the sibling seam: "a raw registerObject, or a stored/artifact row written before the tightening — the two populations parse-time rejection measurably cannot catch, since the engine registers raw objects and never re-parses". Graded exactly there, and ⛔ not escalated: no stored summary field was measured to have never recomputed, which is this card's only escalation condition.

One honest qualifier, measured in the same probe: registration does already emit an ADR-0078 completeness warning for this field (field/relationship-without-reference fires on typeof def.reference !== 'string'). That is a one-shot, console-carried note about the child field at registration; it does not name the parent's declared summary field, does not say the roll-up was dropped, and this package's own vitest config quiets [Registry] output to warn. It is a neighbouring signal, not this one.

Tests

packages/objectql/src/engine-summary-index-unreadable-carrier.test.ts, 7 cases, both directions — because without the second, a change that simply stopped resolving anything would be indistinguishable from a fix:

  • §1 control — a normal reference still resolves fkField (inv_line / inv), and the recorder stays at zero;
  • §2 the defect — an unreadable carrier emits the skip signal, at error and not warn, naming the field, the consequence and both fixes;
  • §3 — both in one index build: the readable roll-up is indexed while the unreadable one is reported;
  • §4 — absence stays silent;
  • §5 — an unreadable sibling declared before the real FK does not hide it;
  • §6 — said once per index build: five consults report once, and a registry mutation makes it report again (without that second leg a "1" could equally mean "once per process").

The zeros in §1, §4 and §5 are readings rather than a dead instrument: §2 drives the same recorder through the same handle and measures it at 1.

Every command below captured its exit code before any pipe, at HEAD 308a3403:

command result
pnpm --filter @objectstack/objectql test 301 files / 5016 tests passed
pnpm --filter @objectstack/objectql typecheck exit 0 — and check:test-typecheck holds at 40 files / 234 errors / 65 signatures, unmoved
pnpm --filter '@objectstack/objectql^...' build exit 0
pnpm exec turbo run build --filter='./packages/*' --filter='./packages/*/*' 72/72 successful
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands then --ran 62 derived, 62 run, 0 NOT-MEASURED, 0 UNRUN — every family carries a recorded exit code and none is 3
pnpm lint (eslint . --no-inline-config, whole repo — no narrowing to declare) exit 0

Five of the 62 first returned exit 2 or 3 — never a pass, nothing measured — and each was cleared rather than reported as one: check-engine-split-ratio and check-plugin-teardown-shape --self-test refused on a shallow clone (deepened with git fetch --shallow-since=2026-06-15, both then exit 0), and check:dual-build-cjs-loads, check:lean-entry-closure and check:type-check-debt refused for want of built output (built, then exit 0).

Acceptance notes

Out-of-scope observations, noted and deliberately not filed:

  • noted, not filed — the !fkField skip is still silent in its other branch: a roll-up whose child declares no relation field at all is dropped with no diagnostic here. It is not this card's input, it is loud at a different layer (the ADR-0078 completeness rule fires on exactly that shape at registration, at severity: 'error'), and widening the new diagnostic to cover it would make every legitimately-unresolvable summary declaration log per index build. Successor: whoever next reopens PR Retire check-reference-carrier-shape; refuse an unreadable reference carrier at the reader #18503's C2 boundary for this function — the decision is now recorded in buildSummaryIndex's docblock, where they will meet it.
  • noted, not filed — when an unreadable sibling carrier sits beside a readable FK that does resolve, the unreadable one is passed over silently (§5 pins that it does not break resolution). Nothing is dropped on that path, so there is no defect to report; the carrier itself is already reported by the ADR-0078 rule at registration. Successor: none — no PR or reader reaches this path with a question the ADR-0078 warning does not already answer.

Sibling card #19081 shares the same root (an unreadable reference carrier) and is deliberately not folded in: different file, different failure direction (it leaks the carrier onward; this one silently drops work), different lane. Triage ruled both should be taken, in either order.


Generated by Claude Code

…ence` carrier is unreadable

The child->parent FK was resolved by comparing the child field's `reference`
carrier to the parent's name. A carrier no reader can read -- a non-string,
where `FieldSchema.reference` declares an optional string -- compared false
against every name, `fkField` stayed unset, and the `continue` dropped a
DECLARED `summary` field out of both indexes with no diagnostic anywhere. The
parent's stored summary value then kept whatever it held through every insert /
update / delete of the child, while each write reported success.

The resolution RULE is deliberately unchanged (a looser comparison would trade a
silent stall for a mis-matched foreign key, which is more expensive). The carrier
is read through the one arbiter, `referenceCarrierOf`, and the skip now reports
itself at `error` with both the consequence and the fix. Absence
(undefined/null/'') stays silent, every readable carrier resolves exactly as
before, and the arbiter's refusal is caught rather than propagated so an
unreadable sibling cannot hide the readable field that IS the foreign key.

Claude-Session: https://claude.ai/code/session_01NcPSwnmJHczmTu6FG7NMjE
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 20, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

21 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json adf4b18777d507236cd24b7ed59b45a7c71bd1fd.

6 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: ObjectQL (symbol, 68 pages)
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 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.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 17 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 adf4b18777d507236cd24b7ed59b45a7c71bd1fdpackageMentionDocs.

Which tree this was computed on

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

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

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

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