Context
Raised by otReviewAgent on PR #1936 (packages/agent/src/sync/requester/durable-sync.ts, the discardedOnlyMetadataResponse check).
The durable requester decides whether a fully-discarded metadata-only page may still advance the meta checkpoint by summing reason-specific "consumed but not persisted" counters:
const deliberatelyDroppedMeta =
droppedSyncControlTriples + (processed.droppedNonIriSubjectTriples ?? 0);
The reviewer's point (valid): the requester's checkpoint predicate should depend on a single semantic "consumed unpersisted metadata" quantity, not enumerate every verifier discard reason. Each new deliberate-discard category otherwise requires a new optional field threaded through DurableBatchProcessResult → wire result → requester plus another addend here — a growing special-case sum.
Proposed follow-up
Introduce one worker-owned aggregate (e.g. consumedUnpersistedMetaTriples) on DurableBatchProcessResult, computed in processDurableBatch from the per-reason counts. The requester's discardedOnlyMetadataResponse compares that single field to totalFetchedMetaQuads (reason-agnostic). Keep the per-reason counters (droppedSyncControlTriples, droppedNonIriSubjectTriples) as diagnostics/logging only, not in the checkpoint contract.
Why deferred from #1921
#1921/PR #1936 is a security/integrity fix scoped to the two 🔴s (non-IRI subjects authenticating data + a test-coverage gap). Consolidating the checkpoint accounting changes the worker→requester contract and touches the already-shipped sync-control checkpoint semantics (droppedSyncControlTriples === totalFetchedMetaQuads), so it deserves its own review + regression tests rather than riding on a security patch. The current per-reason sum is correct and tested (sync-requester-progress.test.ts proves cursor-advance for all-non-IRI and mixed all-discarded pages).
Related: #1921, PR #1936. Sibling defer: #1940 (classifier unification).
Context
Raised by otReviewAgent on PR #1936 (
packages/agent/src/sync/requester/durable-sync.ts, thediscardedOnlyMetadataResponsecheck).The durable requester decides whether a fully-discarded metadata-only page may still advance the meta checkpoint by summing reason-specific "consumed but not persisted" counters:
The reviewer's point (valid): the requester's checkpoint predicate should depend on a single semantic "consumed unpersisted metadata" quantity, not enumerate every verifier discard reason. Each new deliberate-discard category otherwise requires a new optional field threaded through
DurableBatchProcessResult→ wire result → requester plus another addend here — a growing special-case sum.Proposed follow-up
Introduce one worker-owned aggregate (e.g.
consumedUnpersistedMetaTriples) onDurableBatchProcessResult, computed inprocessDurableBatchfrom the per-reason counts. The requester'sdiscardedOnlyMetadataResponsecompares that single field tototalFetchedMetaQuads(reason-agnostic). Keep the per-reason counters (droppedSyncControlTriples,droppedNonIriSubjectTriples) as diagnostics/logging only, not in the checkpoint contract.Why deferred from #1921
#1921/PR #1936 is a security/integrity fix scoped to the two 🔴s (non-IRI subjects authenticating data + a test-coverage gap). Consolidating the checkpoint accounting changes the worker→requester contract and touches the already-shipped sync-control checkpoint semantics (
droppedSyncControlTriples === totalFetchedMetaQuads), so it deserves its own review + regression tests rather than riding on a security patch. The current per-reason sum is correct and tested (sync-requester-progress.test.tsproves cursor-advance for all-non-IRI and mixed all-discarded pages).Related: #1921, PR #1936. Sibling defer: #1940 (classifier unification).