Skip to content

Commit 0cfca13

Browse files
committed
fix: 5 corrections from code audit
- #5: Replace custom itoa() with strconv.Itoa() in scored_matcher (+ regression test) - #7: Revert wrong emoji in curation report (📦 was correct, 🫠 was accidental) - #4: Add sendAsync() helper with error logging — fire-and-forget goroutines no longer silently fail - #6: Increase SkipThreshold default 1→3 — one accidental skip no longer permanently suppresses - #2: Gate episode auto-search behind LLMSearch config — stops unconditional startup latency - #3: Cache memory prompt block in BuildSystemPrompt — invalidate on Add/Replace/Remove/Append/Clear (+ regression test) + verbose gating: skill lifecycle output (load/save/suggest/delete) suppressed when verbose=false (notifier events still fire for Telegram/WebUI in all modes)
1 parent 98fb52c commit 0cfca13

14 files changed

Lines changed: 229 additions & 84 deletions

File tree

cmd/odek/main.go

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -845,6 +845,12 @@ func run(args []string) error {
845845
color := !resolved.NoColor && render.ColorEnabled()
846846
rend := render.New(os.Stderr, color).WithModel(modelLabel)
847847

848+
// Wire skill verbosity to the renderer so skill lifecycle
849+
// notifications (save, suggest, delete) respect the config.
850+
if resolved.Skills.Learn {
851+
rend.WithSkillVerbose(resolved.Skills.Verbose)
852+
}
853+
848854
// Resolve skills config pointer (only when learn mode is enabled)
849855
var skillsCfg *skills.SkillsConfig
850856
if resolved.Skills.Learn {
@@ -1331,7 +1337,7 @@ func runLearnLoop(messages []llm.Message, task string, sm *skills.SkillManager,
13311337
// Filter out previously-skipped suggestions
13321338
filtered, skipped := skills.FilterSkipped(suggestions, userDir,
13331339
skillsCfg.Curation.SkipThreshold, skillsCfg.Curation.SkipResetDays)
1334-
if skipped > 0 {
1340+
if skipped > 0 && skillsCfg.Verbose {
13351341
fmt.Fprintf(os.Stderr, " (%d suggestion(s) previously skipped, suppressed)\n", skipped)
13361342
}
13371343
if len(filtered) == 0 {
@@ -1342,23 +1348,28 @@ func runLearnLoop(messages []llm.Message, task string, sm *skills.SkillManager,
13421348
if skillsCfg.AutoSave.Enabled {
13431349
if !skillsCfg.AutoSave.RequireLLM || skillsCfg.LLMLearn {
13441350
result := skills.AutoSaveSuggestions(filtered, userDir, skillsCfg)
1345-
for _, name := range result.Saved {
1346-
heuristic := result.Heuristics[name]
1347-
if heuristic != "" {
1348-
fmt.Fprintf(os.Stderr, " ✓ Auto-saved skill %q (%s)\n", name, heuristic)
1349-
} else {
1350-
fmt.Fprintf(os.Stderr, " ✓ Auto-saved skill %q\n", name)
1351+
if skillsCfg.Verbose {
1352+
for _, name := range result.Saved {
1353+
heuristic := result.Heuristics[name]
1354+
if heuristic != "" {
1355+
fmt.Fprintf(os.Stderr, " ✓ Auto-saved skill %q (%s)\n", name, heuristic)
1356+
} else {
1357+
fmt.Fprintf(os.Stderr, " ✓ Auto-saved skill %q\n", name)
1358+
}
1359+
}
1360+
if result.Skipped > 0 {
1361+
fmt.Fprintf(os.Stderr, " (%d previously skipped, suppressed)\n", result.Skipped)
13511362
}
1363+
for _, name := range result.Failed {
1364+
fmt.Fprintf(os.Stderr, " ⚠ Quality gate failed for %q (use --no-auto-save to review manually)\n", name)
1365+
}
1366+
}
1367+
// Fire notifier events even when silent so WebUI/Telegram get them
1368+
for _, name := range result.Saved {
13521369
sm.Notifier.Notify(skills.SkillEvent{
13531370
Type: "saved", SkillName: name, Timestamp: time.Now().UTC(),
13541371
})
13551372
}
1356-
if result.Skipped > 0 {
1357-
fmt.Fprintf(os.Stderr, " (%d previously skipped, suppressed)\n", result.Skipped)
1358-
}
1359-
for _, name := range result.Failed {
1360-
fmt.Fprintf(os.Stderr, " ⚠ Quality gate failed for %q (use --no-auto-save to review manually)\n", name)
1361-
}
13621373
if len(result.Saved) > 0 {
13631374
sm.Reload()
13641375
// Run micro-curation after auto-save
@@ -1369,6 +1380,9 @@ func runLearnLoop(messages []llm.Message, task string, sm *skills.SkillManager,
13691380
}
13701381

13711382
// Interactive fallback: show preview and prompt
1383+
if !skillsCfg.Verbose {
1384+
return // silently skip interactive prompt in non-verbose mode
1385+
}
13721386
fmt.Fprintf(os.Stderr, "\n🔍 Learning: detected %d skill pattern(s)\n", len(filtered))
13731387
for _, s := range filtered {
13741388
fmt.Fprint(os.Stderr, skills.FormatSuggestionWithPreview(s, true, 400))
@@ -1407,7 +1421,7 @@ func runAutoCurate(userDir string, sm *skills.SkillManager, cfg skills.SkillsCon
14071421
}
14081422
}
14091423
msg := skills.RunAutoCurate(userDir, newSkills, allSkills, cfg, llmClient)
1410-
if msg != "" {
1424+
if msg != "" && cfg.Verbose {
14111425
fmt.Fprint(os.Stderr, msg)
14121426
}
14131427
}

cmd/odek/main_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1369,7 +1369,7 @@ func TestRunLearn_MultiStepProcedure(t *testing.T) {
13691369

13701370
// Create local odek.json with auto_save enabled, LLM enhancement disabled
13711371
// (mock server can't handle enhancement prompts)
1372-
configContent := `{"skills": {"auto_save": {"enabled": true, "require_llm": false}, "llm_learn": false}}`
1372+
configContent := `{"skills": {"verbose": true, "auto_save": {"enabled": true, "require_llm": false}, "llm_learn": false}}`
13731373
os.WriteFile("odek.json", []byte(configContent), 0644)
13741374
defer os.Remove("odek.json")
13751375

@@ -1426,7 +1426,7 @@ func TestRunLearn_InteractiveReject(t *testing.T) {
14261426
}()
14271427

14281428
// Create local odek.json with auto_save disabled and LLM enhancement disabled
1429-
configContent := `{"skills": {"auto_save": {"enabled": false}, "llm_learn": false}}`
1429+
configContent := `{"skills": {"verbose": true, "auto_save": {"enabled": false}, "llm_learn": false}}`
14301430
os.WriteFile("odek.json", []byte(configContent), 0644)
14311431
defer os.Remove("odek.json")
14321432

cmd/odek/telegram.go

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -745,16 +745,16 @@ func handleChatMessage(
745745
},
746746
SkillEventHandler: func(event skills.SkillEvent) {
747747
switch event.Type {
748-
case "loaded":
749-
names := strings.Join(event.Skills, ", ")
750-
go bot.SendMessage(chatID, "📚 Loaded skill: "+names, &telegram.SendOpts{ReplyToMessageID: messageID})
751-
case "autoloaded":
752-
names := strings.Join(event.Skills, ", ")
753-
go bot.SendMessage(chatID, "📚 Auto-loaded skills: "+names, &telegram.SendOpts{ReplyToMessageID: messageID})
754-
case "saved":
755-
go bot.SendMessage(chatID, fmt.Sprintf("✓ Saved skill %q", event.SkillName), &telegram.SendOpts{ReplyToMessageID: messageID})
756-
case "deleted":
757-
go bot.SendMessage(chatID, fmt.Sprintf("✗ Deleted skill %q", event.SkillName), &telegram.SendOpts{ReplyToMessageID: messageID})
748+
case "loaded":
749+
names := strings.Join(event.Skills, ", ")
750+
sendAsync(bot, chatID, "📚 Loaded skill: "+names, &telegram.SendOpts{ReplyToMessageID: messageID})
751+
case "autoloaded":
752+
names := strings.Join(event.Skills, ", ")
753+
sendAsync(bot, chatID, "📚 Auto-loaded skills: "+names, &telegram.SendOpts{ReplyToMessageID: messageID})
754+
case "saved":
755+
sendAsync(bot, chatID, fmt.Sprintf("✓ Saved skill %q", event.SkillName), &telegram.SendOpts{ReplyToMessageID: messageID})
756+
case "deleted":
757+
sendAsync(bot, chatID, fmt.Sprintf("✗ Deleted skill %q", event.SkillName), &telegram.SendOpts{ReplyToMessageID: messageID})
758758
case "suggested":
759759
replyMarkup := &telegram.InlineKeyboardMarkup{
760760
InlineKeyboard: [][]telegram.InlineKeyboardButton{
@@ -779,8 +779,8 @@ func handleChatMessage(
779779
}
780780
msg += fmt.Sprintf("\n\n```\n%s\n```", preview)
781781
}
782-
go bot.SendMessage(chatID, msg,
783-
&telegram.SendOpts{ReplyMarkup: replyMarkup, ParseMode: "Markdown", ReplyToMessageID: messageID})
782+
sendAsync(bot, chatID, msg,
783+
&telegram.SendOpts{ReplyMarkup: replyMarkup, ParseMode: "Markdown", ReplyToMessageID: messageID})
784784
}
785785
},
786786
}
@@ -890,9 +890,9 @@ func handleChatMessage(
890890
}
891891
}
892892
msg := skills.RunAutoCurate(userDir, newSkills, allSkills, *skillsCfg, nil)
893-
if msg != "" {
894-
go bot.SendMessage(chatID, msg, nil)
895-
}
893+
if msg != "" {
894+
sendAsync(bot, chatID, msg, nil)
895+
}
896896
}
897897
} else {
898898
// Store suggestions for inline keyboard callback handling
@@ -1002,6 +1002,17 @@ func reportError(bot *telegram.Bot, chatID int64, messageID int, msg string) {
10021002
}
10031003
}
10041004

1005+
// sendAsync sends a Telegram message in a background goroutine and logs
1006+
// any errors to stderr. Use this instead of raw go bot.SendMessage() to
1007+
// prevent silent delivery failures.
1008+
func sendAsync(bot *telegram.Bot, chatID int64, text string, opts *telegram.SendOpts) {
1009+
go func() {
1010+
if _, err := bot.SendMessage(chatID, text, opts); err != nil {
1011+
fmt.Fprintf(os.Stderr, "odek telegram: async send failed: %v\n", err)
1012+
}
1013+
}()
1014+
}
1015+
10051016
// ── Singleton Lock ─────────────────────────────────────────────────────
10061017
//
10071018
// Prevents two bot instances from polling Telegram simultaneously (which

internal/memory/memory.go

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,12 @@ type MemoryManager struct {
7575
merge *MergeDetector
7676
llm LLMClient
7777
cfg MemoryConfig
78+
79+
// prompt caching avoids rebuilding the system prompt block on every
80+
// iteration when memory hasn't changed. The cache is invalidated
81+
// whenever facts or buffer are modified.
82+
promptCache string
83+
promptDirty bool
7884
}
7985

8086
// NewMemoryManager creates a fully wired MemoryManager.
@@ -222,6 +228,7 @@ func (m *MemoryManager) AddFact(target, content string) error {
222228
if err := m.facts.Add(target, content); err != nil {
223229
return err
224230
}
231+
m.promptDirty = true
225232

226233
// Incrementally update merge detector instead of re-reading + re-embedding all.
227234
// Check dedup: if content already existed in the entries we read at the top,
@@ -260,6 +267,7 @@ func (m *MemoryManager) ReplaceFact(target, oldText, content string) error {
260267
if err := m.facts.Replace(target, oldText, content); err != nil {
261268
return err
262269
}
270+
m.promptDirty = true
263271
// Re-fit merge detector
264272
if m.cfg.MergeOnWrite != nil && *m.cfg.MergeOnWrite {
265273
entries, _ := m.facts.Entries(target)
@@ -276,6 +284,7 @@ func (m *MemoryManager) RemoveFact(target, oldText string) error {
276284
if err := m.facts.Remove(target, oldText); err != nil {
277285
return err
278286
}
287+
m.promptDirty = true
279288
// Re-fit merge detector
280289
if m.cfg.MergeOnWrite != nil && *m.cfg.MergeOnWrite {
281290
entries, _ := m.facts.Entries(target)
@@ -350,6 +359,7 @@ func (m *MemoryManager) AppendBuffer(role, message string) {
350359
}
351360
line := FormatBufferLine(role, message)
352361
m.buffer.Append(line)
362+
m.promptDirty = true
353363
}
354364

355365
// GetBuffer returns the current buffer lines (for system prompt injection).
@@ -369,11 +379,13 @@ func (m *MemoryManager) RestoreBuffer(lines []string) {
369379
for _, line := range lines {
370380
m.buffer.Append(line)
371381
}
382+
m.promptDirty = true
372383
}
373384

374385
// ClearBuffer resets the buffer for a new session.
375386
func (m *MemoryManager) ClearBuffer() {
376387
m.buffer.Clear()
388+
m.promptDirty = true
377389
}
378390

379391
// ── Episode Operations ───────────────────────────────────────────────
@@ -427,6 +439,11 @@ func (m *MemoryManager) BuildSystemPrompt() string {
427439
return ""
428440
}
429441

442+
// Return cached prompt if memory hasn't changed since last build.
443+
if !m.promptDirty && m.promptCache != "" {
444+
return m.promptCache
445+
}
446+
430447
userFact, _ := m.facts.Read("user")
431448
envFact, _ := m.facts.Read("env")
432449
bufferLines := m.GetBuffer()
@@ -497,7 +514,9 @@ func (m *MemoryManager) BuildSystemPrompt() string {
497514
}
498515

499516
b.WriteString("───────────────────────────────\n")
500-
return b.String()
517+
m.promptCache = b.String()
518+
m.promptDirty = false
519+
return m.promptCache
501520
}
502521

503522
// ── Private helpers ──────────────────────────────────────────────────

internal/memory/memory_test.go

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -484,3 +484,76 @@ func TestMin(t *testing.T) {
484484
t.Errorf("min(0, 0) = %d, want 0", got)
485485
}
486486
}
487+
488+
// TestMemoryPromptCache verifies that BuildSystemPrompt returns a cached
489+
// result when memory hasn't changed, and invalidates the cache on mutation.
490+
func TestMemoryPromptCache(t *testing.T) {
491+
dir := t.TempDir()
492+
mm := NewMemoryManager(dir, nil, DefaultMemoryConfig())
493+
494+
// First call builds the prompt and caches it.
495+
mm.AddFact("user", "User prefers Go")
496+
p1 := mm.BuildSystemPrompt()
497+
if !strings.Contains(p1, "User prefers Go") {
498+
t.Fatal("expected fact in initial prompt")
499+
}
500+
501+
// Cached result — same call returns identical prompt.
502+
p1b := mm.BuildSystemPrompt()
503+
if p1b != p1 {
504+
t.Error("prompt should be cached when no mutation occurred")
505+
}
506+
507+
// Add a DIFFERENT fact — should invalidate cache.
508+
mm.AddFact("user", "User also likes Python")
509+
p2 := mm.BuildSystemPrompt()
510+
if p2 == p1 {
511+
t.Error("prompt should differ after AddFact with new content")
512+
}
513+
if !strings.Contains(p2, "User also likes Python") {
514+
t.Errorf("expected new fact in prompt, got %q", p2)
515+
}
516+
517+
// AppendBuffer — should invalidate.
518+
mm.AppendBuffer("user", "buffer entry")
519+
p3 := mm.BuildSystemPrompt()
520+
if p3 == p2 {
521+
t.Error("prompt should differ after AppendBuffer")
522+
}
523+
524+
// ReplaceFact — should invalidate.
525+
mm.ReplaceFact("user", "Go", "User prefers Rust")
526+
p4 := mm.BuildSystemPrompt()
527+
if p4 == p3 {
528+
t.Error("prompt should differ after ReplaceFact")
529+
}
530+
if !strings.Contains(p4, "Rust") {
531+
t.Errorf("expected replaced fact in prompt, got %q", p4)
532+
}
533+
if strings.Contains(p4, "User prefers Go") {
534+
t.Error("old fact should not appear after ReplaceFact")
535+
}
536+
537+
// RemoveFact — should invalidate.
538+
mm.RemoveFact("user", "Python")
539+
p5 := mm.BuildSystemPrompt()
540+
if p5 == p4 {
541+
t.Error("prompt should differ after RemoveFact")
542+
}
543+
if strings.Contains(p5, "Python") {
544+
t.Error("removed fact should not appear in prompt")
545+
}
546+
547+
// Cached after no mutation.
548+
p5b := mm.BuildSystemPrompt()
549+
if p5b != p5 {
550+
t.Error("prompt should be cached after no mutation")
551+
}
552+
553+
// ClearBuffer — should invalidate.
554+
mm.ClearBuffer()
555+
p6 := mm.BuildSystemPrompt()
556+
if p6 == p5b {
557+
t.Error("prompt should differ after ClearBuffer")
558+
}
559+
}

0 commit comments

Comments
 (0)