perf(sync): memoize the already-materialized digest check (#2079) - #2112
Conversation
Deciding whether a graph-scoped KA is already materialized ran a COUNT,
a full CONSTRUCT of the assertion graph, and a SHA-256 over the result -
per descriptor, per pass, INSIDE withKaWriteLock, so it blocked live
gossip for that KA. Amplified by 4 concurrent peers x 4 passes.
Adds a node-local witness recording that THIS node read the graph back
and matched a specific digest. A warm check becomes a bound-subject ASK.
Measured per-KA (50/200/1000/5000 quads): CONSTRUCT+digest is
0.91/2.69/15.98/197.81ms; the ASK is ~0.03-0.06ms. 46/55/74/90% less
local work. Default backend is oxigraph-worker, where CONSTRUCT results
cross a postMessage + structured clone, so those are lower bounds.
NOT what the issue asked for, deliberately. It proposed replacing the
whole predicate with a single ASK. The COUNT gate is KEPT, and that is
the load-bearing decision:
- Three paths remove an assertion graph outside this lock - the SWM
TTL sweep, VM promote/publish/update (a DIFFERENT lock map), and the
chain-reset wipe, whose scoped delete filters on the context-graph,
publisher and changelog prefixes only and therefore SPARES a
urn:dkg:local:* witness. ASK-only would certify a wiped store as
parity, permanently and silently.
- The count catches all three for free (count 0 != expected).
- Measured, also dropping the count buys a further 1.5-10.5%. Trading
self-healing for that is not a good trade.
The witness is written from the VERIFICATION branch, not the replace
path - only after this node computed the digest over its own store
content and matched it. So it can never record a peer's assertion, and
there is no crash window in which a witness exists for content that was
never verified.
Equal-count v1->v2 (the one case the count cannot catch) is handled by
the READ binding the digest, not by invalidation: a standing v1 row
cannot match an ASK for v2's digest. Invalidation in replaceGraph is
defence in depth, and the comment says so rather than overclaiming.
Write uses tryReplaceSubjectAtomically and SKIPS entirely when the
adapter cannot do it - never the usual delete-then-insert fallback. A
missing witness costs one recomputation; a split write could leave two
digest rows for one graph, which is a standing lie.
Honest scope: this does NOT make a repeat pass O(1). hasValidSnapshot
still does a whole .nq read, parse and a second digest per manifest ref
before onSnapshotReady fires. The pass stays O(total CG bytes).
Verified by mutation, disjoint rows, assertion deaths:
- remove the COUNT conjunct -> drop-then-recheck row dies
"expected true to be false" (a standing witness certifies an empty
graph - exactly the ASK-only failure).
- never write the witness -> warm-path row dies "expected 2 to be 1"
(a second CONSTRUCT reappears).
Added to the vitest include allow-list; a file not listed is silently
uncollected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012PC4iJT3UBkNC2iygHpVyd
otReviewAgent
left a comment
otReviewAgent
left a comment
There was a problem hiding this comment.
Operational Notice: Review Agent could not complete this review.
Business logic reviewer failed: WARNING: failed to clean up stale arg0 temp dirs: Permission denied (os error 13)
Jurij89
left a comment
Jurij89
left a comment
There was a problem hiding this comment.
Review at d5ecafaef
The shape is right and the write-up is unusually honest — the "honest scope" and "not done: hit-rate" sections are exactly what a perf PR should say. But I would not merge this yet. The module doc states a contract that the PR itself does not satisfy, and the one invariant the whole design rests on is pinned by nothing.
Everything below was checked against the pinned head; the false-hit work was executed against a real OxigraphStore, not reasoned about.
What holds — verified, not assumed
- The chain-reset claim is true.
chain-reset-wipe.ts:148-152filters onV10_GRAPH_PREFIX || PUBLISHER_GRAPH_PREFIX || CHANGELOG_GRAPH, sourn:dkg:local:*survives the scoped wipe exactly as described. The COUNT gate does catch all three named removal paths. - Graph-only subject keying does evict atomically, and
tryReplaceSubjectAtomicallydoes write nothing when unsupported. - Isolation is genuinely safe, and better than the PR argues. Serve-side exclusion is allow-list, not deny-list:
isCandidateGraph(graph-plan.ts:1408) gates both the durable lane (:1695) and the changelog delta lane (:1506), the SWM lanes synthesize graph URIs rather than enumerating (:2392-2399), peer bytes are filtered byparseAndFilterNQuadsbefore any insert,descriptor.assertionGraphis compared against a locally derived expectation and throws on mismatch (graph-scoped-swm-recovery.ts:127-141), and remote SPARQL cannot name a graph. No peer can read or write this graph. - The motivating framing is accurate — the check really is inside the lock (
shared-memory-sync.ts:647→:681).
Two things I would fix before merge
H1 — The module's own contract is violated by the one writer that satisfies its precondition
swm-materialization-witness.ts:140-143 says:
Call this from every path that replaces or removes the graph's content WITHIN a lock this module can see.
Live gossip does exactly that and does not call it:
workspace-handler.ts:1352computes the same graph —knowledgeAssetLayerGraphUri(cgId, SharedWorkingMemory, contentScope, subGraphName);:1363takesswmKaWriteLockKey(...)— the identical key, with a comment saying "so the public catch-up materializer serializes on the identical string";:1476tryReplaceGraphAtomically(this.store, swmGraph, normalized, …).
packages/publisher/src contains no witness reference at all. The PR's module doc enumerates three removal paths and correctly says the count catches them; it never enumerates the replace paths, which the count cannot catch.
To be fair about reachability — and this is the part the finding as first written got wrong — the ordinary path is safe. A successful gossip apply advances the head, so guard (a) (shared-memory-sync.ts:667-675) skips an older descriptor before isGraphAssetMaterialized is ever consulted. The false hit needs a torn apply: :1476 replaces the graph, then :1502 (snapshot file) and :1520 (head) can throw, and withWriteLocks is a lock, not a transaction. That leaves content=v2, head=v1, witness=D1. A peer re-offering v1 then passes guard (a) (v1 does not outrank v1), passes the count gate (equal count), and hits the witness — reported materialized while the store holds v2. Pre-PR the CONSTRUCT returned false there and replaceGraph repaired it.
So: narrow, but it converts a self-healing check into a sticky one, and content-ahead-of-head is a known crash artifact rather than a hypothetical.
Fix is one line — call invalidateSwmMaterializationWitness from the publisher's replace path, .catch(() => {}) like the existing site. That takes the residual to zero and makes the module doc true.
H2 — "Only the verifier writes it" is the whole design, and nothing tests it
That claim is why #2079's head-row proposal was killed, and it is the sole guard for one state. Delete the if (matches) at :206 — keep the body, so the write becomes unconditional — and every test in the repo still passes.
Every call site that can reach the mismatch branch asserts only the return value: swm-materialization-witness.test.ts:172, swm-snapshot-materializer.test.ts:159 and :326. Every other materializer test starts from an empty store and returns at the count gate without reaching the CONSTRUCT at all.
Under that mutation the failure is real and sticky: a mismatch writes a witness for descriptor.publicQuadsDigest — precisely the value the next round's ASK binds — and if replaceGraph then fails, which it does on a missing snapshot (graph-scoped-swm-recovery.ts:273 throws, caught at shared-memory-sync.ts:748-756, so the only invalidator never runs), the next round hits and returns true forever.
Fix is one row: seed v1, isGraphAssetMaterialized(descriptorFor(v2)) → false, then assert readSwmMaterializationWitness(store, GRAPH, v2digest) === false and call it a second time, still false. The second call is what kills the mutant.
Medium
-
The ordering comment asserts a false safety property.
:250-254says a crash between the replace and the invalidate "leaves a stale row that misses rather than no row at all, which is identical in effect". It misses for the new digest and hits for the old one. And because the invalidate is.catch(() => {}), a swallowed failure reaches that state with no crash at all. Relatedly, claim 3 is asymmetric: binding the digest covers witness(old) + descriptor(new); it does not cover witness(old) + content(new) + descriptor(old). The invalidate is not defence in depth for that direction — it is the only cover, and it is best-effort. -
Witness writes append changelog markers and advance
seq.ChangelogStore's reserved set is{CHANGELOG_GRAPH}plusoptions.reservedGraphs(changelog-store.ts:232) — andreservedGraphshas zero callers anywhere in the repo. So every cold-path witness write emits a marker. It does not reach peers (the delta lane filtersisCandidateGraphatgraph-plan.ts:1506), so this is local bloat and sequence churn rather than a protocol leak — but a memo billed as free is writing to the change log on every miss. -
A pure read became a write path, inside the lock this PR is trying to shorten. On every cold check
isGraphAssetMaterializednow performs an atomic subject replace, which also runsbumpMutation()/maintainTouchedGraphson the graph-set index — the structure behind #1549's fullSELECT DISTINCT ?gscans. Cold or churning stores now pay ASK + CONSTRUCT + digest + write where they paid COUNT + CONSTRUCT + digest. The PR admits hit rate is unmeasured; the break-even is the number that decides whether this is a win, and it is the one number missing. -
No GC.
invalidateSwmMaterializationWitnesshas exactly one caller. The TTL sweep, VM publish and the chain-reset wipe all destroy assertion graphs and orphan their rows permanently — the very survival the PR cites as the reason to keep the COUNT gate is also an unbounded leak of 2 quads per KA ever materialized, and it is not mentioned. Worth noting the chain-reset wipe was already extended once for exactly this reason (chain-reset-wipe.ts:143-148, the changelog graph); the witness has the same shape and was not added. -
On
sparql-httpwithatomicUpdates:falsethe memo can never pay and the code cannot learn it. The write returns false and stores nothing; the read still runs unconditionally on every check. Permanent added cost, zero possible benefit, no detection. -
The witness ASK is the only new store call with no
.catch. The write and the invalidate are both contained;:183is not. It shares thebackgroundlane with the existing two queries so it is not a new shedding class, but it is a third place a transient store error can fail a check that would otherwise have succeeded.
Low
isSwmMaterializationWitnessGraphis exported with zero consumers and a docstring promising "sync/serve exclusion assertions" that do not exist. The good news is that nothing is missing — exclusion is by allow-list, as above — so this is dead code. Wire it into an assertion or delete it; as written it implies a guard that isn't there.${assertionGraph}#dkg-swm-materialized:assertSafeIri(sparql-safe.ts:28) rejects<>"{}|\^, backtick and control chars but not#, so a graph IRI already containing#yields a double-fragment IRI. Worth a guard or a comment stating the precondition.JSON.stringify(digest)is used as SPARQL literal escaping in both the read and the write, where the repo hassparqlString/escapeSparqlLiteralfor exactly this. It happens to be correct for a hex digest; it is the wrong idiom to copy.verified-at-msis written on every witness, read by nothing and asserted by nothing — it doubles the row count for no consumer.- The measured percentages live in a source comment (
:180) with no benchmark artifact in the repo. Prose in the PR is the right home; code is not.
CI: 53 pass, 0 fail (2 pending at time of writing).
Verdict. The design is sound — writer-only, count-gated, digest-bound — and the isolation argument holds up better than the PR claims. What is missing is the last mile: the contract the module states is not satisfied by the one lock-visible writer outside this file, and the invariant that makes the whole thing safe has no test. H1 is one line, H2 is one test row. With both, I would merge this.
One meta-note, meant kindly: the "Why the COUNT gate stays" section is the best part of the write-up and it is what made the removal paths easy to verify. The same treatment applied to the replace paths would have caught H1 before review.
Addresses review at d5ecafa. Both blockers were reproduced before fixing, not taken on report. H1 — the module doc said "call this from every path that replaces or removes the graph's content WITHIN a lock this module can see", and the one such path did not. Live gossip (workspace-handler) derives the same swmGraph, takes the IDENTICAL swmKaWriteLockKey - with a comment saying it does so "so the public catch-up materializer serializes on the identical string" - calls tryReplaceGraphAtomically, and packages/ publisher had ZERO witness references. Confirmed by grep. The ordinary path is safe: a successful apply advances the head, so catch-up skips an older descriptor before the witness is consulted. The hole is a TORN apply - the replace succeeds, the snapshot-file or head write throws, leaving content=v2 head=v1 witness=D1 - after which a peer re-offering v1 passes the head guard, passes the count gate, and HITS the witness. Pre-#2079 the read-back returned false there and repaired it. Now invalidated from that path. H2 — "only the branch that VERIFIED it writes it" is the entire soundness argument, and NOTHING pinned it. Reproduced: changing `if (matches)` to `if (true)` passed every test in the repo. I predicted exactly this mutation in the plan's mutation set and then never ran it. Added a row that seeds v1, asks about v2 (count matches, digest does not), asserts no witness exists for v2's digest, and asks AGAIN. The second call is what kills the mutant, because an unconditional write would have memoized v2 on the first. Mutant now dies "expected true to be false". Also from the review: - The replaceGraph ordering comment asserted a FALSE safety property ("identical in effect"). A stale row misses for the new digest and HITS for the old one. Rewritten to state the asymmetry the digest binding does NOT cover - witness(v1) + content(v2) + descriptor(v1) - and to say plainly that this call is best-effort, so the residual is real rather than zero. Second time this comment overclaimed. - Dropped `verified-at-ms`: read by nothing, and every witness write appends a changelog marker, so it doubled that churn for no consumer. - Removed `isSwmMaterializationWitnessGraph` - exported, zero consumers, and its docstring implied a serve-exclusion guard that does not exist. Exclusion is by allow-list (isCandidateGraph), so nothing is missing. - `sparqlString` instead of `JSON.stringify` for literal escaping - the repo has a helper for exactly this and JSON.stringify was the wrong idiom to copy. - The witness ASK now `.catch`es to null: it is a pure optimisation, so a transient store error must degrade to "not memoized" rather than fail a check that would otherwise have succeeded. - Documented the `#`-in-graph-IRI precondition (assertSafeIri does not reject `#`). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012PC4iJT3UBkNC2iygHpVyd
Both blockers addressed —
|
otReviewAgent
left a comment
otReviewAgent
left a comment
There was a problem hiding this comment.
Operational Notice: Review Agent could not complete this review.
Business logic reviewer failed: WARNING: failed to clean up stale arg0 temp dirs: Permission denied (os error 13)
Jurij89
left a comment
Jurij89
left a comment
There was a problem hiding this comment.
Round-2 review at 098b93c0e
Both round-1 findings are genuinely fixed, and I proved it by execution rather than reading. One thing left, and it is the same defect at two more sites — which is partly my fault for naming one.
H1 — fixed, and proven end to end
Driven through the real SharedMemoryHandler.handle against a real OxigraphStore, with an armable snapshot store whose putSnapshot throws after the atomic replace commits. That reproduces exactly the torn state the new comment describes: handler swallows the error (torn apply threw to caller: false), countQuads(swmGraph) === 6, digest ≠ D1, and assertionVersion === '1' because the head write never ran.
- At
098b93c0e: witness survived = false,isGraphAssetMaterialized(dV1)= false. - With the
:1511call deleted: both true — a false hit certifying v1 while the store holds v2.
Placement is right too: inside withWriteLocks (:1370), after the replace (:1480), and before the snapshot (:1528) and head (:1546) writes — so a tear after the invalidate leaves no witness rather than a stale one.
H2 — fixed, and the row discriminates on both of its assertions
Making the write unconditional kills exactly one test — writes NO witness when the digest does NOT match, and stays false on re-check — failing at assertion 2 (line 182), with assertion 1 correctly passing as a control. Neutralising assertion 2 and re-running showed assertion 3 (line 187) kills independently: isGraphAssetMaterialized(d2) returns true on the second call while the store still holds v1. Two independent kills, not one assertion doing all the work.
I also checked the vacuity risk myself: payload('v1', 6) and payload('v2', 6) really do produce equal counts with different digests, so the row reaches the CONSTRUCT branch rather than short-circuiting at the count gate.
Also confirmed fixed: the ordering comment now states the asymmetry correctly and admits the residual is real rather than zero; the ASK is contained (.catch(() => null)); verified-at-ms is gone; sparqlString is used on both sides — verified in the built artifact, not just source, so the feature is provably not inert; the dead export is gone; the # precondition is documented.
The one thing left: the fix swept the instance, not the class
My round-1 write-up said "the one writer that satisfies its precondition", and named gossip. That framing invited an instance fix, and I should have gone looking for siblings. There are at least two, on the byte-identical URI, neither invalidating:
packages/agent/src/dkg-agent-publish.ts:2080—tryReplaceGraphAtomically(this.store, canonicalSwmGraph, …)on the graph-scoped update path, with the head persisted only afterwards.packages/publisher/src/storage-ack-handler.ts:915—tryReplaceGraphAtomically(this.store, swmGraphUri, normalized, …), withstoreKnowledgeAssetWorkspaceHeadat:937and two intervening store calls that can throw.
Both carry the same torn-write window as the gossip path that was just closed, and both are reachable from ordinary node operation. Bounded rather than permanent — both producers are owner-retried, and on retry the head advances so a later round evicts the stale row — but that is the same "bounded by retry" that applied to H1.
And the module doc is now actively wrong, which matters more than the sites because it is the safety argument the whole design rests on. swm-materialization-witness.ts:26-40 says:
Three paths remove an assertion graph … VM promotion / publish / update (
dropGraph(swmGraph)) … All three leave an empty or absent graph, which aCOUNTcatches for free.
The VM bullet is characterised as a drop. The graph-scoped update path replaces (dkg-agent-publish.ts:2080). A replace does not leave an empty graph, and an equal-count replace is precisely what the doc's own next paragraph says the count cannot catch. So the enumeration is not merely incomplete — its conclusion is false for one of the three entries it names.
To be precise about what is not wrong: the new comment at workspace-handler.ts:1495 ("the ONLY replace path outside the materializer holding a lock the witness module can see") is literally true — swmKaWriteLockKey has exactly two users. The problem is the module doc, not that sentence.
Suggested: add the same best-effort invalidate at dkg-agent-publish.ts:2080 and storage-ack-handler.ts:915 (both already hold the URI), and rewrite :26-40 to separate removals (count-covered) from replaces (not count-covered, must invalidate) rather than asserting a closed set of three.
Lower priority, same class: dkg-agent-lifecycle.ts:8836 (the SWM recovery lane, via swm-recovery.ts:345/:474) also replaces without invalidating. In automatic operation it is lane-disjoint — planSharedMemorySyncContextGraphs partitions on isPrivateContextGraph and the witness is only wired into the public lane — so it is unreachable there. It becomes reachable through the ungated POST /api/context-graph/recover-shared-memory route (context-graph.ts:1728), which has no public/private check and is documented as the repair tool for a corrupt local SWM copy.
Smaller
- H1 itself shipped unpinned. Deleting the
:1511call survives the publisher's full suite — 125 files / 1800 tests, byte-identical to baseline, mutation verified still present afterwards. No test in the repo mentions the witness andSharedMemoryHandlertogether. Worth a guard, given this is the second fix in a row to land without one. The cheap version is three lines inpackages/publisher/test(which imports../src/workspace-handler.jsdirectly): seedwriteSwmMaterializationWitness(store, swmGraph, d), run the graph-scoped apply, assert the witness is gone — no torn-apply rig needed. Note a test inpackages/agent/testwould not work: that lane resolves@origintrail-official/dkg-publisherfromdist, so a src change is invisible without a rebuild, and new files there must also be added to the explicit include list. - The read containment is unpinned too — making
readSwmMaterializationWitnessrethrow survives the agent suites 25/25. - The H1 fix adds a
deleteByPatternto the live gossip apply path, so a KA whose witness stands now costs one extra changelog marker and one index bump per apply. Small, and the decorators short-circuit on a no-op delete, but it is new cost on the hot path — worth knowing, since halving the witness rows was partly about this. - Stray blank line at
swm-snapshot-materializer.ts:220whereDate.now(),was removed. - The PR description has not kept pace with the code. Its "Files changed" table omits
packages/publisher/src/workspace-handler.tsentirely — the file carrying the H1 fix, the most consequential change in this round. It still says "7 rows" (now 8), and it still describes the materializer's invalidation as "defence-in-depth", which the code's own rewritten comment now correctly contradicts. I have twice called that description the best part of this PR; it is worth keeping it true.
Residuals from round 1, unchanged and still fine to ship as tracked
No GC (rows orphaned by the TTL sweep, VM publish and chain-reset wipe); the witness graph is in no reserved set so every write still appends a changelog marker (now one row instead of two); and on sparql-http with atomicUpdates:false the memo can never pay while the read still runs on every check.
CI: 55 pass, 0 fail. Unrelated: packages/storage's own suite is red on base (8 failed / 464 passed — oxigraph-worker-respawn and the storage.test.ts factory, both 5000 ms timeouts, in files this PR does not touch). Not attributable, and not used for any mutation signal above.
Verdict. The two fixes are correct and now proven. Add the invalidate at the two sibling replace sites, correct the three-path enumeration in the module doc, and I would merge. The unpinned-H1 test and the description refresh are worth doing in the same pass but would not hold it.
…2079) Round-2 review. The round-1 fix swept the INSTANCE, not the class - my error, and the module doc made it worse by asserting a closed set. THE DOC WAS ACTIVELY WRONG. It listed "VM promotion / publish / update" as a path that DROPS the graph, concluding "all three leave an empty graph, which a COUNT catches for free". But the graph-scoped update path REPLACES (dkg-agent-publish). A replace leaves the count intact, which the doc's own next paragraph says the count cannot catch. Rewritten to split REMOVALS (count-covered: TTL sweep, chain-reset wipe) from REPLACES (never count-covered, MUST invalidate), with the list marked a snapshot rather than a closed set and a standing rule: a new tryReplaceGraphAtomically against a SWM assertion graph is a new obligation here. Invalidate added at the three sibling replace sites: - dkg-agent-publish (graph-scoped VM update; head persisted after) - storage-ack-handler (head write + two store calls follow) - swm-recovery x2 (lane-disjoint in automatic operation, but the ungated recover-shared-memory route reaches it - and that route exists to repair a corrupt copy, the worst moment for a stale memo) invalidateSwmMaterializationWitness now takes the CAPABILITY it needs (deleteByPattern) rather than a full TripleStore. The recovery lane holds a SwmRecoveryStore; demanding a TripleStore would have made that site uncallable and quietly left it out of the set - the same shape of mistake as the doc's closed list. H1 SHIPPED UNPINNED, as the reviewer found: deleting the gossip invalidate survived the publisher's whole suite. Now pinned in packages/publisher/test (NOT agent - that lane resolves publisher from dist, so a src change would be invisible). Its own test, because the second apply adds metadata and would break sibling assertions. Mutating the invalidate to target a different graph kills it: expected true to be false. READ CONTAINMENT was unpinned too - making the ASK rethrow survived 25/25. Now pinned. Also, per the earlier discussion about the one config where this change could only ever cost: - STATIC PROBE. A store with no replaceSubject can never hold a witness, so every ASK on it was permanent added cost for a hit rate of zero (sparql-http with atomicUpdates:false). Detected before the first query; that config is now byte-identical to pre-#2079. Pinned by a row asserting ZERO ASKs are issued. - A decorator's preflight refusal is deliberately NOT latched: it may be conditional, and latching it would disable the memo for the process on a transient event. - DKG_SWM_MATERIALIZATION_WITNESS kill switch, default on, blank means unset (matching the pass-budget parser: `VAR=` in a compose file must not read as false). It gates the read and the write; the invalidations always run, because turning the memo off must not turn off what keeps existing memos honest - otherwise an off-period replace leaves a stale row that becomes a false hit the moment someone turns it back on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012PC4iJT3UBkNC2iygHpVyd
Round-2 addressed —
|
otReviewAgent
left a comment
otReviewAgent
left a comment
There was a problem hiding this comment.
Operational Notice: Review Agent could not complete this review.
Business logic reviewer failed: WARNING: failed to clean up stale arg0 temp dirs: Permission denied (os error 13)
Jurij89
left a comment
Jurij89
left a comment
There was a problem hiding this comment.
Round-3 review at a79e09d4
The module doc rewrite is exactly right — splitting REMOVALS (count-covered) from REPLACES (must invalidate), and adding "this list is a snapshot, not a closed set … treat adding one without an invalidate as a defect" is a better answer than the one I asked for. The widened structural parameter is a genuinely good call too: requiring a full TripleStore would have made the recovery lane uncallable and quietly left it out of the set, and the comment says so.
Two mechanisms landed. One works. One cannot fire.
The killswitch works — proven by execution
Driven through a counting proxy on a warm store (witness present, content matching), three rounds each:
DKG_SWM_MATERIALIZATION_WITNESS |
ASKs | write attempts | verdict |
|---|---|---|---|
0 / false / FALSE |
0 | 0 | correct |
unset / '' / ' ' |
3 | — | correct, memo ON |
Blank and whitespace really are treated as UNSET, as the comment claims. It gates both the read fast path and the write, so with it off the check is byte-identical to pre-#2079. The env is read at :144 inside the factory, which dkg-agent-lifecycle.ts:6046 calls per SWM sync round per peer — not once per boot — and dkg start passes process.env straight through, so it takes effect on the next round after a restart. That is a real, usable operator control.
The static capability probe is dead
const witnessUnsupported = typeof deps.store.replaceSubject !== 'function' — typeof was measured as literally "function" for all six production assemblies built through the real createTripleStore, including the exact config the comment names:
sparql-http+atomicUpdates:false, bare and decorated →"function"sparql-httpmanaged,blazegraphbare and decorated,oxigraphdecorated →"function"
sparql-http.ts:501 defines replaceSubject and throws UnsupportedTripleStoreCapabilityError inside it at :511. The store is not missing the method — it refuses. All three decorators (ChangelogStore:410, GraphSetIndexStore:433, SharedMemoryLiteralBlobStore:133) likewise define it unconditionally and throw inside. tryReplaceSubjectAtomically (triple-store.ts:315-336) handles both shapes — the typeof check at :322 and the catch at :327-333. The probe replicates only the weaker half, which is the half nothing in this repo produces.
So the comment's claim — that knowing this "takes that config from permanent regression to byte-identical to pre-#2079" — is false in both directions.
And here is the part I'd have missed without pushing on it: the permanent regression it claims to eliminate doesn't exist either. sparql-http gates replaceGraph (:434) on the same atomicUpdates flag, so on that config no writer can populate a SWM assertion graph at all — gossip, StorageACK, VM update, recovery and the materializer's own replace all hard-fail. The graph stays empty, the count gate returns at :193, and the ASK at :206 is never reached. Zero extra round-trips.
Net: dead code carrying a false capability claim. No runtime harm — but the test written for it, issues NO witness ASK when the store cannot hold a witness (:229-263), fabricates its precondition with if (prop === 'replaceSubject') return undefined and restates the false premise in its comment. That is a check that cannot fail, certifying behaviour that never occurs. Round 2 fixed one of these at test:294; this is a new one.
Simplest resolution: delete the probe, the !witnessUnsupported term and that test. If you want to keep a defensive guard, it is defensible for an SDK-injected store (dkg-agent.ts:819-820 lets a caller pass one, and replaceSubject? is optional) — but then say that, and drop the sparql-http sentence. A latch on the first false return would be the real fix, but note it can't be done safely as-is: the write is wrapped in .catch(() => false) at :247, so a transient endpoint error would latch the memo off process-wide — the exact outcome :134-138 is trying to avoid.
A sixth replace site
The doc now enumerates five and says a new one without an invalidate is a defect. There is a sixth in-tree today:
packages/publisher/src/dkg-publisher.ts:8663 — replaceExactKnowledgeAssetGraph(swmGraphUri, swmQuads, 'Knowledge Asset WM-to-SWM promotion'), where swmGraphUri at :8048 is knowledgeAssetLayerGraphUri(cg, MemoryLayer.SharedWorkingMemory, contentScope, subGraphName) — the witness key derivation exactly. invalidateSwmMaterializationWitness has zero occurrences in that file, and replaceExactKnowledgeAssetGraph is tryReplaceGraphAtomically (:6962).
The comment immediately below the call even names the window: "Every write between here and there is fallible." The tail runs :8663 → :8707 storeKnowledgeAssetOperationPublicQuads → :8729 storeKnowledgeAssetWorkspaceHead.
Reachable on default config. The author node does witness its own KA — shared-memory-sync.ts:607 fires onSnapshotReady(snapshot, 'cache') with no self-peer filter — and the curator-ack gate is off by default (dkg-agent-publish.ts:2368, swmAwaitCuratorAck ?? false), with gossip published only after promote returns. So: promote v2, tail throws, curator still advertises v1, next round's descriptor is v1, equal count, witness ASK for v1 hits.
Bounded rather than permanent — WM is deliberately retained to the last step so a promote retry converges — and the blast radius is local, since peers fetch the content-addressed snapshot blob rather than this node's assertion graph. Fix is the same one-liner; the file already imports from dkg-storage. I'd resist folding it into replaceExactKnowledgeAssetGraph itself: five of its seven call sites are VM or WM graphs that can never hold a witness, and it would add a serialised changelog round-trip to each.
Pinning
Round 2's H1 is now genuinely pinned — ka-graph-workspace-receiver.test.ts:143 discriminates, and it discriminates for the right reason: the witness row lives in urn:dkg:local:*, untouched by the graph replace, so only the workspace-handler.ts:1512 call can clear it. Good.
The three new invalidate sites are not. Deleting any of dkg-agent-publish.ts:2092, storage-ack-handler.ts:923, or either swm-recovery.ts call leaves the suite green. Given the module doc now calls exactly that "a defect", one guard per site is worth having — and the publisher test just showed the cheap shape: seed a witness, run the path, assert it's gone.
Smaller
- Placement before
if (!replaced) throwin the two new sites is harmless and mildly fail-safe —tryReplaceGraphAtomicallyreturns false only on a clean preflight refusal with no mutation, so invalidating there costs one recomputation. But it does not buy what it might appear to: a genuine execution failure throws, and the throw propagates before the invalidate line, so an indeterminate replace still leaves the witness standing. Also noteworkspace-handlerputs its call after the throw check and these two put it before — worth making consistent, whichever you prefer. - The killswitch does not gate the invalidates (four of five sites are in other packages and can't see the flag). Safe direction — switching off can never strand a stale witness — but "disabled" still pays one
deleteByPatternper SWM replace, and never GCs rows written while it was on. Cheap:ChangelogStoreonly emits a marker whenremoved > 0. - A stale comment block in the first publisher test asserts "deleting that call must fail this" on a test where deleting the invalidate changes nothing; the scenario was moved to the sibling test below. Same class as the
test:294comment fixed in round 2 — worth deleting the residue. swm-recovery.ts:483's invalidate passes no options, and theSwmRecoveryStoreadapter supplies its own, so that store op is attributed asagent.swmRecovery.deleteByPatternrather than anything witness-specific. Cosmetic, but the backpressure work in #2003/#2107 was specifically about making these attributable.- Residuals unchanged and still fine as tracked: no GC for orphaned rows; the witness graph is in no reserved set.
CI: 53 pass, 0 fail (2 pending).
Verdict. Add the dkg-publisher.ts:8663 invalidate and deal with the dead probe — delete it, or keep it and make its comment and its test true. Neither is large. Everything else here is either correct, deliberately deferred, or a nit. The killswitch in particular is well built: correct parsing, both gates, re-read per round, and I could not find a way to make it lie.
Round-3 review. Both findings valid; the probe one is the sharper. THE STATIC PROBE WAS DEAD, AND ITS TEST WAS A CHECK THAT CANNOT FAIL. `typeof deps.store.replaceSubject !== 'function'` never fires: every adapter and all three decorators (ChangelogStore, GraphSetIndexStore, SharedMemoryLiteralBlobStore) DEFINE replaceSubject and throw UnsupportedTripleStoreCapabilityError INSIDE it. The store does not lack the method - it refuses. tryReplaceSubjectAtomically handles both shapes; the probe replicated only the half nothing in this repo produces. Worse, the regression it claimed to remove does not exist either. sparql-http gates replaceGraph on the SAME atomicUpdates flag, so on that config no writer can populate a SWM assertion graph at all. The graph stays empty, the count gate returns first, and the ASK is never reached. So the comment was false in both directions - and I argued that framing confidently before writing it. The test written for it (`issues NO witness ASK when the store cannot hold a witness`) fabricated its own precondition with `if (prop === 'replaceSubject') return undefined` and restated the false premise in its comment: a check that cannot fail, certifying behaviour that never occurs. Deleted along with the probe. The third instance of that pattern on this PR, and the first one I authored knowingly enough to have caught. The comment now records why there is no probe, and why a latch on the first `false` cannot be added as currently wired: the write is `.catch(() => false)`, so a transient endpoint error is indistinguishable from a capability refusal and would disable the memo process-wide. SIXTH REPLACE SITE: dkg-publisher WM-to-SWM promotion. Same graph derivation, no invalidate, and reachable on DEFAULT config - the author node witnesses its own KA (onSnapshotReady 'cache' has no self-peer filter) and the curator-ack gate is off by default, so promote v2 + a throw in the fallible tail leaves the curator advertising v1, the next descriptor at v1, count matching, and a v1 witness hitting. Not folded into replaceExactKnowledgeAssetGraph: five of its seven call sites are VM or WM graphs that can never hold a witness. Nits from the same round: invalidate placement normalised to AFTER the !replaced throw guard at both new sites (matching workspace-handler); swm-recovery's two calls now carry a witness-specific `source` so the store op is attributable rather than landing under agent.swmRecovery.deleteByPattern; removed the stale comment block left behind in the first publisher test when the scenario moved to its sibling - same residue class as the one fixed in round 2. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012PC4iJT3UBkNC2iygHpVyd
Round-3 addressed —
|
otReviewAgent
left a comment
otReviewAgent
left a comment
There was a problem hiding this comment.
Operational Notice: Review Agent could not complete this review.
Business logic reviewer failed: WARNING: failed to clean up stale arg0 temp dirs: Permission denied (os error 13)
One of the four unpinned invalidate sites the round-3 review flagged. Extends the existing graph-scoped recovery row rather than adding a harness: it already builds a real assertionGraph, seeds stale content in it, and drives recoverContextGraphSwm against a real OxigraphStore, so the only thing missing was the witness. It discriminates for the right reason: the witness lives in urn:dkg:local:*, which the graph replace does not touch, so ONLY the explicit invalidate in the recovery lane can clear it. Mutating that call to target a different graph kills the row: expected true to be false. The other three sites are NOT pinned by this commit, and the reason is worth stating rather than leaving as silence. No test in the repo drives the real WM-to-SWM promote tail (`replaceExactKnowledgeAssetGraph` / "WM-to-SWM promotion" have zero occurrences under test/), so guarding dkg-publisher and the storage-ack path needs new fixtures rather than an insertion. Given this PR has now shipped three guards that did not discriminate, building those in a hurry is the wrong instinct - they want their own pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012PC4iJT3UBkNC2iygHpVyd
otReviewAgent
left a comment
otReviewAgent
left a comment
There was a problem hiding this comment.
Operational Notice: Review Agent could not complete this review.
Business logic reviewer failed: WARNING: failed to clean up stale arg0 temp dirs: Permission denied (os error 13)
Jurij89
left a comment
Jurij89
left a comment
There was a problem hiding this comment.
Convergence review at 1721d515
Close. One four-line fix and I'd merge — plus the doc change that matters more than the fix.
First: the red CI was a GitHub outage, and it's now green
Every run on this head died with Failed to resolve action download info. Error: Service Unavailable — the runner could not download action definitions, so no test executed. The red SQLite lifecycle (Windows) check carried no signal, and the five "failures" were the concurrency cancel that followed. Other branches were passing again from 23:22Z, so I re-ran the failed runs: 56 pass, 3 skipped, 0 fail, 0 pending.
Because CI hadn't run when I started, I ran it locally instead — a fresh pnpm install --frozen-lockfile + full build, with dist grepped to prove emission rather than trusted:
| Suite | Result |
|---|---|
packages/publisher FULL |
125 files / 1801 passed, 6 skipped, 0 failed |
packages/agent, the PR's actual blast radius (11 files across all 3 changed src files) |
220 passed, 0 failed |
packages/agent new witness file under the default CI config |
2 files / 26 passed |
tsc --noEmit × storage, publisher, agent |
exit 0 |
And a question left open in round 3 is closed: scripts/ci-shard-agent.mjs readdirSync-walks test/ rather than reading the include list, so the new test file is discovered by CI automatically.
Two red lanes were baselined rather than assumed: packages/storage's oxigraph-worker timeouts and two RFC-64 agent integration files both reproduce identically with src reverted to the merge base (30 failed / 21 passed either way). Not attributable. One honest caveat: log capture was tail-truncated, so 3 further agent files / 15 failures could not be enumerated and remain unattributed rather than cleared.
Round-4 changes verified
- Probe deletion is behaviour-preserving.
typeof store.replaceSubject === 'function'on all 8 real store shapes — includingsparql-httpwithatomicUpdatesboth false and true — so!witnessUnsupportedwas always true and the old expression always equalled the new one. The single differing case is a store object literally lacking the method (SDK-injected): it now pays one ASK + one CONSTRUCT per round and stores nothing. Wasteful, correct, contained. - The killswitch still works after the two consts collapsed into one: 11 values driven through a counting proxy —
0/false/FALSE/False→ 0 ASKs, 0 rows; unset/''/' '/1/true/off/no→ memo ON. - The sixth site's invalidate fires, proven through the real promote path (
DKGPublisherover a realOxigraphStore, no chain adapter, seeded witness on the resolved SWM graph,assertionPromote→ witness gone) — and the discriminator was verified: replacing the call with a no-op makes the same probe report the witness still standing.
The comment recording why the probe was deleted is the right instinct, and it's accurate on all three of its claims.
The one thing left: an 8th replace site
packages/agent/src/rfc64/public-catalog-native-receiver-v1.ts:1913 — activateExactPublicProjection writes the verified catalog projection into derivePublicSwmGraph(cg, kaId), which resolves to did:dkg:context-graph:{cg}/_shared_memory/{lowercase-addr}/{n} — byte-identical to the witness key. There is zero invalidateSwmMaterializationWitness anywhere under packages/agent/src/rfc64/. It's a REPLACE, so the count gate cannot see it.
The root cause is the tripwire itself. swm-materialization-witness.ts:52 says:
A new
tryReplaceGraphAtomicallyagainst a SWM assertion graph is a new obligation here
This site uses tryReplaceGraphAndSubjectAtomically — a different primitive. Four rounds of greps searched the token the doc names, which is exactly why the count kept coming up short. My own round-3 sweep used it too, and so did one of this round's lenses, which reported "7 is correct and complete" on that basis. The grep was wrong, not the reviewers.
I swept the missing token properly. Seven call sites; within them exactly one genuine miss:
:1913activation — replaces with content, no invalidate → the finding:1739rollback restore — restores the exact preimage, so the witness becomes valid again → no obligation:1823deactivation — replaces with[], so the graph is empty and the count gate covers itfinalization-handler.ts:1282/:1913andgraph-scoped-materialization.ts:384/:400— VerifiableMemory, not SWM
Severity: MEDIUM, and it does not block. rfc64PublicCatalog is documented as "Opt-in, bounded RFC-64 catalog activation for explicitly selected public CGs" and appears in no shipped network config — an operator must deliberately enable it. Where it is enabled, reachability is real rather than theoretical: activation merges its selected CGs into syncContextGraphs, so the two lanes land on the same graph by construction, and the catalog seal writes {scope}/_meta rather than the SWM head, so the version guard doesn't intervene.
One thing I won't overclaim: whether this is worse than pre-#2079 is genuinely arguable. Before, the CONSTRUCT caught the mismatch — but "repaired" it by overwriting the signed catalog projection with an older peer snapshot. After, the projection survives with a stale SWM head. Both are wrong; I don't think either is clearly worse, and since it takes an opt-in to reach at all, it's the operator's trade to understand.
Fix: four lines after the if (!replaced) guard at :1913, mirroring the other seven — and, more importantly, widen the tripwire at :52 to name both primitives. That is the higher-leverage half by a distance: it's the thing that prevents round five.
Follow-ups, none blocking
- The module doc's REPLACE list is stale on arrival — it names five entries and omits the WM→SWM promotion this same commit added, while
dkg-publisher.ts:8668calls that site "the SIXTH replace site". The list is the maintenance contract; it should at least be self-consistent. (The "snapshot, not a closed set" sentence does carry the obligation regardless, which is why this is LOW.) Same comment says "five of its seven call sites" wherereplaceExactKnowledgeAssetGraphhas six. - The promote-site invalidate is unpinned. Mutating it to a no-op left the whole repo green. One publisher row — seed a witness, run promote, assert it's gone — mirrors the gossip test that already works. The other sites can follow.
dkg-agent-publish.ts:2092still invalidates before itsif (!replacedSwm) throw, whileworkspace-handlerand (as of round 4)storage-ack-handlerboth throw first. Fail-safe either way; it's now the only one of three with that ordering, so worth aligning or annotating as deliberate.- The accepted residuals aren't written down anywhere — no GC for rows orphaned by the TTL sweep / VM publish / chain-reset wipe, and a changelog marker per witness write. Both are reasonable trades, but an accepted trade-off nobody recorded is indistinguishable from an oversight six months from now. A short "Known costs" paragraph in the module doc, or an issue, would close this out properly.
- Stray double blank line at
ka-graph-workspace-receiver.test.ts:82where round 4's comment deletion landed.
Verdict: converge after the :1913 invalidate and the :52 tripwire widening. Everything else here is a follow-up. The core design has held up under four rounds of adversarial review — writer-only, count-gated, digest-bound — and the mechanisms added along the way (killswitch, invalidate set, the deleted probe with its negative result recorded) are each now verified by execution rather than argument. The remaining defect is one line of grep in a comment, and the sites it failed to surface.
|
Addendum — the head moved to I reviewed Two updates:
Convergence call is unchanged: the |
Convergence review. The tripwire caused the miss it existed to prevent.
The module doc said "a new `tryReplaceGraphAtomically` against a SWM
assertion graph is a new obligation here". Four review rounds then
grepped the token it named - mine included - and kept reporting the set
complete, while rfc64 public-catalog ACTIVATION sat unlisted and
uninvalidated because it uses tryReplaceGraphAndSubjectAtomically, a
different primitive.
Naming one function in a safety comment turned out to be worse than
naming none: it told every subsequent sweep what to search for, and the
answer was wrong.
The tripwire now names the SHAPE and enumerates all three primitives in
tree (tryReplaceGraphAtomically, tryReplaceGraphAndSubjectAtomically,
bare store.replaceGraph), with the failure recorded so the next person
understands why the list is written that way.
8TH SITE: public-catalog-native-receiver-v1.activateExactPublicProjection.
derivePublicSwmGraph resolves to the byte-identical witness key, and it
is a REPLACE, so the count gate cannot see it. Activation merges its
selected CGs into syncContextGraphs, so where rfc64PublicCatalog is
enabled the two lanes land on the same graph by construction, and the
catalog seal writes {scope}/_meta rather than the SWM head so the
version guard does not intervene. Gated behind an opt-in that appears in
no shipped network config, hence MEDIUM.
Two nearby rfc64 sites are deliberately NOT obligations and the doc now
says so: rollback restore puts back the exact preimage (a standing
witness becomes valid again), and deactivation replaces with [] (empty
graph, count-covered).
Also:
- The REPLACE list was stale on arrival - it omitted the WM->SWM
promotion the same commit added. Now numbered 1-8 and self-consistent.
- "five of its seven call sites" was wrong on both numbers. ENUMERATED:
replaceExactKnowledgeAssetGraph has SIX call sites, of which this is
the only SWM one (rest: dataGraph x2, vmGraph, wmGraph, one
pass-through). The comment now says it was enumerated, not counted.
- KNOWN COSTS paragraph added to the module doc: no GC for orphaned
rows, a changelog marker per write, and a read that is now also a
write on the miss path. All accepted deliberately - written down
because an accepted trade nobody recorded is indistinguishable from an
oversight six months later.
- Stray double blank line from round 4's comment deletion.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012PC4iJT3UBkNC2iygHpVyd
Converged —
|
otReviewAgent
left a comment
otReviewAgent
left a comment
There was a problem hiding this comment.
Operational Notice: Review Agent could not complete this review.
Business logic reviewer failed: WARNING: failed to clean up stale arg0 temp dirs: Permission denied (os error 13)
2 participants
Summary
COUNT, a fullCONSTRUCTof the assertion graph, and a SHA-256 over the result — per descriptor, per pass, insidewithKaWriteLock, so it blocked live gossip for that KA. Amplified byCATCHUP_MAX_CONCURRENT_PEER_SYNCS(4) ×DEFAULT_SWM_CATCHUP_MAX_PASSES(4). A warm check is now a bound-subjectASK.CONSTRUCT+digest is 0.91 / 2.69 / 15.98 / 197.81 ms; the witnessASKis ~0.03–0.06 ms. That is 46 / 55 / 74 / 90 % less local work. The default backend isoxigraph-worker, where CONSTRUCT results cross apostMessage+ structured clone — so these are lower bounds in production.ASK. TheCOUNTgate is kept, and that is the load-bearing decision — see below.Related
Diagrams
The already-materialized check, warm pass
Before — every descriptor pays a full read-back and digest, inside the write lock:
sequenceDiagram participant Sync as catch-up pass participant Lock as withKaWriteLock participant Store as triple store Sync->>Lock: acquire (blocks live gossip for this KA) Lock->>Store: COUNT assertion graph Store-->>Lock: n == expected Lock->>Store: CONSTRUCT whole assertion graph Store-->>Lock: all quads Note over Lock: SHA-256 over every quad<br/>198 ms at 5000 quads Lock-->>Sync: already materializedAfter — the digest is computed once, then remembered:
sequenceDiagram participant Sync as catch-up pass participant Lock as withKaWriteLock participant Store as triple store Sync->>Lock: acquire Lock->>Store: COUNT assertion graph Store-->>Lock: n == expected Note over Lock: count gate KEPT - catches every<br/>drop-shaped damage path for free Lock->>Store: ASK witness (subject + digest bound) Store-->>Lock: hit Lock-->>Sync: already materialized (no CONSTRUCT)Why the witness cannot outlive its content
sequenceDiagram participant Sweep as TTL sweep / VM publish / chain reset participant Store as triple store participant Check as isGraphAssetMaterialized Sweep->>Store: drop assertion graph Note over Store: witness SURVIVES - the chain-reset scoped<br/>delete spares urn:dkg:local:* Check->>Store: COUNT assertion graph Store-->>Check: 0 Note over Check: 0 != expected, so the witness is<br/>never consulted - miss, then repair Check-->>Check: NOT materializedFiles changed
packages/storage/src/swm-materialization-witness.tsurn:dkg:local:swm-materialization-witness, one subject per assertion graph with the digest as an object so a new digest evicts the old claim atomically. Write usestryReplaceSubjectAtomicallyand skips entirely when unsupported. Module doc states why callers must keep a count gate.packages/storage/src/index.tspackages/agent/src/sync/requester/swm-snapshot-materializer.tsASKbetween the count gate and the CONSTRUCT; witness written from the verification branch; invalidation inreplaceGraph; static capability probe +DKG_SWM_MATERIALIZATION_WITNESSkill switchpackages/publisher/src/workspace-handler.tsswmKaWriteLockKey, a replace the count gate cannot seepackages/agent/src/dkg-agent-publish.tspackages/publisher/src/storage-ack-handler.tspackages/agent/src/sync/requester/swm-recovery.tsrecover-shared-memoryroute)packages/publisher/test/ka-graph-workspace-receiver.test.tspackages/publisherbecause the agent lane resolves publisher fromdistpackages/agent/test/swm-materialization-witness.test.tsOxigraphStorepackages/agent/vitest.unit.config.tsincludeallow-list (a file not listed is silently uncollected)Why the COUNT gate stays — read this before approving
The issue asks for "a single bound-subject
ASK", i.e. the witness replacing the whole predicate. That trades away self-healing for very little:Two classes of thing happen to an assertion graph, and conflating them is what an earlier revision of this PR got wrong:
REMOVALS — the count gate covers these for free (
count 0 != expected), no invalidation possible or needed:cleanupExpiredSharedMemory— timer-driven, no lockSPARQL_SCOPED_DELETEfilters on exactly the context-graph, publisher and changelog prefixes — so aurn:dkg:local:*witness survives a wipe that deletes every context-graph tripleUnder ASK-only the wipe certifies an empty store as parity, permanently and silently. The TTL case is worse — self-reinforcing, since the head is reinstalled over an empty graph and re-expired forever. Measured, also dropping the count buys a further 1.5–10.5 %. Not a good trade.
REPLACES — never count-covered; every one must invalidate. A replace can leave the quad count unchanged while the content differs. All five known sites now do: the materializer's own
replaceGraph, live gossip apply, the graph-scoped VM update, storage-ack persistence, and both private-recovery replaces. The list is documented as a snapshot, not a closed set — a newtryReplaceGraphAtomicallyagainst a SWM assertion graph is a new obligation.Soundness
v1 → v2— the case the count cannot catch — is bounded by the read binding the digest: a standing v1 row cannot satisfy anASKfor v2's digest. That covers (old witness, new descriptor). It does not cover (old witness, new content, old descriptor) — which is why every replace site invalidates, and why those calls are not merely hygiene. They are best-effort, so the residual is real rather than zero.replaceSubject(sparql-httpwithatomicUpdates:false) issues zero ASKs and is byte-identical to pre-Sound O(1) already-materialized check via a materializer-written witness #2079. A decorator's preflight refusal is deliberately not latched — it may be conditional, and latching would disable the memo for the process on a transient event.DKG_SWM_MATERIALIZATION_WITNESSdisables the read and the write; the invalidations always run, because turning the memo off must not turn off what keeps existing memos honest.Honest scope — this does NOT make a repeat pass O(1)
hasValidSnapshotstill does a whole.nqreadFile, a parse, and a second full digest per manifest ref insyncPublicSnapshotsForMeta, beforeonSnapshotReadyfires — untouched by a descriptor-level witness. Refs with no descriptor (urn:dkg:public-stage:*entity shares) pay it and gain nothing.The pass stays O(total CG bytes). The claim is 2×–11× less local per-KA work, and the wall-clock share of a whole pass is not yet measured.
Test plan
10 new rows against a real
OxigraphStore, plus a publisher-side guard; existing materializer and recovery suites still greenFull agent closure builds (
tscexit 0)Mutation, disjoint rows, assertion deaths:
COUNTconjunct (ASK-only)expected true to be false— a standing witness certifies an empty graph, i.e. exactly the ASK-only failureexpected 2 to be 1— a secondCONSTRUCTreappearsif (matches))expected true to be false— this is the invariant that killed the head-row proposal, and it shipped unpinned in the first revisionexpected true to be falseNot done: hit-rate instrumentation on a live warm pass. If witness hits turn out to be rare, the memo is not paying and this should be reverted rather than tuned.