Skip to content

Commit 5934f31

Browse files
committed
fix: skip persistence, auto-save preview, and curation improvements
- Changed default SkipThreshold from 3 to 1 (one skip now suppresses) - Added Body field to SkillEvent for preview in Telegram suggestions - Added suppressSuggested param to learnAndSuggest to fix double-handling when auto-save is enabled (no more Save/Skip buttons + auto-save conflict) - AutoSaveResult now carries Heuristics map for informative messages - WebUI (serve.go) now properly wires up auto-save and skip filtering - Added comprehensive tests: SkipList persistence, PassesQualityGate, AutoSaveSuggestions heuristics, FormatSuggestion preview, FilterSkipped, SkipThreshold defaults, ShouldSkip threshold/expiry/clear Fixes: repeated skill suggestions even after user clicked Skip
1 parent 11914eb commit 5934f31

13 files changed

Lines changed: 866 additions & 81 deletions

File tree

cmd/odek/main.go

Lines changed: 105 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -942,7 +942,7 @@ func run(args []string) error {
942942
if resolved.Skills.Learn && sm != nil {
943943
// Create LLM client for skill enhancement
944944
skillsLLM := llm.New(resolved.BaseURL, resolved.APIKey, resolved.Model, "", 30*time.Second)
945-
runLearnLoop(allMessages, f.Task, sm, skillsLLM, resolved.Skills.LLMLearn)
945+
runLearnLoop(allMessages, f.Task, sm, skillsLLM, resolved.Skills)
946946
}
947947

948948
// ── Session end — extract episode if enough turns ──
@@ -1263,7 +1263,9 @@ func getVersion() string {
12631263
// enhancement, fires "suggested" events via the SkillManager's notifier,
12641264
// and returns the enhanced suggestions for interactive handling by callers.
12651265
// This is the non-interactive core shared by CLI, WebUI, and Telegram.
1266-
func learnAndSuggest(messages []llm.Message, sm *skills.SkillManager, llmClient skills.LLMClient, llmLearn bool) []skills.SkillSuggestion {
1266+
// When suppressSuggested is true, "suggested" notifier events are skipped
1267+
// (caller handles presentation, e.g. when auto-save is enabled).
1268+
func learnAndSuggest(messages []llm.Message, sm *skills.SkillManager, llmClient skills.LLMClient, llmLearn, suppressSuggested bool) []skills.SkillSuggestion {
12671269
// Convert llm.Message to skills.LlmMessage
12681270
skillMsgs := make([]skills.LlmMessage, 0, len(messages))
12691271
for _, m := range messages {
@@ -1298,50 +1300,115 @@ func learnAndSuggest(messages []llm.Message, sm *skills.SkillManager, llmClient
12981300
}
12991301
}
13001302

1301-
// Fire suggested events via notifier
1302-
for _, s := range suggestions {
1303-
sm.Notifier.Notify(skills.SkillEvent{
1304-
Type: "suggested",
1305-
SkillName: s.Name,
1306-
Heuristic: s.Heuristic,
1307-
Timestamp: time.Now().UTC(),
1308-
})
1303+
// Fire suggested events via notifier (unless suppressed)
1304+
if !suppressSuggested {
1305+
for _, s := range suggestions {
1306+
sm.Notifier.Notify(skills.SkillEvent{
1307+
Type: "suggested",
1308+
SkillName: s.Name,
1309+
Heuristic: s.Heuristic,
1310+
Body: s.Body,
1311+
Timestamp: time.Now().UTC(),
1312+
})
1313+
}
13091314
}
13101315

13111316
return suggestions
13121317
}
13131318

1314-
func runLearnLoop(messages []llm.Message, task string, sm *skills.SkillManager, llmClient skills.LLMClient, llmLearn bool) {
1315-
suggestions := learnAndSuggest(messages, sm, llmClient, llmLearn)
1319+
func runLearnLoop(messages []llm.Message, task string, sm *skills.SkillManager, llmClient skills.LLMClient, skillsCfg skills.SkillsConfig) {
1320+
suggestions := learnAndSuggest(messages, sm, llmClient, skillsCfg.LLMLearn, true)
13161321
if len(suggestions) == 0 {
13171322
return
13181323
}
13191324

1320-
fmt.Fprintf(os.Stderr, "\n🔍 Learning: detected %d skill pattern(s)\n", len(suggestions))
1321-
for _, s := range suggestions {
1322-
fmt.Fprint(os.Stderr, skills.FormatSuggestion(s))
1323-
fmt.Fprintf(os.Stderr, " Save as skill? [Y/n]: ")
1325+
userDir := expandHome("~/.odek/skills")
1326+
os.MkdirAll(userDir, 0755)
1327+
1328+
// Filter out previously-skipped suggestions
1329+
filtered, skipped := skills.FilterSkipped(suggestions, userDir,
1330+
skillsCfg.Curation.SkipThreshold, skillsCfg.Curation.SkipResetDays)
1331+
if skipped > 0 {
1332+
fmt.Fprintf(os.Stderr, " (%d suggestion(s) previously skipped, suppressed)\n", skipped)
1333+
}
1334+
if len(filtered) == 0 {
1335+
return
1336+
}
1337+
1338+
// Auto-save if enabled
1339+
if skillsCfg.AutoSave.Enabled {
1340+
if !skillsCfg.AutoSave.RequireLLM || skillsCfg.LLMLearn {
1341+
result := skills.AutoSaveSuggestions(filtered, userDir, skillsCfg)
1342+
for _, name := range result.Saved {
1343+
heuristic := result.Heuristics[name]
1344+
if heuristic != "" {
1345+
fmt.Fprintf(os.Stderr, " ✓ Auto-saved skill %q (%s)\n", name, heuristic)
1346+
} else {
1347+
fmt.Fprintf(os.Stderr, " ✓ Auto-saved skill %q\n", name)
1348+
}
1349+
sm.Notifier.Notify(skills.SkillEvent{
1350+
Type: "saved", SkillName: name, Timestamp: time.Now().UTC(),
1351+
})
1352+
}
1353+
if result.Skipped > 0 {
1354+
fmt.Fprintf(os.Stderr, " (%d previously skipped, suppressed)\n", result.Skipped)
1355+
}
1356+
for _, name := range result.Failed {
1357+
fmt.Fprintf(os.Stderr, " ⚠ Quality gate failed for %q (use --no-auto-save to review manually)\n", name)
1358+
}
1359+
if len(result.Saved) > 0 {
1360+
sm.Reload()
1361+
// Run micro-curation after auto-save
1362+
runMicroCuration(userDir, sm, skillsCfg)
1363+
}
1364+
return
1365+
}
1366+
}
1367+
1368+
// Interactive fallback: show preview and prompt
1369+
fmt.Fprintf(os.Stderr, "\n🔍 Learning: detected %d skill pattern(s)\n", len(filtered))
1370+
for _, s := range filtered {
1371+
fmt.Fprint(os.Stderr, skills.FormatSuggestionWithPreview(s, true, 400))
1372+
fmt.Fprintf(os.Stderr, " Save as skill? [Y/n/s=skip always]: ")
13241373

13251374
var response string
13261375
fmt.Scanf("%s", &response)
13271376
response = strings.ToLower(strings.TrimSpace(response))
13281377

13291378
if response == "" || response == "y" || response == "yes" {
1330-
userDir := expandHome("~/.odek/skills")
1331-
os.MkdirAll(userDir, 0755)
13321379
if err := skills.SaveSuggestion(userDir, s); err != nil {
13331380
fmt.Fprintf(os.Stderr, " ✗ Error saving skill: %v\n", err)
13341381
} else {
13351382
fmt.Fprintf(os.Stderr, " ✓ Saved skill %q\n", s.Name)
1336-
// Reload the skill manager to pick up the new skill
13371383
sm.Reload()
13381384
}
1385+
} else if response == "s" || response == "skip" {
1386+
sl := skills.LoadSkipList(userDir)
1387+
sl.RecordSkip(userDir, s.Name, s.Heuristic)
1388+
fmt.Fprintf(os.Stderr, " Skipped permanently. Use `odek skill reset-skips` to re-enable.\n")
13391389
} else {
1390+
sl := skills.LoadSkipList(userDir)
1391+
sl.RecordSkip(userDir, s.Name, s.Heuristic)
13401392
fmt.Fprintf(os.Stderr, " Skipped.\n")
13411393
}
13421394
}
13431395
}
13441396

1397+
// runMicroCuration triggers micro-curation after auto-save.
1398+
func runMicroCuration(userDir string, sm *skills.SkillManager, cfg skills.SkillsConfig) {
1399+
allSkills := sm.AllSkills()
1400+
var newSkills []skills.Skill
1401+
for _, s := range allSkills {
1402+
if s.Quality == skills.QualityDraft {
1403+
newSkills = append(newSkills, s)
1404+
}
1405+
}
1406+
result := skills.MicroCuration(userDir, newSkills, allSkills, cfg.Curation)
1407+
if msg := skills.FormatMicroCurationResult(result); msg != "" {
1408+
fmt.Fprint(os.Stderr, msg)
1409+
}
1410+
}
1411+
13451412
// extractUserMessages extracts user message content from llm messages.
13461413
func extractUserMessages(messages []llm.Message) []string {
13471414
var out []string
@@ -1353,10 +1420,10 @@ func extractUserMessages(messages []llm.Message) []string {
13531420
return out
13541421
}
13551422

1356-
// skillCmd handles `odek skill <list|view|save|delete|import|curate>`.
1423+
// skillCmd handles `odek skill <list|view|save|delete|import|curate|reset-skips>`.
13571424
func skillCmd(args []string) error {
13581425
if len(args) == 0 {
1359-
fmt.Fprintf(os.Stderr, "Usage: odek skill <list|view|save|delete|import|curate> [args]\n")
1426+
fmt.Fprintf(os.Stderr, "Usage: odek skill <list|view|save|delete|import|curate|reset-skips> [args]\n")
13601427
return nil
13611428
}
13621429

@@ -1501,8 +1568,24 @@ func skillCmd(args []string) error {
15011568
fmt.Print(skills.FormatCurationReport(report))
15021569
return nil
15031570

1571+
case "reset-skips":
1572+
sl := skills.LoadSkipList(userDir)
1573+
if len(subArgs) == 0 {
1574+
if err := sl.ClearAllSkips(userDir); err != nil {
1575+
return fmt.Errorf("reset all skips: %w", err)
1576+
}
1577+
fmt.Println("✓ Cleared all skipped suggestions.")
1578+
} else {
1579+
name := subArgs[0]
1580+
if err := sl.ClearSkip(userDir, name); err != nil {
1581+
return fmt.Errorf("reset skip %q: %w", name, err)
1582+
}
1583+
fmt.Printf("✓ Cleared skip for %q.\n", name)
1584+
}
1585+
return nil
1586+
15041587
default:
1505-
return fmt.Errorf("unknown skill command %q (use list, view, delete, import, curate)", sub)
1588+
return fmt.Errorf("unknown skill command %q (use list, view, delete, import, curate, reset-skips)", sub)
15061589
}
15071590
}
15081591

cmd/odek/main_test.go

Lines changed: 22 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1349,7 +1349,7 @@ func multiTurnServer(t *testing.T, terminalCalls int) *httptest.Server {
13491349

13501350
// TestRunLearn_MultiStepProcedure is an end-to-end test of the
13511351
// --learn pipeline: mock LLM simulates 4 terminal calls → multi-step
1352-
// heuristic fires → user accepts → skill file saved on disk.
1352+
// heuristic fires → skill auto-saved (with default auto_save=true).
13531353
func TestRunLearn_MultiStepProcedure(t *testing.T) {
13541354
server := multiTurnServer(t, 4)
13551355
defer server.Close()
@@ -1367,14 +1367,11 @@ func TestRunLearn_MultiStepProcedure(t *testing.T) {
13671367
os.Setenv("HOME", origHome)
13681368
}()
13691369

1370-
// Simulate stdin: "y" to accept the suggestion
1371-
oldStdin := os.Stdin
1372-
inR, inW, _ := os.Pipe()
1373-
os.Stdin = inR
1374-
defer func() { os.Stdin = oldStdin }()
1375-
go func() {
1376-
inW.Write([]byte("y\n"))
1377-
}()
1370+
// Create local odek.json with auto_save enabled, LLM enhancement disabled
1371+
// (mock server can't handle enhancement prompts)
1372+
configContent := `{"skills": {"auto_save": {"enabled": true, "require_llm": false}, "llm_learn": false}}`
1373+
os.WriteFile("odek.json", []byte(configContent), 0644)
1374+
defer os.Remove("odek.json")
13781375

13791376
// Capture stderr
13801377
oldStderr := os.Stderr
@@ -1391,16 +1388,11 @@ func TestRunLearn_MultiStepProcedure(t *testing.T) {
13911388
}
13921389

13931390
stderrStr := string(errOutput)
1391+
t.Logf("STDERR: %s", stderrStr) // DEBUG
13941392

1395-
// Heuristic fired
1396-
if !strings.Contains(stderrStr, "Learning: detected") {
1397-
t.Error("expected 'Learning: detected' in stderr")
1398-
}
1399-
if !strings.Contains(stderrStr, "Save as skill?") {
1400-
t.Error("expected 'Save as skill?' prompt")
1401-
}
1402-
if !strings.Contains(stderrStr, "Saved skill") {
1403-
t.Error("expected 'Saved skill' confirmation")
1393+
// Auto-save should fire with default config (auto_save.enabled=true)
1394+
if !strings.Contains(stderrStr, "Auto-saved skill") {
1395+
t.Error("expected 'Auto-saved skill' in stderr")
14041396
}
14051397
if !strings.Contains(stderrStr, "multi-step") {
14061398
t.Error("expected 'multi-step' heuristic in output")
@@ -1414,9 +1406,9 @@ func TestRunLearn_MultiStepProcedure(t *testing.T) {
14141406
}
14151407
}
14161408

1417-
// TestRunLearn_RejectSuggestion verifies that when the user declines
1418-
// a skill suggestion, no file is written.
1419-
func TestRunLearn_RejectSuggestion(t *testing.T) {
1409+
// TestRunLearn_InteractiveReject verifies that when auto-save is disabled,
1410+
// the interactive prompt appears and user can reject.
1411+
func TestRunLearn_InteractiveReject(t *testing.T) {
14201412
server := multiTurnServer(t, 4)
14211413
defer server.Close()
14221414

@@ -1433,6 +1425,11 @@ func TestRunLearn_RejectSuggestion(t *testing.T) {
14331425
os.Setenv("HOME", origHome)
14341426
}()
14351427

1428+
// Create local odek.json with auto_save disabled and LLM enhancement disabled
1429+
configContent := `{"skills": {"auto_save": {"enabled": false}, "llm_learn": false}}`
1430+
os.WriteFile("odek.json", []byte(configContent), 0644)
1431+
defer os.Remove("odek.json")
1432+
14361433
// Simulate stdin: "n" to reject
14371434
oldStdin := os.Stdin
14381435
inR, inW, _ := os.Pipe()
@@ -1456,16 +1453,14 @@ func TestRunLearn_RejectSuggestion(t *testing.T) {
14561453
}
14571454

14581455
stderrStr := string(errOutput)
1456+
t.Logf("STDERR: %s", stderrStr) // DEBUG
14591457

14601458
if !strings.Contains(stderrStr, "Learning: detected") {
1461-
t.Error("expected detection to fire")
1459+
t.Error("expected 'Learning: detected' in stderr")
14621460
}
14631461
if !strings.Contains(stderrStr, "Skipped") {
14641462
t.Error("expected 'Skipped' when user rejects")
14651463
}
1466-
if strings.Contains(stderrStr, "Saved skill") {
1467-
t.Error("should NOT contain 'Saved skill' when rejected")
1468-
}
14691464

14701465
// Verify no skill file written
14711466
skillDir := filepath.Join(homeDir, ".odek", "skills", "procedure-echo")
@@ -1515,8 +1510,8 @@ func TestRunLearn_NoSuggestions(t *testing.T) {
15151510
if strings.Contains(stderrStr, "Learning: detected") {
15161511
t.Error("should NOT detect learning patterns for text-only response")
15171512
}
1518-
if strings.Contains(stderrStr, "Save as skill?") {
1519-
t.Error("should NOT show save prompt when no patterns detected")
1513+
if strings.Contains(stderrStr, "Auto-saved") {
1514+
t.Error("should NOT auto-save when no patterns detected")
15201515
}
15211516
}
15221517

cmd/odek/serve.go

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -566,7 +566,25 @@ func handlePrompt(
566566
// ── Learn loop: run self-improvement heuristics ──
567567
if agent.SkillManager() != nil {
568568
sm := agent.SkillManager()
569-
learnAndSuggest(allMessages, sm, nil, false)
569+
suggestions := learnAndSuggest(allMessages, sm, nil, false, resolved.Skills.AutoSave.Enabled)
570+
if len(suggestions) > 0 {
571+
userDir := expandHome("~/.odek/skills")
572+
os.MkdirAll(userDir, 0755)
573+
filtered, skipped := skills.FilterSkipped(suggestions, userDir,
574+
resolved.Skills.Curation.SkipThreshold, resolved.Skills.Curation.SkipResetDays)
575+
_ = skipped
576+
if resolved.Skills.AutoSave.Enabled {
577+
result := skills.AutoSaveSuggestions(filtered, userDir, resolved.Skills)
578+
for _, name := range result.Saved {
579+
sm.Notifier.Notify(skills.SkillEvent{
580+
Type: "saved", SkillName: name, Timestamp: time.Now().UTC(),
581+
})
582+
}
583+
if len(result.Saved) > 0 {
584+
sm.Reload()
585+
}
586+
}
587+
}
570588
}
571589

572590
// If we started a new session, return it so the WebSocket loop

0 commit comments

Comments
 (0)