Skip to content

Commit bfabde3

Browse files
committed
fix: memory system — config wiring, *bool migration, per-turn refresh, LLM merge
Critical fixes: - overlayFile now passes Memory section through (was silently dropped) - resolveMemory merges user config onto defaults (partial configs no longer zero out all boolean features) - MemoryConfig bool fields switched to *bool (same pattern as FileConfig) to distinguish 'not set' from 'explicitly false' Behavioral improvements: - Per-turn memory injection via SetMemoryPromptFunc callback: agent sees fresh facts after mutating memory during a session - mergeEntries now uses LLM for semantic merging, falls back to simple concatenation when LLM unavailable - Memory tool reports updated entries after add/replace/remove - Consolidate reports actual new entry count instead of '?' - New 'view' action to read full episode content by session ID Tests: - 6 new config tests for memory section merge chain - 4 new memory tests: view episodes, entries in response, LLM merge path, fallback merge - Full suite passes with -race, memory coverage at 87.3%
1 parent 2050243 commit bfabde3

8 files changed

Lines changed: 517 additions & 74 deletions

File tree

internal/config/loader.go

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -597,11 +597,51 @@ func resolveDangerous(cfg *danger.DangerousConfig) danger.DangerousConfig {
597597
}
598598

599599
// resolveMemory merges file-level memory config with defaults.
600+
// Starts from DefaultMemoryConfig and overlays any non-zero/non-nil
601+
// fields from cfg. This means a partial config like {"buffer_lines": 10}
602+
// won't silently disable all the boolean features.
600603
func resolveMemory(cfg *memory.MemoryConfig) memory.MemoryConfig {
601-
if cfg != nil {
602-
return *cfg
604+
def := memory.DefaultMemoryConfig()
605+
if cfg == nil {
606+
return def
607+
}
608+
if cfg.Enabled != nil {
609+
def.Enabled = cfg.Enabled
610+
}
611+
if cfg.BufferEnabled != nil {
612+
def.BufferEnabled = cfg.BufferEnabled
613+
}
614+
if cfg.MergeOnWrite != nil {
615+
def.MergeOnWrite = cfg.MergeOnWrite
616+
}
617+
if cfg.ExtractOnEnd != nil {
618+
def.ExtractOnEnd = cfg.ExtractOnEnd
619+
}
620+
if cfg.LLMSearch != nil {
621+
def.LLMSearch = cfg.LLMSearch
603622
}
604-
return memory.DefaultMemoryConfig()
623+
if cfg.LLMExtract != nil {
624+
def.LLMExtract = cfg.LLMExtract
625+
}
626+
if cfg.LLMConsolidate != nil {
627+
def.LLMConsolidate = cfg.LLMConsolidate
628+
}
629+
if cfg.FactsLimitUser > 0 {
630+
def.FactsLimitUser = cfg.FactsLimitUser
631+
}
632+
if cfg.FactsLimitEnv > 0 {
633+
def.FactsLimitEnv = cfg.FactsLimitEnv
634+
}
635+
if cfg.BufferLines > 0 {
636+
def.BufferLines = cfg.BufferLines
637+
}
638+
if cfg.MergeThreshold > 0 {
639+
def.MergeThreshold = cfg.MergeThreshold
640+
}
641+
if cfg.AddThreshold > 0 {
642+
def.AddThreshold = cfg.AddThreshold
643+
}
644+
return def
605645
}
606646

607647
// resolveTelegram merges file-level telegram config with defaults.
@@ -719,6 +759,9 @@ func overlayFile(base, override FileConfig) FileConfig {
719759
if override.Skills != nil {
720760
base.Skills = override.Skills
721761
}
762+
if override.Memory != nil {
763+
base.Memory = override.Memory
764+
}
722765
if override.Telegram != nil {
723766
base.Telegram = override.Telegram
724767
}

internal/config/loader_test.go

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import (
44
"os"
55
"path/filepath"
66
"testing"
7+
8+
"github.com/BackendStack21/kode/internal/memory"
79
)
810

911
func boolPtr(b bool) *bool { return &b }
@@ -503,3 +505,189 @@ func TestLoadConfig_SkillsLearnCLIDoesNotClobberSkillsConfig(t *testing.T) {
503505
t.Errorf("Skills.Curation.StalenessDays = %d, want 30", cfg.Skills.Curation.StalenessDays)
504506
}
505507
}
508+
509+
func TestLoadConfig_MemoryDefaults(t *testing.T) {
510+
// When no memory section is configured, the resolved config must have
511+
// sensible defaults (Enabled=true, all features on).
512+
cfg := LoadConfig(CLIFlags{})
513+
mem := cfg.Memory
514+
if mem.Enabled == nil || !*mem.Enabled {
515+
t.Error("Memory.Enabled should default to true")
516+
}
517+
if mem.BufferEnabled == nil || !*mem.BufferEnabled {
518+
t.Error("Memory.BufferEnabled should default to true")
519+
}
520+
if mem.MergeOnWrite == nil || !*mem.MergeOnWrite {
521+
t.Error("Memory.MergeOnWrite should default to true")
522+
}
523+
if mem.ExtractOnEnd == nil || !*mem.ExtractOnEnd {
524+
t.Error("Memory.ExtractOnEnd should default to true")
525+
}
526+
if mem.LLMSearch == nil || !*mem.LLMSearch {
527+
t.Error("Memory.LLMSearch should default to true")
528+
}
529+
if mem.LLMExtract == nil || !*mem.LLMExtract {
530+
t.Error("Memory.LLMExtract should default to true")
531+
}
532+
if mem.LLMConsolidate == nil || !*mem.LLMConsolidate {
533+
t.Error("Memory.LLMConsolidate should default to true")
534+
}
535+
if mem.BufferLines != 20 {
536+
t.Errorf("Memory.BufferLines = %d, want 20", mem.BufferLines)
537+
}
538+
}
539+
540+
func TestLoadConfig_MemoryFromGlobalFile(t *testing.T) {
541+
dir := t.TempDir()
542+
prevHome := os.Getenv("HOME")
543+
os.Setenv("HOME", dir)
544+
defer os.Setenv("HOME", prevHome)
545+
546+
cfgDir := filepath.Join(dir, ".odek")
547+
os.MkdirAll(cfgDir, 0755)
548+
cfgPath := filepath.Join(cfgDir, "config.json")
549+
if err := os.WriteFile(cfgPath, []byte(`{
550+
"memory": {
551+
"enabled": true,
552+
"facts_limit_user": 800,
553+
"buffer_lines": 15,
554+
"merge_on_write": false
555+
}
556+
}`), 0644); err != nil {
557+
t.Fatal(err)
558+
}
559+
560+
cfg := LoadConfig(CLIFlags{})
561+
mem := cfg.Memory
562+
563+
// Explicitly set values
564+
if mem.Enabled == nil || !*mem.Enabled {
565+
t.Error("Memory.Enabled should be true (from file)")
566+
}
567+
if mem.FactsLimitUser != 800 {
568+
t.Errorf("Memory.FactsLimitUser = %d, want 800", mem.FactsLimitUser)
569+
}
570+
if mem.BufferLines != 15 {
571+
t.Errorf("Memory.BufferLines = %d, want 15", mem.BufferLines)
572+
}
573+
if mem.MergeOnWrite == nil || *mem.MergeOnWrite {
574+
t.Error("Memory.MergeOnWrite should be false (from file)")
575+
}
576+
577+
// Unset fields must get defaults
578+
if mem.ExtractOnEnd == nil || !*mem.ExtractOnEnd {
579+
t.Error("Memory.ExtractOnEnd should default to true")
580+
}
581+
if mem.LLMSearch == nil || !*mem.LLMSearch {
582+
t.Error("Memory.LLMSearch should default to true")
583+
}
584+
}
585+
586+
func TestLoadConfig_MemoryProjectOverridesGlobal(t *testing.T) {
587+
dir := t.TempDir()
588+
prevHome := os.Getenv("HOME")
589+
os.Setenv("HOME", dir)
590+
defer os.Setenv("HOME", prevHome)
591+
592+
// Global config with memory section
593+
globalDir := filepath.Join(dir, ".odek")
594+
os.MkdirAll(globalDir, 0755)
595+
if err := os.WriteFile(filepath.Join(globalDir, "config.json"), []byte(`{
596+
"memory": {
597+
"facts_limit_user": 500,
598+
"buffer_lines": 10,
599+
"merge_on_write": true
600+
}
601+
}`), 0644); err != nil {
602+
t.Fatal(err)
603+
}
604+
605+
// Project config overrides some memory fields
606+
cwd, _ := os.Getwd()
607+
os.Chdir(dir)
608+
defer os.Chdir(cwd)
609+
610+
if err := os.WriteFile(filepath.Join(dir, "odek.json"), []byte(`{
611+
"memory": {
612+
"facts_limit_user": 1200,
613+
"buffer_lines": 25
614+
}
615+
}`), 0644); err != nil {
616+
t.Fatal(err)
617+
}
618+
619+
cfg := LoadConfig(CLIFlags{})
620+
mem := cfg.Memory
621+
622+
// Project overrides
623+
if mem.FactsLimitUser != 1200 {
624+
t.Errorf("Memory.FactsLimitUser = %d, want 1200 (project overrides global)", mem.FactsLimitUser)
625+
}
626+
if mem.BufferLines != 25 {
627+
t.Errorf("Memory.BufferLines = %d, want 25 (project overrides global)", mem.BufferLines)
628+
}
629+
630+
// Global value preserved (not overridden by project)
631+
if mem.MergeOnWrite == nil || !*mem.MergeOnWrite {
632+
t.Error("Memory.MergeOnWrite should be true (preserved from global)")
633+
}
634+
635+
// Defaults for fields not set in either file
636+
if mem.Enabled == nil || !*mem.Enabled {
637+
t.Error("Memory.Enabled should default to true")
638+
}
639+
}
640+
641+
func TestResolveMemoryMergesDefaults(t *testing.T) {
642+
// resolveMemory must overlay user config onto DefaultMemoryConfig
643+
// so partial configs don't zero out boolean features.
644+
cfg := &memory.MemoryConfig{
645+
FactsLimitUser: 300,
646+
BufferLines: 5,
647+
}
648+
resolved := resolveMemory(cfg)
649+
650+
if resolved.FactsLimitUser != 300 {
651+
t.Errorf("FactsLimitUser = %d, want 300", resolved.FactsLimitUser)
652+
}
653+
if resolved.BufferLines != 5 {
654+
t.Errorf("BufferLines = %d, want 5", resolved.BufferLines)
655+
}
656+
// Bool defaults must be preserved
657+
if resolved.Enabled == nil || !*resolved.Enabled {
658+
t.Error("Enabled should default to true when not explicitly set")
659+
}
660+
if resolved.ExtractOnEnd == nil || !*resolved.ExtractOnEnd {
661+
t.Error("ExtractOnEnd should default to true")
662+
}
663+
}
664+
665+
func TestResolveMemoryExplicitFalse(t *testing.T) {
666+
// When user explicitly sets a bool to false, it must stay false.
667+
cfg := &memory.MemoryConfig{
668+
Enabled: memory.BoolPtr(false),
669+
}
670+
resolved := resolveMemory(cfg)
671+
672+
if resolved.Enabled == nil || *resolved.Enabled {
673+
t.Error("Enabled should be false when explicitly set to false")
674+
}
675+
// Other bools still get defaults
676+
if resolved.ExtractOnEnd == nil || !*resolved.ExtractOnEnd {
677+
t.Error("ExtractOnEnd should default to true")
678+
}
679+
}
680+
681+
func TestLoadConfig_MemoryNotSetReturnsDefaults(t *testing.T) {
682+
// When memory key is absent from all config layers, resolveMemory(nil)
683+
// must return DefaultMemoryConfig.
684+
resolved := resolveMemory(nil)
685+
def := memory.DefaultMemoryConfig()
686+
687+
if resolved.FactsLimitUser != def.FactsLimitUser {
688+
t.Errorf("FactsLimitUser = %d, want %d (default)", resolved.FactsLimitUser, def.FactsLimitUser)
689+
}
690+
if resolved.Enabled == nil || *resolved.Enabled != *def.Enabled {
691+
t.Error("Enabled should match default")
692+
}
693+
}

internal/loop/loop.go

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,15 +49,21 @@ type Engine struct {
4949
renderer *render.Renderer // optional: colored terminal output
5050
maxIter int
5151
system string
52-
maxContext int // max context tokens (0 = no limit)
53-
skillLoader SkillLoader // optional: loads matching skills
54-
lastSkillMsg string // last user message that triggered skill loading (dedup)
52+
baseSystem string // original system message without memory/skills
53+
maxContext int // max context tokens (0 = no limit)
54+
skillLoader SkillLoader // optional: loads matching skills
55+
lastSkillMsg string // last user message that triggered skill loading (dedup)
5556

5657
toolEventHandler ToolEventHandler // optional: fires during tool execution
5758

5859
// iterationCallback is an optional callback fired after each iteration.
5960
iterationCallback IterationCallback
6061

62+
// memoryPromptFunc is called before each LLM invocation to get fresh
63+
// memory content. This ensures memory mutations during a session
64+
// are visible to the agent on the next turn.
65+
memoryPromptFunc func() string
66+
6167
// PromptCaching enables Anthropic/OpenAI/DeepSeek prompt caching markers.
6268
// When enabled, the system prompt and first user message are annotated
6369
// with cache_control markers, and the system prompt is moved to the
@@ -92,6 +98,17 @@ func New(client *llm.Client, registry *tool.Registry, maxIterations int, systemM
9298
// SetSkillLoader sets the optional skill loader callback.
9399
func (e *Engine) SetSkillLoader(sl SkillLoader) { e.skillLoader = sl }
94100

101+
// SetMemoryPromptFunc sets the optional memory prompt callback.
102+
// When set, it is called before each LLM invocation to get fresh memory
103+
// content. This ensures the agent sees the latest facts even if it
104+
// modifies memory during a session.
105+
func (e *Engine) SetMemoryPromptFunc(fn func() string) {
106+
e.memoryPromptFunc = fn
107+
if fn != nil {
108+
e.baseSystem = e.system
109+
}
110+
}
111+
95112
// SetToolEventHandler sets the optional tool event callback for live streaming.
96113
func (e *Engine) SetToolEventHandler(cb ToolEventHandler) { e.toolEventHandler = cb }
97114

@@ -307,6 +324,18 @@ func (e *Engine) runLoop(ctx context.Context, messages []llm.Message) (string, [
307324
}
308325
}
309326

327+
// Refresh memory content before each LLM call so the agent sees
328+
// the latest facts even if it mutated memory during this session.
329+
if e.memoryPromptFunc != nil {
330+
if memBlock := e.memoryPromptFunc(); memBlock != "" {
331+
e.system = e.baseSystem + memBlock
332+
// Update messages[0] if it's the system message
333+
if len(messages) > 0 && messages[0].Role == "system" {
334+
messages[0].Content = e.system
335+
}
336+
}
337+
}
338+
310339
// THINK (timed)
311340
start := time.Now()
312341

0 commit comments

Comments
 (0)