fix(objectql): buildSummaryIndex reports the skip when a roll-up's reference carrier is unreadable - #19293
Conversation
…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>
…dex skip Claude-Session: https://claude.ai/code/session_01NcPSwnmJHczmTu6FG7NMjE Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 1 package(s): 21 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 6 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 17 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 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
|
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;
buildSummaryIndexisprivateand 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.tstook a lander after the card's reading ref221dabb72—a675ad4e(#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.
buildSummaryIndexis atengine.ts:9009; the comparison the card quotes at:9033is byte-identical and now sits at the same line, and the silentcontinueat:9039is unchanged. #19080 routedplanCascadeAtomicityandcascadeDeleteRelationsthrough 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/lookupfields for one whosereferencenames the parent. That comparison read the carrier raw, so a carrier no reader can read — a non-string, whereFieldSchema.referencedeclares an optional string — comparedfalseagainst every name,fkFieldstayed unset, anddropped a declared
summaryfield 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:
referenceCarrierOf— the accessor Refuse an unreadablereferencecarrier at the ten residual readers (ruling E item 2 residue) #19080 routed the two cascade seams through;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 withsummaryOperations.relationshipField);undefined,nulland''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:
{ object: 'bad' }on amaster_detailreference: 'bad'ObjectSchema.safeParse(the contract door)fields.bad.reference: invalid_typegetMetadataTypeSchema('object')— whatsaveMetaItemresolves for a stored/metawriteregistry.registerObject— the choke point every metadata door funnels through{"object":"bad"};referenceCarrierOfon the registered field throwsSo: 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 rawregisterObject, 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 storedsummaryfield 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-referencefires ontypeof def.reference !== 'string'). That is a one-shot, console-carried note about the child field at registration; it does not name the parent's declaredsummaryfield, does not say the roll-up was dropped, and this package's own vitest config quiets[Registry]output towarn. 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:referencestill resolvesfkField(inv_line/inv), and the recorder stays at zero;errorand notwarn, naming the field, the consequence and both fixes;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:pnpm --filter @objectstack/objectql testpnpm --filter @objectstack/objectql typecheckcheck:test-typecheckholds at 40 files / 234 errors / 65 signatures, unmovedpnpm --filter '@objectstack/objectql^...' buildpnpm exec turbo run build --filter='./packages/*' --filter='./packages/*/*'node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commandsthen--ranpnpm lint(eslint . --no-inline-config, whole repo — no narrowing to declare)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-ratioandcheck-plugin-teardown-shape --self-testrefused on a shallow clone (deepened withgit fetch --shallow-since=2026-06-15, both then exit 0), andcheck:dual-build-cjs-loads,check:lean-entry-closureandcheck:type-check-debtrefused for want of built output (built, then exit 0).Acceptance notes
Out-of-scope observations, noted and deliberately not filed:
!fkFieldskip 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, atseverity: 'error'), and widening the new diagnostic to cover it would make every legitimately-unresolvablesummarydeclaration log per index build. Successor: whoever next reopens PR Retire check-reference-carrier-shape; refuse an unreadablereferencecarrier at the reader #18503's C2 boundary for this function — the decision is now recorded inbuildSummaryIndex's docblock, where they will meet it.Sibling card #19081 shares the same root (an unreadable
referencecarrier) 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