Skip to content

Commit 7662fe0

Browse files
jkyberneeesclaude
andauthored
feat(memory): deterministic buffer turn-summaries (no LLM) (#16)
* feat(memory): deterministic buffer turn-summaries (no LLM) The tier-2 buffer is injected into the system prompt every turn but each entry was a naive truncation of raw text (message[:97]+"..." or shorten(s,100)) — a byte slice that can split a UTF-8 rune and usually captures only filler ("Sure, I'll help. Let me start by..."), leaving the buffer with almost no recall signal. Centralize summarization in MemoryManager.AppendBuffer via a new deterministic, no-LLM, rune-safe summarizeForBuffer: strip fenced/inline code, conservative markdown, collapse whitespace, drop a leading filler clause (only when substance remains), and excerpt to 200 runes at a sentence/word boundary. All 8 call sites now pass raw text. Placement is load-bearing: summarization lives in AppendBuffer, not Buffer.Append, so RestoreBuffer (which calls Buffer.Append directly with already-summarized lines) never re-processes persisted summaries. Old session buffers restore verbatim — no migration. shorten() is retained for its non-buffer callers (session labels, tool-result display). Tests: table-driven heuristic cases (code/markdown/filler/multibyte/ hard-cut), an AppendBuffer no-mid-word-cut regression, and a verbatim RestoreBuffer guard for the invariant. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(memory): harden buffer excerpt against early-terminator collapse Adversarial review (AI Verification Protocol) found two defects in summarizeForBuffer: - D-01 (critical): when the only sentence terminator in the window was an early abbreviation/version/domain ("e.g.", "v1.2", "node.js"), the excerpt collapsed to a few runes (e.g. "e.g.…") — destroying the summary. Now a sentence cut is only preferred when it lands at least halfway through the window; otherwise fall back to the word boundary near the cap. - D-02 (minor): an unclosed code fence (unmatched by the fenced-block regex) left a stray backtick in the output. Residual backticks are now stripped after inline-code unwrapping. Adds regression tests for both. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(memory): make LLM test mocks concurrency-safe CI's `go test -race` flagged a write/write data race on mockLLM.lastUser in TestConsolidateOnEnd_FiresAtSessionEnd. Root cause: OnSessionEndWithProvenance legitimately calls the LLM from two goroutines at once — synchronous episode extraction plus the background consolidation goroutine — but the test mocks held unsynchronized shared state. - mockLLM: guard lastUser with a mutex; add getLastUser() accessor. - countCallsLLM: replace the non-atomic *int counter with atomic.Int64 and a calls() accessor (same latent race, same OnSessionEnd trigger). Test-only change; no production code affected. Verified with `go test ./internal/memory/... -short -race -count=30` and the full `go test ./... -short -race` suite. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent ad0be26 commit 7662fe0

8 files changed

Lines changed: 394 additions & 43 deletions

File tree

cmd/odek/main.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -877,9 +877,9 @@ func run(args []string) error {
877877
messages = append([]llm.Message{{Role: "system", Content: systemMessage}}, messages...)
878878
}
879879

880-
// Append user input to buffer
880+
// Append user input to buffer (AppendBuffer summarizes raw text).
881881
if mm := agent.Memory(); mm != nil {
882-
mm.AppendBuffer("user", shorten(f.Task, 100))
882+
mm.AppendBuffer("user", f.Task)
883883
}
884884

885885
result, allMessages, runErr = agent.RunWithMessages(ctx, messages)
@@ -889,7 +889,7 @@ func run(args []string) error {
889889
if mm := agent.Memory(); mm != nil {
890890
for i := len(allMessages) - 1; i >= 0; i-- {
891891
if allMessages[i].Role == "assistant" && allMessages[i].Content != "" {
892-
mm.AppendBuffer("agent", shorten(allMessages[i].Content, 100))
892+
mm.AppendBuffer("agent", allMessages[i].Content)
893893
break
894894
}
895895
}
@@ -1675,9 +1675,9 @@ func continueCmd(args []string) error {
16751675
messages := sess.GetMessages()
16761676
messages = append(messages, llm.Message{Role: "user", Content: task})
16771677

1678-
// Append user input to buffer
1678+
// Append user input to buffer (AppendBuffer summarizes raw text).
16791679
if mm := agent.Memory(); mm != nil {
1680-
mm.AppendBuffer("user", shorten(task, 100))
1680+
mm.AppendBuffer("user", task)
16811681
}
16821682

16831683
ctx, cancel := signal.NotifyContext(context.Background(), os.Interrupt)
@@ -1710,7 +1710,7 @@ func continueCmd(args []string) error {
17101710
if mm := agent.Memory(); mm != nil {
17111711
for i := len(allMessages) - 1; i >= 0; i-- {
17121712
if allMessages[i].Role == "assistant" && allMessages[i].Content != "" {
1713-
mm.AppendBuffer("agent", shorten(allMessages[i].Content, 100))
1713+
mm.AppendBuffer("agent", allMessages[i].Content)
17141714
break
17151715
}
17161716
}

cmd/odek/repl.go

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -224,13 +224,9 @@ func replCmd(args []string) error {
224224
origLen := len(messages)
225225
messages = append(messages, llm.Message{Role: "user", Content: input})
226226

227-
// Append user input to buffer
227+
// Append user input to buffer (AppendBuffer summarizes raw text).
228228
if mm := agent.Memory(); mm != nil {
229-
userSummary := input
230-
if len(userSummary) > 100 {
231-
userSummary = userSummary[:97] + "..."
232-
}
233-
mm.AppendBuffer("user", userSummary)
229+
mm.AppendBuffer("user", input)
234230
}
235231

236232
// Run agent with full history
@@ -241,14 +237,10 @@ func replCmd(args []string) error {
241237
continue
242238
}
243239

244-
// Append agent response to buffer
240+
// Append agent response to buffer (AppendBuffer summarizes raw text).
245241
if mm := agent.Memory(); mm != nil && len(allMessages) > 0 {
246242
if last := allMessages[len(allMessages)-1]; last.Role == "assistant" {
247-
summary := last.Content
248-
if len(summary) > 100 {
249-
summary = summary[:97] + "..."
250-
}
251-
mm.AppendBuffer("agent", summary)
243+
mm.AppendBuffer("agent", last.Content)
252244
}
253245
}
254246

cmd/odek/serve.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -652,10 +652,9 @@ func handlePrompt(
652652
}
653653
writeWSJSON(conn, map[string]any{"type": "session", "session_id": sid, "model": resolved.Model, "sandbox": resolved.Sandbox})
654654

655-
// Append user input to buffer
655+
// Append user input to buffer (AppendBuffer summarizes raw text).
656656
if mm := agent.Memory(); mm != nil {
657-
userSummary := shorten(prompt, 100)
658-
mm.AppendBuffer("user", userSummary)
657+
mm.AppendBuffer("user", prompt)
659658
}
660659

661660
// Run agent. Audit recorder wired around the loop so every
@@ -753,8 +752,7 @@ func handlePrompt(
753752
if mm := agent.Memory(); mm != nil {
754753
for _, msg := range newMsgs {
755754
if msg.Role == "assistant" && msg.Content != "" {
756-
agentSummary := shorten(msg.Content, 100)
757-
mm.AppendBuffer("agent", agentSummary)
755+
mm.AppendBuffer("agent", msg.Content)
758756
break
759757
}
760758
}

internal/memory/episode_index_test.go

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66
"path/filepath"
77
"sync"
8+
"sync/atomic"
89
"testing"
910
)
1011

@@ -157,12 +158,11 @@ func TestEpisodeIndex_FormatEpisodeContextNoLLM(t *testing.T) {
157158
resetEpIdxes()
158159
dir := t.TempDir()
159160

160-
callCount := 0
161161
llm := &mockLLM{responses: map[string]string{
162162
"Summarize": "session summary",
163163
}}
164164
// Override SimpleCall to count calls.
165-
countingLLM := &countCallsLLM{inner: llm, count: &callCount}
165+
countingLLM := &countCallsLLM{inner: llm}
166166

167167
cfg := DefaultMemoryConfig()
168168
cfg.LLMSearch = boolPtr(true) // explicitly on — FormatEpisodeContext must still skip it
@@ -172,11 +172,11 @@ func TestEpisodeIndex_FormatEpisodeContextNoLLM(t *testing.T) {
172172
msgs := []string{"user: hi", "assistant: built the postgres schema", "user: ok", "assistant: done"}
173173
mm.OnSessionEndWithProvenance("20260605-a", 5, msgs, EpisodeProvenance{})
174174

175-
before := callCount
175+
before := countingLLM.calls()
176176

177177
// FormatEpisodeContext must not add any LLM calls.
178178
_ = mm.FormatEpisodeContext("postgres schema")
179-
after := callCount
179+
after := countingLLM.calls()
180180

181181
if after != before {
182182
t.Errorf("FormatEpisodeContext made %d LLM call(s); want 0 (should use vector index only)",
@@ -185,16 +185,23 @@ func TestEpisodeIndex_FormatEpisodeContextNoLLM(t *testing.T) {
185185
}
186186

187187
// countCallsLLM wraps mockLLM and counts SimpleCall invocations.
188+
//
189+
// The counter is atomic: OnSessionEndWithProvenance calls the LLM from two
190+
// goroutines at once (synchronous episode extraction + background
191+
// consolidation), so a plain int would race under `go test -race`.
188192
type countCallsLLM struct {
189193
inner *mockLLM
190-
count *int
194+
n atomic.Int64
191195
}
192196

193197
func (c *countCallsLLM) SimpleCall(ctx context.Context, system, user string) (string, error) {
194-
*c.count++
198+
c.n.Add(1)
195199
return c.inner.SimpleCall(ctx, system, user)
196200
}
197201

202+
// calls returns the number of SimpleCall invocations so far.
203+
func (c *countCallsLLM) calls() int { return int(c.n.Load()) }
204+
198205
// TestEpisodeIndex_ConcurrentSafety: N goroutines sharing one memory dir write
199206
// episodes and recall concurrently. No race, no crashes.
200207
func TestEpisodeIndex_ConcurrentSafety(t *testing.T) {
@@ -267,22 +274,21 @@ func TestEpisodeIndex_AbsPath(t *testing.T) {
267274
// returns vector-ranked candidates without making any LLM call.
268275
func TestSearchEpisodes_LLMSearchFalse(t *testing.T) {
269276
resetEpIdxes()
270-
llmCalls := 0
271-
llm := &countCallsLLM{inner: &mockLLM{responses: map[string]string{}}, count: &llmCalls}
277+
llm := &countCallsLLM{inner: &mockLLM{responses: map[string]string{}}}
272278
cfg := DefaultMemoryConfig()
273279
cfg.LLMSearch = boolPtr(false)
274280
mm := NewMemoryManager(t.TempDir(), llm, cfg)
275281

276282
msgs := []string{"user: hi", "assistant: postgres schema done", "user: ok", "assistant: done"}
277283
mm.OnSessionEndWithProvenance("20260701-a", 5, msgs, EpisodeProvenance{})
278284

279-
before := llmCalls
285+
before := llm.calls()
280286
results, err := mm.SearchEpisodes("postgres", 3)
281287
if err != nil {
282288
t.Fatalf("SearchEpisodes: %v", err)
283289
}
284-
if llmCalls != before {
285-
t.Errorf("llm_search=false should fire 0 LLM calls, got %d", llmCalls-before)
290+
if after := llm.calls(); after != before {
291+
t.Errorf("llm_search=false should fire 0 LLM calls, got %d", after-before)
286292
}
287293
_ = results
288294
}
@@ -373,15 +379,13 @@ func TestSearchEpisodes_RankFnErrorFallback(t *testing.T) {
373379
func TestSearchEpisodes_OOVFallbackToLLM(t *testing.T) {
374380
resetEpIdxes()
375381
dir := t.TempDir()
376-
llmCalls := 0
377382
llm := &countCallsLLM{
378383
inner: &mockLLM{responses: map[string]string{
379384
"Summarize": "postgres work",
380385
"relevance": "0",
381386
"rank": "0",
382387
"most relev": "0",
383388
}},
384-
count: &llmCalls,
385389
}
386390
cfg := DefaultMemoryConfig()
387391
cfg.LLMSearch = boolPtr(true)
@@ -390,10 +394,10 @@ func TestSearchEpisodes_OOVFallbackToLLM(t *testing.T) {
390394
msgs := []string{"user: hi", "assistant: postgres schema done", "user: ok", "assistant: done"}
391395
mm.OnSessionEndWithProvenance("20260705-a", 5, msgs, EpisodeProvenance{})
392396

393-
before := llmCalls
397+
before := llm.calls()
394398
// Query with zero vocabulary overlap (all OOV) → recallByVector returns nil → fallback to Search
395399
_, _ = mm.SearchEpisodes("xyzzy wumpus frobnitz", 3)
396-
after := llmCalls
400+
after := llm.calls()
397401
t.Logf("OOV query: llm calls=%d (0 means vector returned nothing, fallback to LLM Search)", after-before)
398402
// After D-05 fix: OOV → recallByVector returns nil → SearchEpisodes falls back to episodes.Search
399403
// which uses the LLM ranker. We don't assert an exact count because the fallback

internal/memory/memory.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -449,11 +449,18 @@ Entries for %s:
449449

450450
// ── Buffer Operations ────────────────────────────────────────────────
451451

452-
// AppendBuffer adds a turn summary to the in-memory ring buffer.
452+
// AppendBuffer adds a turn summary to the in-memory ring buffer. Callers pass
453+
// the RAW turn text; summarizeForBuffer derives a clean, bounded excerpt here so
454+
// every entry point shares one policy.
455+
//
456+
// Invariant: summarization lives here, not in Buffer.Append, so that
457+
// RestoreBuffer (which calls Buffer.Append directly with already-formatted,
458+
// already-summarized lines) never re-processes persisted summaries.
453459
func (m *MemoryManager) AppendBuffer(role, message string) {
454460
if m.cfg.BufferEnabled == nil || !*m.cfg.BufferEnabled {
455461
return
456462
}
463+
message = summarizeForBuffer(message)
457464
line := FormatBufferLine(role, message)
458465
m.buffer.Append(line)
459466
m.markPromptDirty()
@@ -468,6 +475,10 @@ func (m *MemoryManager) GetBuffer() []string {
468475
}
469476

470477
// RestoreBuffer loads buffer lines from a saved slice (e.g., from session).
478+
//
479+
// Invariant: these lines are already-formatted, already-summarized buffer
480+
// entries. They go straight to Buffer.Append and MUST NOT be routed through
481+
// AppendBuffer/summarizeForBuffer, which would corrupt the persisted summaries.
471482
func (m *MemoryManager) RestoreBuffer(lines []string) {
472483
if m.cfg.BufferEnabled == nil || !*m.cfg.BufferEnabled {
473484
return

internal/memory/memory_test.go

Lines changed: 88 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,27 @@ import (
44
"context"
55
"os"
66
"strings"
7+
"sync"
78
"testing"
9+
"unicode/utf8"
810
)
911

1012
// mockLLM is a simple LLMClient mock for testing.
13+
//
14+
// SimpleCall is concurrency-safe: OnSessionEndWithProvenance legitimately calls
15+
// the LLM from two goroutines at once (synchronous episode extraction + the
16+
// background consolidation goroutine), so the mock must guard its shared state
17+
// or `go test -race` flags a write/write race on lastUser.
1118
type mockLLM struct {
12-
responses map[string]string // query prefix → response
19+
responses map[string]string // query prefix → response (read-only after init)
20+
mu sync.Mutex // guards lastUser
1321
lastUser string // captured last user prompt
1422
}
1523

1624
func (m *mockLLM) SimpleCall(ctx context.Context, system, user string) (string, error) {
25+
m.mu.Lock()
1726
m.lastUser = user
27+
m.mu.Unlock()
1828
for prefix, resp := range m.responses {
1929
if strings.Contains(system, prefix) || strings.Contains(user, prefix) {
2030
return resp, nil
@@ -23,6 +33,13 @@ func (m *mockLLM) SimpleCall(ctx context.Context, system, user string) (string,
2333
return "", nil
2434
}
2535

36+
// getLastUser returns the last captured user prompt under the lock.
37+
func (m *mockLLM) getLastUser() string {
38+
m.mu.Lock()
39+
defer m.mu.Unlock()
40+
return m.lastUser
41+
}
42+
2643
func TestMemoryManagerAddAndReadFacts(t *testing.T) {
2744
dir := t.TempDir()
2845
mm := NewMemoryManager(dir, nil, DefaultMemoryConfig())
@@ -149,6 +166,72 @@ func TestMemoryManagerBufferRestore(t *testing.T) {
149166
}
150167
}
151168

169+
// bufferMessage extracts the message portion of a formatted buffer line
170+
// ("HH:MM role message").
171+
func bufferMessage(t *testing.T, line string) string {
172+
t.Helper()
173+
parts := strings.SplitN(line, " ", 3)
174+
if len(parts) != 3 {
175+
t.Fatalf("unexpected buffer line format: %q", line)
176+
}
177+
return parts[2]
178+
}
179+
180+
// TestAppendBufferCleansAndDoesNotMidWordCut verifies that AppendBuffer routes
181+
// raw text through summarizeForBuffer: code/markdown noise is stripped and the
182+
// excerpt is bounded and rune-safe (no mid-word/mid-rune chop).
183+
func TestAppendBufferCleansAndDoesNotMidWordCut(t *testing.T) {
184+
dir := t.TempDir()
185+
mm := NewMemoryManager(dir, nil, DefaultMemoryConfig())
186+
187+
raw := "Sure, I'll help with that.\n\n```go\nfunc main() {}\n```\n" +
188+
strings.Repeat("Then we verify the behavior carefully. ", 30)
189+
mm.AppendBuffer("agent", raw)
190+
191+
lines := mm.GetBuffer()
192+
if len(lines) != 1 {
193+
t.Fatalf("expected 1 buffer line, got %d", len(lines))
194+
}
195+
msg := bufferMessage(t, lines[0])
196+
197+
if strings.Contains(msg, "\n") {
198+
t.Errorf("buffer message contains a newline: %q", msg)
199+
}
200+
if strings.Contains(msg, "```") {
201+
t.Errorf("buffer message still contains a code fence: %q", msg)
202+
}
203+
if !utf8.ValidString(msg) {
204+
t.Errorf("buffer message is not valid UTF-8: %q", msg)
205+
}
206+
if n := utf8.RuneCountInString(msg); n > maxBufferSummaryRunes+1 {
207+
t.Errorf("buffer message rune count %d exceeds cap %d (+1)", n, maxBufferSummaryRunes)
208+
}
209+
}
210+
211+
// TestRestoreBufferPreservesLinesVerbatim guards the load-bearing invariant:
212+
// RestoreBuffer must NOT re-summarize. It includes a line whose content would be
213+
// mangled if it were routed through summarizeForBuffer.
214+
func TestRestoreBufferPreservesLinesVerbatim(t *testing.T) {
215+
dir := t.TempDir()
216+
mm := NewMemoryManager(dir, nil, DefaultMemoryConfig())
217+
218+
saved := []string{
219+
"14:00 user first turn",
220+
"14:01 agent Sure, I'll help. ```code``` # heading",
221+
}
222+
mm.RestoreBuffer(saved)
223+
224+
lines := mm.GetBuffer()
225+
if len(lines) != len(saved) {
226+
t.Fatalf("expected %d lines, got %d", len(saved), len(lines))
227+
}
228+
for i := range saved {
229+
if lines[i] != saved[i] {
230+
t.Errorf("line %d not verbatim:\n got %q\n want %q", i, lines[i], saved[i])
231+
}
232+
}
233+
}
234+
152235
func TestMemoryManagerBuildSystemPrompt(t *testing.T) {
153236
dir := t.TempDir()
154237
mm := NewMemoryManager(dir, nil, DefaultMemoryConfig())
@@ -255,12 +338,13 @@ func TestOnSessionEnd_StructuredPrompt(t *testing.T) {
255338
"user: great, please add tests",
256339
})
257340

258-
if llm.lastUser == "" {
341+
last := llm.getLastUser()
342+
if last == "" {
259343
t.Fatal("extraction LLM was not called")
260344
}
261-
lower := strings.ToLower(llm.lastUser)
345+
lower := strings.ToLower(last)
262346
if !strings.Contains(lower, "user:") && !strings.Contains(lower, "assistant:") {
263-
t.Error("extraction prompt should contain user:/assistant: labels, got:\n" + llm.lastUser)
347+
t.Error("extraction prompt should contain user:/assistant: labels, got:\n" + last)
264348
}
265349
}
266350

0 commit comments

Comments
 (0)