fix(consolidation): dedup against live memories within a run (#747)#9
Merged
Conversation
The mem::consolidate handler snapshots existing memories into `existingMemories` once before the concept-group loop and never refreshes it. A parallel `existingTitles` Set was populated on every create/evolve but never read - dead code. The dedup check therefore scanned only the stale snapshot, so when two concept groups in the same run synthesized the same title, the second group failed to find the memory the first had just created. It fell into the create branch and produced a second isLatest memory with no parent: a duplicate plus an orphaned predecessor, and a broken version chain. Keep the snapshot live instead of carrying an unused index: - Remove the dead `existingTitles` Set. - Push each newly created memory into `existingMemories`. - On evolve, replace the matched predecessor in place. The title match does not filter on isLatest, so appending would leave the superseded row matchable and let a third same-title group evolve it, recreating a second latest; in-place replacement keeps exactly one matchable row per title and a linear A -> B -> C chain. The cross-project guard in the match predicate is untouched, so scoped runs still refuse to evolve another project's identically titled memory. Adds regression tests to test/consolidate-project-scope.test.ts: a two-group collision (asserts one active memory, intact parent chain, no orphan) and a three-group collision (asserts one active memory, linear three-version chain). Both fail on current main and pass with this fix. Signed-off-by: MarvinFS <MarvinFS@users.noreply.github.com>
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
1 participant
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.
What
Ports the upstream #747 fix (rohitg00/agentmemory PR #749) to this LanceDB-backed fork. The
consolidate.tsfile here is byte-identical to upstream, so the same defect and the same patch apply.The
mem::consolidatehandler snapshots existing memories intoexistingMemoriesonce before the concept-group loop and never refreshes it; the parallelexistingTitlesSet is written but never read (dead code). The dedup.find()only sees the stale snapshot, so a second concept group that synthesizes the same title in the same run misses the just-created memory, takes the create branch, and writes a duplicateisLatestmemory with noparentId- a duplicate plus an orphaned predecessor and a broken version chain.The fix
existingTitlesSet.existingMemories.isLatest, so a plain append would let a third same-title group re-match the superseded row and recreate a second latest). This keeps exactly one matchable row per title and a linearA -> B -> Cchain.Cross-project guard untouched.
Verify
Two regression tests added (two-group and three-group same-title collisions), red on the pristine file and green with the fix. Full local suite: 1281 passed (+2 vs baseline), with the same 9 pre-existing Windows-environmental failures unchanged - consolidation is not among them.
Refs #747, upstream PR rohitg00/agentmemory#749.