fix(consolidation): dedup against live memories within a run (#747)

[MarvinFS]

What

Fixes the within-run duplication and orphaning in mem::consolidate reported in #747.

The 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 (existingMemories.find(...)) therefore only ever saw the stale snapshot, so when two concept groups in the same run synthesized the same title, the second group did not find the memory the first had just created. It took the create branch and wrote a second isLatest memory with no parentId - a duplicate active memory plus an orphaned predecessor and a broken version chain.

Why it happens

Each concept group is its own provider.compress call, and distinct concepts frequently compress to the same synthesized title. The snapshot is read once; new memories are persisted via kv.set but never added back to the in-memory array the dedup scans.

The fix

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 a plain append would leave the now-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 a scoped run still refuses to evolve another project's identically titled memory (the existing consolidate-project-scope tests pass unchanged).

How to verify

npm install --legacy-peer-deps
npm run build
npm test -- test/consolidate-project-scope.test.ts

Two regression tests were added to test/consolidate-project-scope.test.ts:

• two concept groups -> same title in one run: asserts exactly one active memory, one parentless root, intact parentId/supersedes chain, no orphan.
• three concept groups -> same title: asserts still exactly one active memory in a linear three-version chain (guards the in-place-replace subtlety).

Both are red on main (the 2-group case yields 2 active memories / 2 parentless roots, the 3-group case yields 3) and green with this change.

Fixes #747

Summary by CodeRabbit

• Bug Fixes

  • Fixed an issue where the consolidation process could generate duplicate "latest" memories when multiple concept groups in the same run resolved to the same synthesized title; consolidation now preserves a single active memory and correct parent/supersedes relationships.

• Tests

  • Added a test suite validating same-title deduplication within a run, ensuring one active root and a correct linear evolution across multiple concept groups.

[vercel]

@MarvinFS is attempting to deploy a commit to the rohitg00's projects Team on Vercel.

A member of the Team first needs to authorize it.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d40d6d51-d972-4b19-a742-be877cfb32f9

📥 Commits

Reviewing files that changed from the base of the PR and between ce3d185 and 775df5f.

📒 Files selected for processing (1)

• src/functions/consolidate.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/functions/consolidate.ts

---

📝 Walkthrough

Walkthrough

This PR fixes issue #747 by replacing a stale titles Set with live snapshot mutation in the consolidation loop. When multiple concept groups produce the same memory title within one run, the system now evolves the prior memory instead of creating duplicates, maintaining correct parent-supersedes chains and a single active memory per title.

Changes

Same-title memory consolidation fix

Layer / File(s)	Summary
Live snapshot memory updates src/functions/consolidate.ts	Consolidation removes reliance on the stale existingTitles Set and instead keeps existingMemories fresh by replacing evolved memories in-place and appending newly created memories, enabling subsequent concept groups in the same run to match and evolve rather than duplicate.
Same-title deduplication test suite test/consolidate-project-scope.test.ts	New test suite with seedGroup helper validates that multiple concept groups generating identical synthesized titles within one run produce exactly one active memory, one root, and a linear parent-successor chain, tested across two and three group collisions.

Sequence Diagram(s)

sequenceDiagram
  participant ConceptGroup
  participant Consolidate
  participant existingMemories
  ConceptGroup->>Consolidate: provide observation group (synthesized title)
  Consolidate->>existingMemories: search for match by title
  alt match found
    Consolidate->>Consolidate: evolve matched memory -> evolved
    Consolidate->>existingMemories: reflectMemoryInSnapshot(replaced by evolved)
  else no match
    Consolidate->>Consolidate: create new memory
    Consolidate->>existingMemories: reflectMemoryInSnapshot(appended new)
  end
  Consolidate->>ConceptGroup: continue with next group

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• rohitg00/agentmemory#662: Related consolidation changes; both modify how existing memories are matched/evolved (project-scoped constraints vs in-run snapshot updates).

Suggested reviewers

• rohitg00

Poem

🐰 A snapshot kept fresh, no titles left stale,
When concepts collide, their memories prevail,
From two groups to three, the chain holds so true—
One memory's legacy, evolved through and through!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly describes the main fix: deduplication against live memories within a run, matching the core problem and solution outlined in the changeset.
Linked Issues check	✅ Passed	The PR fully addresses all coding requirements from issue #747: maintaining live existingMemories snapshots, removing unused existingTitles, preventing duplicate active memories, preserving parent chains, maintaining project scoping, and adding regression tests.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the within-run deduplication bug: consolidate.ts refactoring and test additions for regression coverage; no unrelated modifications present.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

src/functions/consolidate.ts (1)

202-210: ⚡ Quick win

Remove WHAT-style inline comments and encode intent in code structure.

Line 202–210 and Line 228–231 add explanatory WHAT comments in src/, which violates the repository rule. Prefer extracting these blocks into clearly named helpers so intent is self-describing without inline narrative comments.

As per coding guidelines, src/**/*.{ts,js}: "No code comments explaining WHAT — use clear naming instead".

Also applies to: 228-231

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/functions/consolidate.ts` around lines 202 - 210, The inline WHAT-style
comment above the in-memory snapshot update should be removed and the intent
encoded in a clearly named helper; extract the replace-or-append logic into a
function (for example updateInMemorySnapshot(existingMemories, existingMatch,
evolved) or mergeEvolvedMemoryIntoSnapshot) and call it from consolidate so the
code reads self-descriptively (use the same helper pattern for the similar block
at 228–231). Ensure the new helper contains the existing index/replace-or-push
logic operating on existingMemories and is used in place of the comment+inline
code so no explanatory WHAT comments remain in src.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/functions/consolidate.ts`:
- Around line 202-210: The inline WHAT-style comment above the in-memory
snapshot update should be removed and the intent encoded in a clearly named
helper; extract the replace-or-append logic into a function (for example
updateInMemorySnapshot(existingMemories, existingMatch, evolved) or
mergeEvolvedMemoryIntoSnapshot) and call it from consolidate so the code reads
self-descriptively (use the same helper pattern for the similar block at
228–231). Ensure the new helper contains the existing index/replace-or-push
logic operating on existingMemories and is used in place of the comment+inline
code so no explanatory WHAT comments remain in src.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4f47a6b0-7b5d-4278-a6f6-30977143148a

📥 Commits

Reviewing files that changed from the base of the PR and between fd9e3bd and ce3d185.

📒 Files selected for processing (2)

• src/functions/consolidate.ts
• test/consolidate-project-scope.test.ts