perf(sync): bound selected SWM responder planning - #2283
Conversation
| }, | ||
| ), | ||
| }) | ||
| : [], |
There was a problem hiding this comment.
🔴 Bug: TTL SWM DATA no longer serves graph-backed snapshot payloads
What's wrong
The empty graph list avoids inventory scans, but it also empties the allowlist that the graph-backed snapshot planner uses. Modern rootless SWM operations with dkg:publicSnapshotGraph can still sync their metadata, but their payload graph is never returned, leaving cold receivers unable to materialize those assets.
Example
A fresh graph-scoped SWM operation has head metadata plus dkg:publicSnapshotGraph <G> and dkg:publicQuadsCount 3, with 3 rows stored in graph G and no legacy dkg:rootEntity. Through registerSyncHandler, the TTL DATA request now calls readSwmDataPage with graphList: []; graph-backed discovery skips the meta graph and the response is empty. Expected behavior is that the DATA phase returns the rows from G so the requester can count/digest-materialize the advertised graph-backed KA.
Suggested direction
Do not pass an empty inventory into the graph-backed path unless that path is also changed to discover/count the advertised snapshot graphs without graphSet. Alternatively, if graph-backed content is being moved to another phase, update the requester contract at the same time.
For Agents
Look at registerSyncHandler's SWM DATA branch and readFreshGraphBackedSwmSnapshotEntries. Preserve the no-full-inventory optimization for root-scoped legacy data, but still allow graph-backed snapshot metadata/graphs to be discovered and counted. Add a handler-level test for a publicSnapshotGraph operation with no rootEntity that proves the DATA response includes the immutable snapshot graph rows.
| // index lookup (0.1s rather than 10s+ on the 500-KA canary corpus). | ||
| const branches = chunk.map((metaGraph) => `{ | ||
| GRAPH <${assertSafeIri(metaGraph)}> { | ||
| ?op <${DKG_ROOT_ENTITY}> ?root ; |
There was a problem hiding this comment.
🔴 Bug: Fresh root preflight admits non-operation metadata roots
What's wrong
The responder and requester now disagree on what makes a legacy SWM root valid. A stray or malicious metadata subject with rootEntity and publishedAt can cause the responder to transmit data that the requester will reject, wasting byte-budget pages and potentially exposing rows that were not backed by a valid workspace operation.
Example
Insert SWM metadata rows { subject: 'urn:not-an-op', predicate: dkg:rootEntity, object: 'urn:r' } and { subject: 'urn:not-an-op', predicate: dkg:publishedAt, object: freshDate }, but no rdf:type dkg:WorkspaceOperation, plus payload rows rooted at urn:r. The new planner selects urn:r and serves those payload rows; the requester verifier later treats the root as invalid and drops them. Expected behavior is for the responder plan to ignore the root just like the verifier does.
Suggested direction
Require rdf:type dkg:WorkspaceOperation in the preflight, or perform an equivalent validation before adding roots to freshRootsByMetaGraph.
For Agents
In buildFreshSwmDataGraphPlan, keep the cheap metadata preflight but make its root admission match sync-verify-worker-impl.ts: rootEntity should come from a fresh WorkspaceOperation in the same meta graph. Add a plan/handler test where rootEntity+publishedAt without WorkspaceOperation does not produce DATA rows.
| expect(new TextEncoder().encode(first).byteLength) | ||
| .toBeLessThanOrEqual(SYNC_BYTE_BUDGET_RESPONSE_BYTES); | ||
|
|
||
| const second = await cap.invoke({ ...request, offset: firstRows.length }); |
There was a problem hiding this comment.
🔴 Bug: The byte-truncated continuation test does not verify snapshot retention
What's wrong
The test added for continuing a byte-truncated shared-memory meta page gives false confidence: because the store is unchanged between requests, it would pass even if the responder released the session snapshot after the short serialized page and rebuilt from live data on the second request.
Example
After first is read, delete one early row or insert a new row that sorts before the second-page offset, then fetch offset: firstRows.length. A responder that dropped the cached snapshot could skip or duplicate rows, but the current test does not exercise that failure mode.
Suggested direction
Capture the expected first-session row set before mutation, mutate the store between page requests, then assert page 2 continues that original row set and does not reflect the mutation.
For Agents
Strengthen packages/agent/test/sync-byte-budget-pages.test.ts in the byte-truncated SWM meta continuation case: mutate the meta graph between page 1 and page 2 and assert the combined pages are the original snapshot, with no duplicates or missing original rows. This should prove the releaseCacheOnShortPage behavior rather than just byte sizing.
| ); | ||
| const rows = await readSwmMetaPage({ | ||
| store, | ||
| graphList: cutoff == null |
There was a problem hiding this comment.
🟡 Issue: Do not use an empty graph list as a hidden planner mode
What's wrong
This adds a sentinel-based control path across the handler and graph planner. It makes the boundary harder to reason about because graphList no longer consistently means "the admitted graph inventory"; its meaning depends on phase and TTL state. That is exactly the kind of implicit mode that tends to become permanent debt as more SWM lanes are added.
Example
A reader of readSwmDataPage({ graphList: [] }) cannot tell whether the caller means "there are no graphs" or "the planner should discover candidates itself". The same required parameter is now an admission filter in legacy paths, ignored in TTL meta, and a discovery-mode flag in TTL data.
Suggested direction
Make graph discovery/admission an explicit model instead of overloading graphList. The code should say whether the planner owns candidate discovery or is filtering a known inventory, so each helper has one clear contract.
For Agents
Look at sync-handler.ts SWM meta/data calls and graph-plan.ts readSwmMetaPage/readSwmDataPage/buildFreshSwmDataGraphPlan. Preserve the no-full-inventory TTL behavior, but replace the empty-array sentinel with an explicit typed input such as graphInventory?: readonly string[] or candidateSource: 'registered-prefixes' | 'known-inventory'. Keep tests proving TTL rootless requests do not call listGraphs.
| // META negotiates byte-budget paging on the wire (#1916/#1923). Durable | ||
| // meta is subject-atomic; SWM meta is reassembled and verified as one | ||
| // retained manifest. Both remain bounded by the common response serializer. | ||
| const usesMetaByteBudget = phase === 'meta' && |
There was a problem hiding this comment.
🟡 Issue: Centralize byte-budget response policy instead of reimplementing it for META
What's wrong
The PR expands byte-budget paging from DATA into META, but the responder implementation does not extend the existing policy boundary. Instead it threads a second ad-hoc policy through the already busy handler. That increases branching complexity and makes the transport contract depend on several scattered conditionals staying aligned.
Example
Adding another negotiated page mode or another phase now requires keeping resolveDataRequestPolicy, usesMetaByteBudget, metaLimit, SWM meta serialization, durable meta serialization, and cache-release flags in sync by hand.
Suggested direction
Promote the byte-budget decision into a single policy abstraction that covers both DATA and META, rather than adding parallel META booleans and ternaries in the handler.
For Agents
Start in packages/agent/src/sync/responder/data-request-policy.ts and generalize it into a responder page policy for DATA and META. Have it return normalized hint rows, serializer mode, cache-release behavior, and durable-meta oversized policy. Then make sync-handler.ts consume that policy instead of recomputing phase-specific booleans in multiple branches. Existing byte-budget page tests should continue to pass unchanged.
2 participants
User impact
A provider can serve large selected public context graphs without repeatedly scanning unrelated graphs or probing payload shapes that cannot contribute. Metadata is negotiated and paged by bytes, and selected-graph planning uses bounded subject and graph indexes.
Before
sequenceDiagram participant R as Receiver participant P as Provider participant S as Store R->>P: Catch up selected CG loop Metadata and data pages P->>S: Global graph discovery P->>S: Rootless payload probes P->>S: Rebuild broad plan end Note over R,P: Store work grows with unrelated local dataAfter
sequenceDiagram participant R as Receiver participant P as Provider participant I as Selected CG indexes participant S as Store R->>P: Catch up selected CG with byte budget P->>I: Resolve bounded subject and graph plan loop Byte-bounded pages P->>S: Read only planned selected graphs P-->>R: Bounded metadata or data page end Note over R,P: Work follows the selected CG, not the whole storeSafety
Validation
Focused byte-budget, metadata-ceiling, SWM-subgraph, and snapshot-cache suites cover the bounded path. The complete stack reached 500/500 SWM on Testnet without manual catch-up.