Priority: P2 retained-memory/lifecycle simplification. Source-confirmed lifetime gap; no measured OOM or cross-project disclosure is claimed.
Evidence
Reviewed #707 at a8c322a02ce7f5453dab0a3d89393b9b729f6b4d:
- snapshot_cache.rs:
DerivedSnapshotCache.scopes inserts an Arc slot for every new scope before computation. There is no removal, capacity, or retirement path; failed/cancelled first computations also leave empty slots.
- memory_service/projection.rs and similarity.rs hold this cache in process-global OnceLocks keyed by
state.mem_db_path.
Replacing a revision bounds retained revisions per scope, not scopes per process. Visiting more stores retains their latest derived JSON/fact payloads until process exit, even after the owning dashboard/project state disappears. Merely bounding output point count does not bound total retained cache memory.
Change
Prefer putting the existing cache slot on the actual retained store/dashboard owner so dropping/retiring that owner releases its derived state. If sharing across mounted views is required, reuse the existing registered store identity/lifetime rather than a process-global map keyed only by path. Otherwise implement bounded eviction plus exact retirement in this existing cache, not another cache manager.
Remove the static caches when their owner-local replacement is wired. Preserve revision, query/limit, authorization and single-flight semantics; an old Arc keeping bytes alive must not authorize a response under a replacement store. Prevent a late computation from reinstalling retired state. Do not add a persistent cache or make cache hits a freshness authority.
Acceptance
Repeatedly open, query and retire distinct stores, and verify cache-owned payloads/slots return to the declared bound. Failure/cancellation before first install does not accumulate empty scopes. An admitted reader may finish while retirement blocks future installation; a same-path replacement cannot reuse the retired owner's entry. Existing warm-hit/no-vector-read and unrelated-scope overlap behavior remains intact. Measure retained cache bytes separately from caller-held Arcs.
Related #815 fixes global lock serialization, not retained lifetime; its concurrency-test gap is reported there separately. This is not #976's semantic evaluation cache. No native/browser tests or benchmarks executed by this review. Target the #707 integration branch; keep #707 draft.
Priority: P2 retained-memory/lifecycle simplification. Source-confirmed lifetime gap; no measured OOM or cross-project disclosure is claimed.
Evidence
Reviewed #707 at
a8c322a02ce7f5453dab0a3d89393b9b729f6b4d:DerivedSnapshotCache.scopesinserts an Arc slot for every new scope before computation. There is no removal, capacity, or retirement path; failed/cancelled first computations also leave empty slots.state.mem_db_path.Replacing a revision bounds retained revisions per scope, not scopes per process. Visiting more stores retains their latest derived JSON/fact payloads until process exit, even after the owning dashboard/project state disappears. Merely bounding output point count does not bound total retained cache memory.
Change
Prefer putting the existing cache slot on the actual retained store/dashboard owner so dropping/retiring that owner releases its derived state. If sharing across mounted views is required, reuse the existing registered store identity/lifetime rather than a process-global map keyed only by path. Otherwise implement bounded eviction plus exact retirement in this existing cache, not another cache manager.
Remove the static caches when their owner-local replacement is wired. Preserve revision, query/limit, authorization and single-flight semantics; an old Arc keeping bytes alive must not authorize a response under a replacement store. Prevent a late computation from reinstalling retired state. Do not add a persistent cache or make cache hits a freshness authority.
Acceptance
Repeatedly open, query and retire distinct stores, and verify cache-owned payloads/slots return to the declared bound. Failure/cancellation before first install does not accumulate empty scopes. An admitted reader may finish while retirement blocks future installation; a same-path replacement cannot reuse the retired owner's entry. Existing warm-hit/no-vector-read and unrelated-scope overlap behavior remains intact. Measure retained cache bytes separately from caller-held Arcs.
Related #815 fixes global lock serialization, not retained lifetime; its concurrency-test gap is reported there separately. This is not #976's semantic evaluation cache. No native/browser tests or benchmarks executed by this review. Target the #707 integration branch; keep #707 draft.