Repository navigation
finding(objectql): the roll-up summary index's registry read swallows into an empty index — and CACHES the invented emptiness, so every parent roll-up silently stops recomputing #9154
Description
Activity
Triage (first-touch grading):
domain:engine-core(already routed at filing); promotedfinding→pm:queue, type Task.Scope: the uncontroversial half is mandatory — a failed registry read must not be CACHED as a measured-empty index (revision-stamped emptiness) and must not be silent. For the degrade-vs-propagate call, apply the #8895 family ruling (discriminate or propagate) with measurement; if the implementer concludes the write-path contract must move (fail writes when the index cannot be built), stop and report the fork rather than deciding it inside the PR.
Premise re-verified on
origin/main:engine.ts:6942still swallows into?? []+catch, andensureSummaryIndexesstill revision-stamps the result.Serial constraint for the dispatching seat: same file as #9002's cascade seams — confirm #9002's PR is merged and re-verify this seam on the merged ref before dispatch.
Size/model suggestion: M, opus.
Generated by Claude Code
Claim: PM loop — batch 5
Session:session_01NTKPDRoynY8i3HmdSFUxFj· Branchclaude/issue-9154-summary-index-cached-emptiness· Worktreeobjectstack-issue-9154
Domain:domain:engine-core· File surface:packages/objectql/src/engine.ts—buildSummaryIndex/ensureSummaryIndexesonly
Model:opus· ⛔ Clause-② check: NOT triggered — nopackages/spec/**, no ledger, no accept/reject face. This changes an internal read's failure behaviour only. Stated at claim time deliberately (#9211).
Serial:engine.tshas no in-flight claim. #9002's fix (PR #9163) landed as this card's sibling.裁决(不可重裁)
The read propagates. Same family as #8895 and #9002, same reasoning: an unreadable registry is never truthfully "no roll-ups", so there is no benign class to discriminate — remove the swallow.
⭐ But this one is worse than its two siblings and the fix must address the extra limb. #9002's seams invented an answer once; here
ensureSummaryIndexes()caches the invented emptiness under the currentobjectRevision. So the failure outlives the condition that caused it: every parent roll-up silently stops recomputing until an unrelated registry change happens to bump the revision. ⛔ Removing the swallow without addressing the cache leaves a shorter version of the same bug — a poisoned cache entry must not survive the read that poisoned it.PM 机制假设(须实测,鼓励证伪)
getAllObjects()still has no throwing path — finding(objectql): the delete-cascade path's two registry-read swallows are the #8895 shape one layer up —catch → returndisables every referential guard at once, silently #9002's dev measured that (Map walk,resolveObject()returnsundefinedon every modelled failure, fold is spreads and comparisons). Re-derive on today's tree; if it has grown one, this card is live, not dormant, and I want that before the fix.- ⭐ Measure the cache's actual lifetime. How long does a poisoned entry survive — until the next
objectRevisionbump, process restart, or something narrower? The answer decides whether this is "briefly wrong" or "wrong until an unrelated event". ⛔ Do not assume; the card asserts it and the assertion is the card's whole severity argument. ⚠️ Expectcheck:durability-log-levelto be blind to this. It classifies acatchby the expression it returns; finding(objectql): the delete-cascade path's two registry-read swallows are the #8895 shape one layer up —catch → returndisables every referential guard at once, silently #9002 measured its census as unchanged across an equivalent fix (the check-durability-degradation-log-level: the read-seam invention rule keys onreturn, so acatchthat degrades by FALLING THROUGH into an empty accumulator is structurally invisible #8845 blind spot). ⛔ Do not read its green as certification, and ⛔ do not "fix" the gate.⚠️ Removing a swallow may turn tests red by exposing vacuous ones. That happened twice this shift — PR fix(metadata-protocol): four read seams that failed no longer answer from an empty accumulator #9067 (stub engines missingfindOne) and PR fix(objectql): the delete-cascade path's two registry reads propagate instead of inventing 'no relations' (#9002) #9163 (a registry double missinggetAllObjects). A red test after a swallow removal is evidence the test was previously vacuous, not damage to repair. Report the count and what each red proves.
Gates
Derive with
scripts/pm/dispatch-gates.mjsafter the final commit and read CI's own job list — the derivation missed five gates on a sibling PR this round (#9171, #9192). Predict the reverse-verification direction in writing first, and report it as observed even if the shape is wrong.Fixes #9154.
Generated by Claude Code
{ "issue": 9154, "status": "done", "branch": "claude/issue-9154-summary-index-swallow", "pr": "https://github.com/objectstack-ai/objectstack/pull/9284", "premise_still_valid": true, "summary": "buildSummaryIndex's registry read now propagates (both halves of the swallow removed: the catch AND the optional call ?.(), matching the canonical shape PR #9163 landed for the cascade seams), and ensureSummaryIndexes can no longer cache an invented emptiness -- the build completes into a local before anything is published to the instance, the revision stamp is written last, and a throw clears any cached index and resets the stamp before rethrowing unchanged. Removing the swallow turned 83 tests red across 9 suites whose vi.mock('./registry') doubles never modelled getAllObjects; all nine were repaired (declaring getAllObjects: () => [], truthful there since none declares a summary field), not suppressed. NOTE ON BRANCH NAME: the dispatch prompt named claude/issue-9154-summary-index-swallow and the claim comment named claude/issue-9154-summary-index-cached-emptiness; I followed the prompt and am surfacing the conflict rather than picking silently.", "tests": "All at HEAD 65a5e2afa (the final commit; every gate below ran after it). NEW PIN: packages/objectql/src/engine-summary-index-registry-read-failure.test.ts, 8 tests, green. PACKAGE: pnpm --filter @objectstack/objectql test -> 'Test Files 214 passed (214) / Tests 3763 passed (3763)'; pristine origin/main baseline measured for comparison -> 213 files / 3755 tests. pnpm --filter @objectstack/objectql typecheck -> clean. DOWNSTREAM SWEEP (prefix filter '...@objectstack/objectql', i.e. packages that DEPEND ON objectql): metadata-protocol 116 files/1606 tests, rest 122 files/1896 tests, runtime 165 files/2464 tests, all green -- the first rest/runtime runs showed suite-LOAD failures naming unbuilt sibling packages with ZERO test failures, cleared by building each package's own dependency closure, so an artefact of a scoped build and not of this change. REVERSE VERIFICATION (direction predicted in writing first: controls green, seam+cache red; observed exactly that): fix reverted, pins kept -> 6 failed / 2 passed, the 2 green being the positive controls. GATES (derived with scripts/pm/dispatch-gates.mjs on the real changed paths, all green): check:changeset-gate-self-tests, check:durability-log-level, check:objectui-changeset, check:stack-collection-maps, check:query-options-erasure, check:engine-double-contract, check:where-matcher, check:nul-bytes, check:type-check-coverage, check:cross-package-test-inputs, check-engine-split-ratio. check:type-check-debt --re-measure REFUSES without a fully built workspace (that throw means NOT MEASURED), so the ledger delta was measured directly with objectql's closure built and its tests in the program: 355 before, 355 after -- the gate's exact recorded number for @objectstack/objectql, unmoved. No ablation in this card. CI on the draft PR is in_progress; not waited on, per the report-at-draft-PR-time rule.", "open_questions": [], "out_of_scope_findings": [ "filed as #9285: three more `getAllObjects?.() ?? []` seams in packages/objectql/src/plugin.ts invent an empty registry -- syncRegisteredSchemas() then early-returns, so an unreadable registry silently skips schema sync for every object at boot; reconcileFederatedBindings() and the action-governance inventory go silent the same way. Labelled `finding` + `domain:engine-core`, unassigned, dormancy argument re-derived." ] }PM assumptions — all four measured, all four HOLD
-
getAllObjects()still has no throwing path — HOLDS. Re-derived on today's tree: the walk overobjectContributorscallsresolveObject(), which returnsundefinedon every failure branch it models (no contributors; no owner, the orphan-overlay case after aconsole.warn); below itfoldExtendersOntoDefinitiontomergeObjectDefinitionsis spreads and conditionals, andscalarOverridesPackagedBaseis three comparisons. No I/O, no driver, nothrow. Dormant, so this is a structural close — the card is not live. -
⭐ The poisoned cache's lifetime — HOLDS, and is now measured rather than argued.
objectRevisionmoves on exactly seven sites, every one a metadata mutation (registerObject,removeObjectOverlay,unregisterObjectsByPackage,unregisterObject,invalidate,invalidateAll,reset); none is on a data write. A probe against the pre-fix engine — poison the first index build, heal the registry, then perform four ordinary writes with no registry mutation at all:pre-fix seamAnswer: [] rev 2 -> 2 parentRow: {"name":"INV-1","id":"r_1"} children: 3, sum 47 post-fix seamAnswer: THREW boom rev 2 -> 2 parentRow: {"name":"INV-1","line_total":47,"line_count":3,"id":"r_1"}Pre-fix the parent row carries no roll-up keys at all — not even the insert-time seed of 0 — while three children summing 47 sit in the store and every write returned success. The registry was healthy for all four of those writes; only the cached emptiness kept it wrong. The card's severity claim is right: not "briefly wrong", but wrong until a metadata mutation or a process restart.
-
check:durability-log-levelis blind to this — HOLDS. Census run on both trees, identical: 26 durability-critical catch seams, 66 read seams. Its green certifies nothing here and the gate was not touched. Worth recording: the newcatch/rethrow inensureSummaryIndexesdid not enter the census either. -
A swallow removal turns vacuous tests red — HOLDS, nine files worth. 83 tests across 9 suites failed with the identical
TypeError: this._registry.getAllObjects is not a function. The prior green was vacuous in a specific, measurable way: the write path in all nine had been silently skipping the insert-time roll-up seed (Field.summary 的 count 汇总：从未有过子记录的父行停在 NULL，删光子记录才变 0 —— 同一个「零」两种值，筛选 = 0 静默漏行 #5749) and the post-write recompute, and no assertion could see it because an incomplete double was indistinguishable from an empty registry.
One thing worth knowing for the next card in this family
The index is built lazily, so a failing read only poisons anything if it lands on a real build. Arming the failure after a successful write proves nothing — measured: an
insertover a warm index makes zerogetAllObjects()calls, and the seam is never re-entered. The pins therefore poison the FIRST build, which is the shape a booting deployment actually has. My first draft of the tests armed the failure after a warm-up insert and produced three false greens; the test file records this so the next author does not repeat it.
Generated by Claude Code
-
- added 3 commits that reference this issue
on Aug 23, 2026 - added a commit that references this issue
on Sep 28, 2026
Found while implementing #9002 (the delete-cascade path's two registry-read swallows). Filed unassigned, not a claim. #9002's PR deliberately does not touch this — its declared file surface was the two cascade seams, and the ruling that governs them was scoped to the delete-cascade path.
Measured on
origin/main@3851f87f0, located by symbol (line numbers inengine.tsmove constantly).The seam
packages/objectql/src/engine.ts,ObjectQL.buildSummaryIndex(), first statement:buildSummaryIndex()walks every registered object fortype: 'summary'fields and builds the two roll-up indexes (byChild,byParent). Those indexes are whatrecomputeSummaries()consults after every insert / update / delete to decide which parent roll-ups a child write must recompute.An empty
objectstherefore means no roll-up is ever recomputed — every parent summary field silently keeps a stale value, with nothing logged and every write reporting success. Same shape as #9002's two seams and #8895's probe: a read that could not run is answered with an invented "there is nothing here", where "nothing" and "unreadable" have opposite consequences.The extra property this one has: the invented emptiness is CACHED
ensureSummaryIndexes()rebuilds only when the registry'sobjectRevisionhas moved since the index was built:So a single failed read does not degrade one write — it installs an empty index and stamps it with the current revision, and every subsequent write reads that cached emptiness until some unrelated registry change bumps the revision. The
?? []on the optional call has the same effect for a registry that does not implement the method at all.That is the same failure the
ensureSummaryIndexesdocblock already records as a shipped incident from the other direction (an index built before a published object existed, so a roll-up shipped empty over correct metadata — cloud#970). Thecatchre-opens that outcome through a different door.Why this is filed as an observation, not a defect card
Same dormancy argument #9002 was graded on, re-derived here:
SchemaRegistry.getAllObjects()is a walk over in-memoryMaps callingresolveObject(), which returnsundefinedon every failure branch it models and never throws; the fold below it (foldExtendersOntoDefinitiontomergeObjectDefinitions,scalarOverridesPackagedBase) is spreads and comparisons. No I/O, no driver, nothrowon the measured path — so there is no measured way to make thiscatchfire from a running deployment today.One caveat worth recording, because #9002 measured it: the
catchalso absorbs structural failures, not just thrown ones. A registry double that omitsgetAllObjectsreaches this seam as aTypeError— and that is not hypothetical,engine-middleware-operation-vocabulary.test.tsships exactly such a double, and its omission was invisible until #9002 removed the two cascade swallows (three tests went red at once). Here the?.()swallows the same omission without even needing thecatch.Suggested disposition (triage's call, not mine)
Not obviously the same answer as #9002's. There the ruling was propagate, because an unreadable registry is never truthfully "no relations" and the seam guards referential integrity, so failing the write is the truthful outcome. A roll-up index is a derived aggregate, not an integrity guard, and "fail every write because the summary index could not be built" is a genuine judgement call rather than a mechanical consequence — which is why #9002 left it alone rather than riding it in. The cheaper half is uncontroversial regardless of that call: the invented emptiness must not be cached as if it were a measured answer, and it must not be silent.
Related: #9002 (the two cascade seams, same file, same class), #8895 (the dependents-probe fix that ruled the family discriminate or propagate), #8845 (the gate blind spot that keeps this shape invisible to
check:durability-log-level).Deliberately not included:
announceMigrationGates()'s walk overgetAllObjects()in the same file also sits inside atry/catch, and that one is judged benign here rather than left unexamined — it emits a one-shot advisory log atkernel:bootstrappedand writes nothing, so a swallowed failure costs an advisory and no data.