feat(agent-memory): Phase 6 — consolidate()#254
Conversation
6e7c2e9 to
2dc4339
Compare
5cdeb4c to
c317978
Compare
2dc4339 to
e4646ec
Compare
d5a95b9 to
e27477f
Compare
KIvanow
left a comment
There was a problem hiding this comment.
Phase 6 reads well: write-before-delete is genuinely safe, the mass-delete guard mirrors forgetByScope, and keeping summarize injected is the right call. Two things I would like addressed before merging:
1. Push olderThanSeconds and maxImportance into the query instead of filtering after the scan. Right now the FT.SEARCH uses only buildScopeFilter, fetches up to CONSOLIDATE_SCAN_LIMIT in index order, and then isConsolidationCandidate filters in JS. Both fields are indexed NUMERIC, so they can go straight into the filter as @created_at:[-inf (cutoff] and @importance:[-inf max]. As written, a scope larger than the scan limit returns an arbitrary first window and filters that, so matching old or low-importance candidates beyond the window are never selected even though a range query would find them. It also pulls content for rows that are then discarded. Pushing the predicates down makes the cap apply to actual matches and trims the transfer.
2. The scan does not exclude prior summaries, so default runs fold summaries into summaries. Because there is no @source:{summary} exclusion, a default consolidate({ summarize, threadId }) with no maxImportance re-selects the previous summary (importance 0.7) and deletes it into a new one. A maxImportance below 0.7 happens to protect them, but only implicitly. If rolling re-consolidation is intended, it is worth documenting; if not, the candidate scan should exclude @source:{summary}.
a820313 to
ad379d5
Compare
e27477f to
6864f6b
Compare
|
@KIvanow Addressed in 6864f6b: (1) |
ad379d5 to
7ee56df
Compare
6864f6b to
3ee71c8
Compare
KIvanow
left a comment
There was a problem hiding this comment.
Thanks, both points are addressed: the NUMERIC range clauses (@created_at:[-inf ...], @importance:[-inf ...]) are faithful to the old isConsolidationCandidate semantics, candidates now come straight from the response, and -@source:{summary} stops the default re-folding. I also confirmed it never degenerates into a pure-negation query (the criteria guard always contributes a positive clause) and joinClauses never returns * here, so the mass-delete guard stays intact.
One small wording fix before I approve: the commit message and the code comment both say prior summaries can be re-consolidated by "targeting them explicitly," but MemoryStore always passes excludeSource: SUMMARY_SOURCE, so -@source:{summary} is unconditional and there is no exposed way to include summaries. The behavior is fine (unconditional exclusion fully resolves the accidental-folding concern), but the comment promises an option that does not exist. Please either drop the "target them explicitly" phrasing or actually expose the toggle.
3ee71c8 to
1b71d3e
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1b71d3e. Configure here.
KIvanow
left a comment
There was a problem hiding this comment.
Verified at 384e10b. The outstanding wording fix is resolved: the "target them explicitly" phrasing is gone from the code comment, commit message, and PR description, and the consolidate comment now correctly states prior summaries are always excluded via -@source:{summary} with no toggle implied. The earlier predicate push-down (NUMERIC range clauses for olderThanSeconds/maxImportance) and the summary exclusion both hold.
384e10b is a good catch on top: writing the summary through remember() let enforceCapacity evict the just-written summary while the sources still inflated the scope, then the sources were deleted, losing the consolidated content. Routing consolidate through the new capacity-free writeMemory() (shared with remember(), which keeps its best-effort enforceCapacity) fixes it, and the reasoning holds since consolidation is a net reduction so the cap stays honored without a pass. Write-before-delete ordering is preserved.
Non-blocking minor: there does not appear to be a test covering the capacity-free consolidate path (consolidate with maxItemsPerScope set should keep the summary rather than drop it). Worth adding to lock in the 384e10b behavior, but not a blocker.
Approving.
7ee56df to
2c6482d
Compare
- consolidate() summarizes old/low-importance memories into one durable source:'summary' memory via a caller-supplied summarize(items) callback - Select candidates by scope/tags + olderThanSeconds + maxImportance; write the summary before deleting sources so a failure never destroys memories without leaving the replacement - deleteSources defaults true; summaryImportance defaults 0.7 - Require at least one selection criterion to prevent whole-store consolidation (mirrors forgetByScope's mass-delete guard) - Fetch only the fields parseMemoryItem needs (no vector blobs) on the candidate scan - Add ConsolidateOptions/ConsolidateResult types
…p summaries - olderThanSeconds/maxImportance are now NUMERIC range clauses in the FT.SEARCH filter, so the scan limit applies to actual matches instead of an arbitrary first window, and content isn't transferred for discarded rows - the candidate scan always excludes @source:{summary}, so a run never re-folds prior summaries into a new summary - extract buildConsolidateFilter (shares scope-clause logic with buildScopeFilter)
With maxItemsPerScope set, consolidate wrote the summary via remember(), whose enforceCapacity could evict that very summary while the sources still inflated the scope — then the sources were deleted, losing the consolidated content entirely. Write the summary via a capacity-free path; consolidation is a net reduction, so the cap stays honored without a pass.
384e10b to
f834211
Compare

Stacked on #253 (Phase 5 — eviction & TTL).
What
Phase 6 of
@betterdb/agent-memory: consolidation — summarize many old/low-value memories into fewer durable ones. Callable, not scheduled (per spec).consolidate(options):threadId/agentId/namespace+tags) filtered byolderThanSecondsandmaxImportance.summarize(items) => Promise<string>(the library never hard-codes an LLM — mirrors the judge-injection pattern).source: 'summary'atsummaryImportance(default0.7), scoped/tagged to the request.deleteSources(defaulttrue) removes the originals; returns{ consolidated, created, deleted }.Design / review notes
olderThanSeconds, ormaxImportance) — a criteria-lessconsolidate({summarize})would otherwise summarize+delete the entire store. MirrorsforgetByScope's guard.RETURNof only the fieldsparseMemoryItemneeds, so it never transfers vector blobs.CONSOLIDATE_SCAN_LIMIT); a singlesummarize()over a whole huge scope is impractical anyway, and the remainder consolidates on subsequent calls.Tests (
MemoryStore.consolidate.test.ts, 8)candidate summarize+write+delete+counts ·
olderThanSecondsfilter ·maxImportancefilter · summary scope+summaryImportance·deleteSources:falsekeeps sources · nothing-matches → zeros & no summarize/write · no-criteria guard throws · defaults (0.7/ delete).55/55 package tests green ·
tsc --noEmitclean · prettier clean.Note
Medium Risk
Bulk delete and summary writes touch persistent agent memory; safeguards (criteria guard, write-before-delete, summary exclusion) reduce data-loss risk but misconfigured callers could still remove large candidate sets.
Overview
Adds
MemoryStore.consolidate()so callers can merge many scoped, low-value or old memories into one durable summary via an injectedsummarize(items)callback (no built-in LLM).Candidates are selected with
FT.SEARCHusing a newbuildConsolidateFilter(scope/tags plus server-sidecreated_at/importancebounds, and-@source:{summary}so prior summaries are not re-folded). The flow writes the summary first (source: 'summary', default importance 0.7), optionally deletes sources (default on), and returns{ consolidated, created, deleted }.rememberis refactored through a privatewriteMemorypath so consolidation skipsenforceCapacityand cannot evict the new summary while sources still count toward the cap. A criteria guard blocks unscoped whole-store runs.Public
ConsolidateOptions/ConsolidateResultare exported; tests cover filters, defaults,deleteSources: false, empty matches, and the capacity bypass.Reviewed by Cursor Bugbot for commit f834211. Bugbot is set up for automated code reviews on this repo. Configure here.