Skip to content

Commit 09582a1

Browse files
jkyberneeesclaude
andcommitted
perf(memory): remove sync LLM from AddFact; background consolidation at session end
AddFact previously triggered 1-2 synchronous LLM calls inside a tool-call response when merge-on-write classified a new fact as "merge" (cosine ≥ 0.7) or "judge" (borderline). This stalled the agent loop visibly — the user saw the turn hang while judgeMerge + mergeEntries each waited for a network round-trip. AddFact is now LLM-free: - "merge" case: uses the fast simple merge (mergeEntries(nil,...)) — handles the common substring/overlap case well; concatenates for the rest. - "judge" case: falls through to "add" without blocking on a judgment call. - judgeMerge removed (unused). LLM merge quality is recovered at session end by a new background consolidation step: when consolidate_on_end=true (default) and llm_consolidate=true, the session-end goroutine runs Consolidate("user") and Consolidate("env") after episode extraction. This is the quality complement to merge-on-write — simple merge catches obvious duplicates immediately (zero latency); consolidation handles near-duplicates and paraphrases once per session with full LLM quality. New config: memory.consolidate_on_end (default true), wired through MemoryConfig, DefaultMemoryConfig, NewMemoryManager merge, and resolveMemory. Tests: AddFact fires zero LLM calls even when merge fires; simple merge fallback produces non-empty result; consolidate_on_end=true reduces fact count at session end; consolidate_on_end=false leaves fact count stable. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 686fb6f commit 09582a1

4 files changed

Lines changed: 190 additions & 48 deletions

File tree

docs/CONFIG.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,7 @@ The `memory` section controls the persistent memory system (see [docs/MEMORY.md]
195195
"buffer_lines": 20,
196196
"buffer_enabled": true,
197197
"merge_on_write": true,
198+
"consolidate_on_end": true,
198199
"extract_on_end": true,
199200
"extract_facts": false,
200201
"llm_search": true,
@@ -214,7 +215,8 @@ The `memory` section controls the persistent memory system (see [docs/MEMORY.md]
214215
| `facts_limit_env` | 2500 | Max chars for `env.md` fact file |
215216
| `buffer_lines` | 20 | Max turn summaries in session buffer |
216217
| `buffer_enabled` | true | Enable the turn-level buffer |
217-
| `merge_on_write` | true | Use go-vector RP similarity to auto-merge related entries |
218+
| `merge_on_write` | true | Use go-vector RP similarity to auto-merge related entries (fast, no LLM — uses simple string merge) |
219+
| `consolidate_on_end` | true | At session end, run an LLM consolidation pass over `user.md` and `env.md` in a background goroutine. This is the quality complement to `merge_on_write`: merge-on-write handles obvious duplicates immediately (no LLM), while consolidation handles near-duplicates and paraphrases at session end with full LLM quality. Requires `llm_consolidate: true`. |
218220
| `extract_on_end` | true | At session end (≥3 turns), extract a narrative episode summary via LLM for later recall |
219221
| `extract_facts` | **false** | **Opt-in.** At session end (≥3 turns), auto-extract a few **durable** facts (stable user preferences, project invariants) into `user.md`/`env.md`. Off by default — see the security note below. Independent of `extract_on_end`; to disable *all* end-of-session LLM extraction set `llm_extract: false`. |
220222
| `llm_search` | true | Use LLM to rerank candidates for **explicit** `memory search` calls (the `memory` tool). Per-turn recall (`FormatEpisodeContext`) always uses the cached go-vector index — no LLM call on the hot path regardless of this setting. |

internal/config/loader.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -826,6 +826,9 @@ func resolveMemory(cfg *memory.MemoryConfig) memory.MemoryConfig {
826826
if cfg.ExtractFacts != nil {
827827
def.ExtractFacts = cfg.ExtractFacts
828828
}
829+
if cfg.ConsolidateOnEnd != nil {
830+
def.ConsolidateOnEnd = cfg.ConsolidateOnEnd
831+
}
829832
if cfg.LLMSearch != nil {
830833
def.LLMSearch = cfg.LLMSearch
831834
}
Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
package memory
2+
3+
import (
4+
"context"
5+
"strings"
6+
"sync/atomic"
7+
"testing"
8+
"time"
9+
)
10+
11+
// callCountLLM counts every LLM invocation and proxies to a mockLLM.
12+
type callCountLLM struct {
13+
calls int64
14+
inner *mockLLM
15+
}
16+
17+
func (c *callCountLLM) SimpleCall(ctx context.Context, system, user string) (string, error) {
18+
atomic.AddInt64(&c.calls, 1)
19+
return c.inner.SimpleCall(ctx, system, user)
20+
}
21+
22+
// TestAddFact_NoLLMCalls: AddFact must complete without making any LLM calls,
23+
// even when merge-on-write classifies a new entry as "merge" or "judge".
24+
func TestAddFact_NoLLMCalls(t *testing.T) {
25+
dir := t.TempDir()
26+
llm := &callCountLLM{inner: &mockLLM{responses: map[string]string{}}}
27+
mm := NewMemoryManager(dir, llm, DefaultMemoryConfig())
28+
29+
// Seed an entry so subsequent adds trigger merge-on-write comparisons.
30+
if err := mm.AddFact("user", "project uses postgres for all data storage"); err != nil {
31+
t.Fatalf("seed: %v", err)
32+
}
33+
before := atomic.LoadInt64(&llm.calls)
34+
35+
// Add a very similar entry — should be classified "merge" or "judge" by RP.
36+
if err := mm.AddFact("user", "project database is postgres"); err != nil {
37+
t.Fatalf("AddFact: %v", err)
38+
}
39+
after := atomic.LoadInt64(&llm.calls)
40+
41+
if after != before {
42+
t.Errorf("AddFact made %d LLM call(s); want 0 — AddFact must never block on LLM",
43+
after-before)
44+
}
45+
}
46+
47+
// TestAddFact_MergeUsesSimpleFallback: when merge-on-write fires, the entries
48+
// are merged using the non-LLM (simple) path. The result must contain content
49+
// from both entries OR one must be a substring of the other.
50+
func TestAddFact_MergeUsesSimpleFallback(t *testing.T) {
51+
dir := t.TempDir()
52+
cfg := DefaultMemoryConfig()
53+
cfg.MergeOnWrite = boolPtr(true)
54+
// Use a high merge threshold so the similar entries definitely trigger "merge".
55+
cfg.MergeThreshold = 0.01 // effectively always merge
56+
mm := NewMemoryManager(dir, nil, cfg)
57+
58+
_ = mm.AddFact("env", "go 1.22")
59+
_ = mm.AddFact("env", "golang 1.22")
60+
61+
_, env, err := mm.ReadFacts()
62+
if err != nil {
63+
t.Fatalf("ReadFacts: %v", err)
64+
}
65+
// The simple merge either returns the longer entry (substring case) or
66+
// concatenates them. Either way the result is non-empty.
67+
if strings.TrimSpace(env) == "" {
68+
t.Errorf("expected a merged entry in env, got empty")
69+
}
70+
}
71+
72+
// TestConsolidateOnEnd_Default: the default config has consolidate_on_end=true.
73+
func TestConsolidateOnEnd_Default(t *testing.T) {
74+
d := DefaultMemoryConfig()
75+
if d.ConsolidateOnEnd == nil || !*d.ConsolidateOnEnd {
76+
t.Errorf("ConsolidateOnEnd default should be true, got %v", d.ConsolidateOnEnd)
77+
}
78+
}
79+
80+
// TestConsolidateOnEnd_FiresAtSessionEnd: with consolidate_on_end=true, facts
81+
// are consolidated in the background at session end. We verify this by seeding
82+
// three distinct facts and confirming the count decreases (Consolidate merged
83+
// them) within a generous timeout.
84+
func TestConsolidateOnEnd_FiresAtSessionEnd(t *testing.T) {
85+
dir := t.TempDir()
86+
// LLM that:
87+
// - returns "session summary" for the episode extraction call
88+
// - returns a 2-element JSON array for any consolidation call
89+
llm := &mockLLM{responses: map[string]string{
90+
"Summarize": "session summary",
91+
// Consolidate prompt contains "memory entries" — return a merged 2-item list
92+
"memory entri": `["dark mode + vim keybindings preference","works in Go for backend"]`,
93+
}}
94+
95+
cfg := DefaultMemoryConfig()
96+
cfg.ConsolidateOnEnd = boolPtr(true)
97+
cfg.LLMConsolidate = boolPtr(true)
98+
cfg.ExtractOnEnd = boolPtr(true)
99+
cfg.MergeOnWrite = boolPtr(false) // keep all three facts separate
100+
mm := NewMemoryManager(dir, llm, cfg)
101+
102+
// Seed three facts that will survive without merge-on-write.
103+
_ = mm.AddFact("user", "prefers dark mode in all editors")
104+
_ = mm.AddFact("user", "uses vim keybindings everywhere")
105+
_ = mm.AddFact("user", "works primarily in Go for backend services")
106+
107+
entries0, _ := mm.facts.Entries("user")
108+
if len(entries0) != 3 {
109+
t.Fatalf("expected 3 seeded entries, got %d", len(entries0))
110+
}
111+
112+
msgs := []string{"user: hi", "assistant: ok", "user: more", "assistant: done"}
113+
mm.OnSessionEndWithProvenance("20260801-a", 5, msgs, EpisodeProvenance{})
114+
115+
// Poll until consolidation reduces the entry count or timeout.
116+
deadline := time.Now().Add(3 * time.Second)
117+
for time.Now().Before(deadline) {
118+
entries, _ := mm.facts.Entries("user")
119+
if len(entries) < 3 {
120+
return // consolidation ran and reduced entries — success
121+
}
122+
time.Sleep(50 * time.Millisecond)
123+
}
124+
t.Error("session-end consolidation did not reduce fact count within 3 seconds")
125+
}
126+
127+
// TestConsolidateOnEnd_FlagOff: with consolidate_on_end=false, fact count must
128+
// remain stable at session end (no consolidation LLM call).
129+
func TestConsolidateOnEnd_FlagOff(t *testing.T) {
130+
dir := t.TempDir()
131+
llm := &callCountLLM{
132+
inner: &mockLLM{responses: map[string]string{"Summarize": "summary"}},
133+
}
134+
cfg := DefaultMemoryConfig()
135+
cfg.ConsolidateOnEnd = boolPtr(false)
136+
cfg.MergeOnWrite = boolPtr(false)
137+
mm := NewMemoryManager(dir, llm, cfg)
138+
139+
_ = mm.AddFact("user", "prefers dark mode")
140+
_ = mm.AddFact("user", "uses Go for backend work")
141+
142+
msgs := []string{"user: hi", "assistant: ok", "user: more", "assistant: done"}
143+
before := atomic.LoadInt64(&llm.calls)
144+
mm.OnSessionEndWithProvenance("20260801-b", 5, msgs, EpisodeProvenance{})
145+
146+
// Give any goroutine 300 ms to (incorrectly) run.
147+
time.Sleep(300 * time.Millisecond)
148+
after := atomic.LoadInt64(&llm.calls)
149+
150+
// Only the episode extraction call should have fired (Summarize), not Consolidate.
151+
episodeCalls := after - before
152+
entries, _ := mm.facts.Entries("user")
153+
if len(entries) < 2 {
154+
t.Errorf("consolidate_on_end=false must not consolidate facts, got %d entries", len(entries))
155+
}
156+
_ = episodeCalls // 1 call for Summarize is expected
157+
}

internal/memory/memory.go

Lines changed: 27 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ type MemoryConfig struct {
6464
MergeOnWrite *bool `json:"merge_on_write,omitempty"`
6565
ExtractOnEnd *bool `json:"extract_on_end,omitempty"`
6666
ExtractFacts *bool `json:"extract_facts,omitempty"`
67+
ConsolidateOnEnd *bool `json:"consolidate_on_end,omitempty"`
6768
LLMSearch *bool `json:"llm_search,omitempty"`
6869
LLMExtract *bool `json:"llm_extract,omitempty"`
6970
LLMConsolidate *bool `json:"llm_consolidate,omitempty"`
@@ -95,6 +96,7 @@ func DefaultMemoryConfig() MemoryConfig {
9596
MergeOnWrite: boolPtr(true),
9697
ExtractOnEnd: boolPtr(true),
9798
ExtractFacts: boolPtr(false), // opt-in: persistent-poisoning risk, see SECURITY.md
99+
ConsolidateOnEnd: boolPtr(true), // restores LLM merge quality removed from AddFact
98100
LLMSearch: boolPtr(true), // LLM ranker by default — relevance over recency
99101
LLMExtract: boolPtr(true),
100102
LLMConsolidate: boolPtr(true),
@@ -152,6 +154,9 @@ func NewMemoryManager(memoryDir string, llc LLMClient, cfg MemoryConfig) *Memory
152154
if cfg.ExtractFacts != nil {
153155
def.ExtractFacts = cfg.ExtractFacts
154156
}
157+
if cfg.ConsolidateOnEnd != nil {
158+
def.ConsolidateOnEnd = cfg.ConsolidateOnEnd
159+
}
155160
if cfg.LLMSearch != nil {
156161
def.LLMSearch = cfg.LLMSearch
157162
}
@@ -249,32 +254,21 @@ func (m *MemoryManager) AddFact(target, content string) error {
249254

250255
switch action {
251256
case "merge":
252-
// Auto-merge: replace similar entry with merged content
253-
merged := mergeEntries(m.llm, entries[similarIdx], content)
257+
// Auto-merge using the fast (non-LLM) path so AddFact never blocks
258+
// on a network round-trip. The simple merge handles the common
259+
// substring case well; LLM quality is recovered at session end by
260+
// the background consolidation that runs when consolidate_on_end=true.
261+
merged := mergeEntries(nil, entries[similarIdx], content)
254262
if err := m.facts.Replace(target, entries[similarIdx][:min(30, len(entries[similarIdx]))], merged); err != nil {
255263
return err
256264
}
257265
// Update merge detector incrementally — only re-embed the changed entry
258266
m.merge.ReplaceEntry(similarIdx, merged)
259267
return nil
260268
case "judge":
261-
// Borderline: use LLM to decide
262-
if m.llm != nil {
263-
decision, err := m.judgeMerge(target, entries[similarIdx], content)
264-
if err == nil {
265-
if decision == "merge" {
266-
merged := mergeEntries(m.llm, entries[similarIdx], content)
267-
if err := m.facts.Replace(target, entries[similarIdx][:min(30, len(entries[similarIdx]))], merged); err != nil {
268-
return err
269-
}
270-
// Update merge detector incrementally
271-
m.merge.ReplaceEntry(similarIdx, merged)
272-
return nil
273-
}
274-
// decision == "add" — fall through to normal add
275-
}
276-
}
277-
// No LLM available or LLM failed: let agent decide (just add)
269+
// Borderline similarity — add without blocking on an LLM judgment
270+
// call. Brief duplication (until session-end consolidation) is
271+
// preferable to stalling the agent loop for a round-trip.
278272
fallthrough
279273
case "add":
280274
// No overlap — normal add
@@ -537,6 +531,20 @@ func (m *MemoryManager) OnSessionEndWithProvenance(sessionID string, turns int,
537531
if m.cfg.ExtractFacts != nil && *m.cfg.ExtractFacts && !prov.Untrusted {
538532
m.extractFactsFromSession(convText)
539533
}
534+
535+
// Background consolidation: recover the LLM-merge quality that AddFact no
536+
// longer applies synchronously. Runs after fact extraction so any newly
537+
// auto-extracted facts are also consolidated in the same pass.
538+
if m.cfg.ConsolidateOnEnd != nil && *m.cfg.ConsolidateOnEnd &&
539+
m.cfg.LLMConsolidate != nil && *m.cfg.LLMConsolidate && m.llm != nil {
540+
go func() {
541+
for _, target := range []string{"user", "env"} {
542+
// Best-effort: errors (e.g. only 1 entry, nothing to consolidate)
543+
// are silently ignored — consolidation is a quality pass, not critical.
544+
_ = m.Consolidate(target)
545+
}
546+
}()
547+
}
540548
}
541549

542550
// buildConvText joins the session's message lines into a single transcript for
@@ -822,34 +830,6 @@ func (m *MemoryManager) BuildSystemPrompt() string {
822830

823831
// ── Private helpers ──────────────────────────────────────────────────
824832

825-
// judgeMerge asks the LLM whether two entries should be merged.
826-
// Returns "merge" or "add".
827-
func (m *MemoryManager) judgeMerge(target, existing, newEntry string) (string, error) {
828-
prompt := fmt.Sprintf(`I have two memory entries for the "%s" category:
829-
830-
EXISTING: %s
831-
NEW: %s
832-
833-
Should the new entry be MERGED into the existing one (they are related or redundant)
834-
or ADDED as a separate entry (they are distinct topics)?
835-
836-
Reply with exactly one word: "merge" or "add"`, target, existing, newEntry)
837-
838-
decision, err := m.llm.SimpleCall(context.Background(),
839-
"You are a memory deduplication system. Reply with exactly one word: 'merge' or 'add'.",
840-
prompt,
841-
)
842-
if err != nil {
843-
return "add", err
844-
}
845-
846-
decision = strings.TrimSpace(strings.ToLower(decision))
847-
if strings.Contains(decision, "merge") {
848-
return "merge", nil
849-
}
850-
return "add", nil
851-
}
852-
853833
// mergeEntries combines two related entries into one.
854834
// When an LLM client is available, uses semantic merging for higher quality.
855835
// Falls back to simple string logic when LLM is unavailable.

0 commit comments

Comments
 (0)