fix(store): record an oplog entry for every auto-pruned insight - #96
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR makes auto-pruning observable by recording an oplog entry for each insight that AutoPrune soft-deletes, closing a gap where this destructive mutation path previously left no audit/history trail.
Changes:
- Add
db.LogOp("prune", ...)per successfully pruned insight duringautoPrune. - Add a regression test to assert that each auto-pruned insight produces a corresponding
pruneoplog entry and that the logged IDs reflect pruned insights.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/memory/store/node.go | Writes an oplog entry for each insight pruned by auto-prune, including capacity context in detail. |
| internal/memory/store/store_test.go | Adds a regression test covering per-pruned-ID oplog logging behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Auto-prune soft-deletes insights and hard-deletes their edges, but was the only mutation path that wrote nothing to the oplog. Every other mutation (remember, forget, link, diff-replace, diff-skip, embed) records one, so the oplog reads as a complete history when it is not: a store that has shed insights offers no way to discover which ones, when, or why. This is easy to hit. AutoPrune runs on every remember once the active count exceeds MaxInsights (default 1000), pruning up to PruneBatchSize per call. A store that has been over capacity for a while can quietly lose thousands of insights, and the loss is only visible as a count that stopped growing. Log one entry per pruned id, with the active count and the cap that triggered it, so the oplog can be used as a recovery index. No behavior change beyond the added log line.
audreyt
force-pushed
the
fix/prune-audit-log
branch
from
August 12, 2026 03:06
b4fa165 to
a4faa72
Compare
Grivn
approved these changes
Aug 12, 2026
Grivn
left a comment
Member
There was a problem hiding this comment.
LGTM — thanks for closing this audit gap!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Auto-prune soft-deletes insights and hard-deletes their edges, but is the only mutation path that writes nothing to the oplog. Every other mutation — remember, forget, link, diff-replace, diff-skip, embed — records one, so the oplog reads as a complete history when it isn't.
This is easy to hit.
AutoPruneruns on every remember once the active count exceedsMaxInsights(default 1000), pruning up toPruneBatchSizeper call. A store that has been over capacity for a while can quietly shed thousands of insights, and the only visible symptom is a count that stopped growing. There is currently no way to discover which ones went, when, or why.Log one entry per pruned id, with the active count and the cap that triggered it, so the oplog can serve as a recovery index.
Blast radius: none beyond the added log line. No change to what gets pruned or when.
Test:
TestAutoPrune_RecordsOplogEntryPerPrunedInsight— fails without the change withwant 2 prune oplog entries, got 0. It also asserts every id the oplog names is genuinely soft-deleted, so the log can be trusted as an index.