From 354d82cf1f2bd827bae38319099035c3cc84bfda Mon Sep 17 00:00:00 2001 From: Lobsterdog Contributors Date: Mon, 11 May 2026 02:05:33 -0600 Subject: [PATCH 1/5] ll-k3pro: add REM phase proposed-changes.md writer with skill patches - Add paths.GetProposedChangesPath() mirroring GetDreamDiaryPath() - Add DreamerConfig.ProposedChangesPath + Dreamer.proposedChangesPath struct field - Add config.DreamConfig.ProposedChangesPath with yaml tag and mapping - Implement WriteProposedChanges method on Dreamer (append-only, mutex-protected) - Implement buildSkillPatchPrompt for LLM-generated [SKILL PATCH] sections - Extend Run() to call WriteProposedChanges after WriteDiary when REM has insights - Extend extractBehavioralInsights to add proposed_changes_link metadata to procedure memories - Add fallback skill patch content when LLM is unavailable - Add all 10 required tests for proposed-changes functionality --- internal/config/config.go | 4 +- internal/dream/dream.go | 284 +++++++++++++++++++++ internal/dream/dream_test.go | 469 +++++++++++++++++++++++++++++++++++ internal/paths/paths.go | 5 + internal/paths/paths_test.go | 18 ++ 5 files changed, 779 insertions(+), 1 deletion(-) diff --git a/internal/config/config.go b/internal/config/config.go index da7cef1..c5b99bd 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -42,7 +42,8 @@ type DreamConfig struct { Model string `yaml:"model"` // ModelTimeout is the timeout for each LLM call during REM behavioral insight generation. // Parsed as a Go duration string (e.g. "5m", "120s"). Defaults to "5m". - ModelTimeout string `yaml:"model_timeout"` + ModelTimeout string `yaml:"model_timeout"` + ProposedChangesPath string `yaml:"proposed_changes_path"` } // SessionConfig holds session lifecycle settings. @@ -239,6 +240,7 @@ func (c *Config) DreamerConfig() dream.DreamerConfig { StaleProcedureDays: c.Dream.StaleProcedureDays, DiaryPath: c.Dream.DiaryPath, ReportPath: c.Dream.ReportPath, + ProposedChangesPath: c.Dream.ProposedChangesPath, BaseURL: ollamaURL, Model: model, ModelTimeout: modelTimeout, diff --git a/internal/dream/dream.go b/internal/dream/dream.go index b9cdf16..21277a0 100644 --- a/internal/dream/dream.go +++ b/internal/dream/dream.go @@ -6,6 +6,7 @@ import ( "context" "fmt" "log/slog" + "maps" "net/http" "os" "path/filepath" @@ -87,6 +88,9 @@ type DreamerConfig struct { // ReportPath path for writing dream report. Defaults from paths.GetDreamReportPath(). ReportPath string + // ProposedChangesPath path for writing proposed-changes.md. Defaults from paths.GetProposedChangesPath(). + ProposedChangesPath string + // OllamaClient is an optional pre-configured OllamaClient. Takes precedence over BaseURL/HTTPClient. // When nil, the constructor will attempt to create one from BaseURL. OllamaClient *ollama.OllamaClient @@ -163,6 +167,7 @@ type Dreamer struct { staleProcedureDays int diaryPath string reportPath string + proposedChangesPath string ollama *ollama.OllamaClient model string modelTimeout time.Duration @@ -237,6 +242,10 @@ func NewDreamer(cfg DreamerConfig) (*Dreamer, error) { if reportPath == "" { reportPath = paths.GetDreamReportPath() } + proposedChangesPath := cfg.ProposedChangesPath + if proposedChangesPath == "" { + proposedChangesPath = paths.GetProposedChangesPath() + } model := cfg.Model if model == "" { model = defaultDreamModel @@ -286,6 +295,7 @@ func NewDreamer(cfg DreamerConfig) (*Dreamer, error) { staleProcedureDays: staleProcedureDays, diaryPath: diaryPath, reportPath: reportPath, + proposedChangesPath: proposedChangesPath, ollama: client, model: model, modelTimeout: modelTimeout, @@ -321,6 +331,13 @@ func (d *Dreamer) Run(ctx context.Context, apply bool, phase string) (*DreamResu } } + // Write proposed-changes.md if applied and REM phase has behavioral insights + if apply && result.Rem != nil && len(result.Rem.BehavioralInsights) > 0 { + if err := d.WriteProposedChanges(result); err != nil { + slog.Warn("llmem: dream: failed to write proposed changes", "error", err) + } + } + return result, nil } @@ -670,6 +687,18 @@ func (d *Dreamer) extractBehavioralInsights(ctx context.Context, apply bool) []B slog.Debug("llmem: dream: failed to store REM insight", "error", err) } else { insightID = id + // Link the procedure memory to its proposed-changes.md entry + metadata, getErr := d.store.Get(ctx, id, false) + if getErr == nil && metadata != nil { + mergedMetadata := maps.Clone(metadata.Metadata) + if mergedMetadata == nil { + mergedMetadata = map[string]any{} + } + mergedMetadata["proposed_changes_link"] = d.proposedChangesPath + if _, updateErr := d.store.Update(ctx, store.UpdateParams{ID: id, Metadata: mergedMetadata}); updateErr != nil { + slog.Debug("llmem: dream: failed to update proposed_changes_link metadata", "id", id, "error", updateErr) + } + } } } insights = append(insights, BehavioralInsight{ @@ -724,6 +753,41 @@ func joinSamples(samples []string) string { return strings.Join(samples, "; ") } +// buildSkillPatchPrompt builds an LLM prompt for generating a [SKILL PATCH] section +// for the proposed-changes.md file. It does NOT call the LLM — it only constructs +// the prompt string. +// Specific to the proposed-changes.md format; not reusable outside the dream package. +func buildSkillPatchPrompt(category string, count int, lookbackDays int, samples []string) string { + description, ok := taxonomy.ErrorTaxonomy[category] + if !ok { + description = category + } + + var sb strings.Builder + sb.WriteString("You are a software engineering coach. Based on the following recurring error pattern found in self-assessments, generate a structured [SKILL PATCH] that can be added to a developer's skill file.\n\n") + sb.WriteString(fmt.Sprintf("Category: %s\n", category)) + sb.WriteString(fmt.Sprintf("Definition: %s\n", description)) + sb.WriteString(fmt.Sprintf("Occurrences in the last %d days: %d\n\n", lookbackDays, count)) + + if len(samples) > 0 { + sb.WriteString("Representative self-assessment examples:\n") + for i, s := range samples { + sb.WriteString(fmt.Sprintf("%d. %s\n", i+1, s)) + } + sb.WriteString("\n") + } + + sb.WriteString("Generate a [SKILL PATCH] section with these exact fields:\n\n") + sb.WriteString("**Detection Rule:** When is this pattern likely occurring? (Be specific about the code context or situation.)\n\n") + sb.WriteString("**Checklist:**\n") + sb.WriteString("- [ ] (list 2-4 concrete checks the developer should perform)\n\n") + sb.WriteString("**Pitfall:** What's the most common mistake when trying to fix this category of error?\n\n") + sb.WriteString("**Verification:** How can the developer confirm the fix is working? (Be specific — what to check, what command to run, what metric to observe.)\n\n") + sb.WriteString("Keep the total response under 300 words. Be specific and practical.\n") + + return sb.String() +} + // WriteDiary writes the dream diary as markdown to the configured path. // Uses sync.Mutex for concurrency safety within the process. func (d *Dreamer) WriteDiary(result *DreamResult) error { @@ -792,6 +856,226 @@ func (d *Dreamer) WriteDiary(result *DreamResult) error { return nil } +// WriteProposedChanges writes behavioral insight and skill patch sections to +// d.proposedChangesPath. The file is append-only within a dream run: existing +// content is preserved and new content is appended after a timestamp header. +// Uses sync.Mutex for concurrency safety within the process. +// Only writes when result.Rem has behavioral insights. +// Specific to Dreamer; not reusable outside the dream package. +func (d *Dreamer) WriteProposedChanges(result *DreamResult) error { + d.mu.Lock() + defer d.mu.Unlock() + + if result.Rem == nil || len(result.Rem.BehavioralInsights) == 0 { + return nil + } + + // Validate write path + resolvedPath, err := paths.ValidateWritePath(d.proposedChangesPath, "proposed changes") + if err != nil { + return fmtErr("proposed changes: validate path: %w", err) + } + + dir := filepath.Dir(resolvedPath) + if err := os.MkdirAll(dir, 0700); err != nil { + return fmtErr("proposed changes: create directory: %w", err) + } + + // Read existing content if file exists (append-only) + var existing []byte + if data, readErr := os.ReadFile(resolvedPath); readErr == nil { + existing = data + } + + var sb strings.Builder + + // Add a newline separator if the existing content doesn't end with one + if len(existing) > 0 && existing[len(existing)-1] != '\n' { + sb.Write(existing) + sb.WriteByte('\n') + } else { + sb.Write(existing) + } + + // Write dream run timestamp header + sb.WriteString(fmt.Sprintf("# Dream Run: %s\n\n", time.Now().UTC().Format(time.RFC3339))) + + // Build content for each behavioral insight + // Determine if we should try to get LLM-generated SKILL PATCH content + useLLM := false + if d.ollama != nil { + availCtx, availCancel := context.WithTimeout(context.Background(), 5*time.Second) + useLLM = d.ollama.IsAvailable(availCtx) + availCancel() + } + + for _, insight := range result.Rem.BehavioralInsights { + // Get category samples for this insight from the DB + cutoff := time.Now().UTC().AddDate(0, 0, -d.behavioralLookbackDays).Format(time.RFC3339) + selfAssessments, _ := d.store.Search(context.Background(), store.SearchParams{ + Type: "self_assessment", + ValidOnly: true, + Limit: 500, + }) + categorySamples := []string{} + for _, m := range selfAssessments { + if m.UpdatedAt != "" && m.UpdatedAt >= cutoff { + if strings.Contains(m.Content, "Category: "+insight.Category) { + snippet := m.Content + if len(snippet) > maxSampleLength { + snippet = snippet[:maxSampleLength] + } + if len(categorySamples) < maxBehavioralSamples { + categorySamples = append(categorySamples, snippet) + } + } + } + } + + sb.WriteString(fmt.Sprintf("## %s (×%d)\n\n", insight.Category, insight.Count)) + sb.WriteString(fmt.Sprintf("**Category:** %s\n", insight.Category)) + sb.WriteString(fmt.Sprintf("**Occurrences in the last %d days:** %d\n", d.behavioralLookbackDays, insight.Count)) + + if len(categorySamples) > 0 { + sb.WriteString("**Representative examples:**\n") + for i, s := range categorySamples { + sb.WriteString(fmt.Sprintf("%d. %s\n", i+1, s)) + } + } + sb.WriteString("\n") + + sb.WriteString("### Behavioral Directive\n\n") + + // Generate Do and Verify from the LLM or use fallback + if useLLM { + prompt := buildBehavioralInsightPrompt(insight.Category, insight.Count, d.behavioralLookbackDays, categorySamples) + llmCtx, llmCancel := context.WithTimeout(context.Background(), defaultDreamModelTimeout) + response, llmErr := d.ollama.Generate(llmCtx, prompt, d.model) + llmCancel() + if llmErr != nil { + slog.Error("llmem: dream: proposed changes behavioral insight LLM call failed, using fallback", "category", insight.Category, "error", llmErr) + } + if response != "" { + sb.WriteString("**Do:**\n") + // Parse Do and Verify from the LLM response + doLines, verifyLine := parseLLMDirectiveResponse(response) + for _, line := range doLines { + sb.WriteString(fmt.Sprintf("- %s\n", line)) + } + sb.WriteString(fmt.Sprintf("\n**Verify:** %s\n\n", verifyLine)) + } else { + // Fallback + sb.WriteString(fmt.Sprintf("**Do:**\n- Review %s patterns in recent work\n- Apply taxonomy-defined checks for %s\n\n", insight.Category, insight.Category)) + sb.WriteString(fmt.Sprintf("**Verify:** Run `llmem search %s --type self_assessment` to confirm reduction\n\n", insight.Category)) + } + } else { + sb.WriteString(fmt.Sprintf("**Do:**\n- Review %s patterns in recent work\n- Apply taxonomy-defined checks for %s\n\n", insight.Category, insight.Category)) + sb.WriteString(fmt.Sprintf("**Verify:** Run `llmem search %s --type self_assessment` to confirm reduction\n\n", insight.Category)) + } + + sb.WriteString("### [SKILL PATCH]\n\n") + + if useLLM { + skillPrompt := buildSkillPatchPrompt(insight.Category, insight.Count, d.behavioralLookbackDays, categorySamples) + skillCtx, skillCancel := context.WithTimeout(context.Background(), defaultDreamModelTimeout) + skillResponse, skillErr := d.ollama.Generate(skillCtx, skillPrompt, d.model) + skillCancel() + if skillErr != nil { + slog.Error("llmem: dream: proposed changes skill patch LLM call failed, using fallback", "category", insight.Category, "error", skillErr) + } + if skillResponse != "" { + sb.WriteString(skillResponse + "\n\n") + } else { + // Simplified fallback [SKILL PATCH] + writeFallbackSkillPatch(&sb, insight.Category, insight.Count, d.behavioralLookbackDays) + } + } else { + writeFallbackSkillPatch(&sb, insight.Category, insight.Count, d.behavioralLookbackDays) + } + } + + if err := os.WriteFile(resolvedPath, []byte(sb.String()), 0600); err != nil { + return fmtErr("proposed changes: write: %w", err) + } + + return nil +} + +// parseLLMDirectiveResponse extracts Do lines and Verify text from an LLM response. +// Returns doLines (behavioral directives) and verifyLine (verification step). +func parseLLMDirectiveResponse(response string) ([]string, string) { + var doLines []string + var verifyLine string + + lines := strings.Split(response, "\n") + var inDoSection bool + for _, line := range lines { + trimmed := strings.TrimSpace(line) + if strings.HasPrefix(trimmed, "Do") || strings.HasPrefix(trimmed, "**Do") { + inDoSection = true + continue + } + if strings.HasPrefix(trimmed, "Verify") || strings.HasPrefix(trimmed, "**Verify") { + inDoSection = false + // Extract verify content from same or next line + afterColon := extractAfterColon(trimmed) + if afterColon != "" { + verifyLine = afterColon + } + continue + } + if inDoSection && (strings.HasPrefix(trimmed, "-") || strings.HasPrefix(trimmed, "*") || strings.HasPrefix(trimmed, "1.") || strings.HasPrefix(trimmed, "2.") || strings.HasPrefix(trimmed, "3.") || strings.HasPrefix(trimmed, "4.") || strings.HasPrefix(trimmed, "5.")) { + cleaned := strings.TrimPrefix(trimmed, "- ") + cleaned = strings.TrimPrefix(cleaned, "* ") + cleaned = strings.TrimPrefix(cleaned, "1. ") + cleaned = strings.TrimPrefix(cleaned, "2. ") + cleaned = strings.TrimPrefix(cleaned, "3. ") + cleaned = strings.TrimPrefix(cleaned, "4. ") + cleaned = strings.TrimPrefix(cleaned, "5. ") + if cleaned != "" { + doLines = append(doLines, cleaned) + } + } else if !inDoSection && verifyLine == "" && trimmed != "" { + // If we didn't find a Verify: prefix, look for the line after "Verify" + // that could be the verification content + } + } + + if len(doLines) == 0 { + doLines = []string{fmt.Sprintf("Apply %s checks from taxonomy", "category")} + } + if verifyLine == "" { + verifyLine = "Run llmem search to confirm reduction" + } + + return doLines, verifyLine +} + +// extractAfterColon returns the text after ": " in s, or "" if not found. +func extractAfterColon(s string) string { + idx := strings.Index(s, ": ") + if idx < 0 { + return "" + } + return strings.TrimSpace(s[idx+2:]) +} + +// writeFallbackSkillPatch writes a simplified [SKILL PATCH] section when the +// LLM is unavailable. Uses the category's taxonomy definition for content. +func writeFallbackSkillPatch(sb *strings.Builder, category string, count, lookbackDays int) { + description, ok := taxonomy.ErrorTaxonomy[category] + if !ok { + description = category + } + sb.WriteString(fmt.Sprintf("**Detection Rule:** When working with code that may involve %s — specifically: %s\n\n", category, description)) + sb.WriteString("**Checklist:**\n") + sb.WriteString(fmt.Sprintf("- [ ] Check for %s patterns before committing\n", category)) + sb.WriteString(fmt.Sprintf("- [ ] Verify %s handling is present and correct\n", category)) + sb.WriteString("\n") + sb.WriteString(fmt.Sprintf("**Pitfall:** %s errors often appear fixed but recur due to incomplete handling\n\n", category)) + sb.WriteString(fmt.Sprintf("**Verification:** Run `llmem search %s --type self_assessment` and confirm occurrence rate drops below %d in the next %d days\n", category, count, lookbackDays)) +} + // GenerateDreamReport generates an HTML dream report at the given path. // Validates reportPath via paths.ValidateWritePath. func (d *Dreamer) GenerateDreamReport(result *DreamResult, reportPath string) error { diff --git a/internal/dream/dream_test.go b/internal/dream/dream_test.go index 2f84e80..569d61b 100644 --- a/internal/dream/dream_test.go +++ b/internal/dream/dream_test.go @@ -12,6 +12,7 @@ import ( "time" "github.com/MichielDean/LLMem/internal/ollama" + "github.com/MichielDean/LLMem/internal/paths" "github.com/MichielDean/LLMem/internal/store" ) @@ -1251,4 +1252,472 @@ func TestExtractBehavioralInsights_LogsSkipped_WhenOllamaUnavailable(t *testing. } } t.Error("expected ERROR_HANDLING insight not found") +} + +// TestDreamer_WriteProposedChanges_AppendsNewSections verifies that WriteProposedChanges +// appends behavioral insight sections with category/occurrence count, Do directives, +// Verify step, and [SKILL PATCH] with all four fields. +func TestDreamer_WriteProposedChanges_AppendsNewSections(t *testing.T) { + ms := newTestStore(t) + dir := t.TempDir() + proposedChangesPath := filepath.Join(dir, "proposed-changes.md") + + // Add self_assessment memories to exceed the threshold + for i := 0; i < 4; i++ { + addSelfAssessment(t, ms, "ERROR_HANDLING", "Missing error handling in HTTP call "+strings.Repeat("x", 20)) + } + + // Use a server that's not available, so we get count-based fallback + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.NotFound(w, r) + })) + defer server.Close() + + d, err := NewDreamer(DreamerConfig{ + Store: ms, + OllamaClient: newTestOllamaClient(t, server), + ProposedChangesPath: proposedChangesPath, + }) + if err != nil { + t.Fatalf("NewDreamer: %v", err) + } + + result, err := d.Run(context.Background(), true, "rem") + if err != nil { + t.Fatalf("Run: %v", err) + } + if result.Rem == nil || len(result.Rem.BehavioralInsights) == 0 { + t.Fatal("expected at least one behavioral insight") + } + + // Write proposed changes + err = d.WriteProposedChanges(result) + if err != nil { + t.Fatalf("WriteProposedChanges: %v", err) + } + + data, err := os.ReadFile(proposedChangesPath) + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + content := string(data) + + // Verify timestamp header + if !contains(content, "# Dream Run:") { + t.Error("expected 'Dream Run:' timestamp header in proposed-changes.md") + } + // Verify category header + if !contains(content, "## ERROR_HANDLING") { + t.Error("expected '## ERROR_HANDLING' section in proposed-changes.md") + } + // Verify occurrence count + if !contains(content, "×4") { + t.Error("expected '×4' occurrence count in proposed-changes.md") + } + // Verify behavioral directive section + if !contains(content, "### Behavioral Directive") { + t.Error("expected '### Behavioral Directive' section in proposed-changes.md") + } + // Verify Do directive + if !contains(content, "**Do:**") { + t.Error("expected '**Do:**' in proposed-changes.md") + } + // Verify Verify step + if !contains(content, "**Verify:**") { + t.Error("expected '**Verify:**' in proposed-changes.md") + } + // Verify SKILL PATCH section + if !contains(content, "### [SKILL PATCH]") { + t.Error("expected '### [SKILL PATCH]' section in proposed-changes.md") + } + // Verify SKILL PATCH fields + if !contains(content, "**Detection Rule:**") { + t.Error("expected '**Detection Rule:**' in proposed-changes.md") + } + if !contains(content, "**Checklist:**") { + t.Error("expected '**Checklist:**' in proposed-changes.md") + } + if !contains(content, "**Pitfall:**") { + t.Error("expected '**Pitfall:**' in proposed-changes.md") + } + if !contains(content, "**Verification:**") { + t.Error("expected '**Verification:**' in proposed-changes.md") + } +} + +// TestDreamer_WriteProposedChanges_PreservesExistingContent verifies that +// existing content in proposed-changes.md is preserved when appending. +func TestDreamer_WriteProposedChanges_PreservesExistingContent(t *testing.T) { + ms := newTestStore(t) + dir := t.TempDir() + proposedChangesPath := filepath.Join(dir, "proposed-changes.md") + + // Write initial content + initialContent := "# Previous Dream Run\n\nOld content here.\n" + if err := os.WriteFile(proposedChangesPath, []byte(initialContent), 0600); err != nil { + t.Fatalf("WriteFile initial: %v", err) + } + + // Add self_assessment memories to exceed the threshold + for i := 0; i < 3; i++ { + addSelfAssessment(t, ms, "NULL_SAFETY", "Missing null check in function "+strings.Repeat("z", 10)) + } + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.NotFound(w, r) + })) + defer server.Close() + + d, err := NewDreamer(DreamerConfig{ + Store: ms, + OllamaClient: newTestOllamaClient(t, server), + ProposedChangesPath: proposedChangesPath, + }) + if err != nil { + t.Fatalf("NewDreamer: %v", err) + } + + result, err := d.Run(context.Background(), true, "rem") + if err != nil { + t.Fatalf("Run: %v", err) + } + + err = d.WriteProposedChanges(result) + if err != nil { + t.Fatalf("WriteProposedChanges: %v", err) + } + + data, err := os.ReadFile(proposedChangesPath) + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + content := string(data) + + // Verify old content still present + if !contains(content, "Old content here.") { + t.Error("expected existing content to be preserved in proposed-changes.md") + } + // Verify new content appended + if !contains(content, "# Dream Run:") { + t.Error("expected new dream run timestamp header") + } +} + +// TestDreamer_WriteProposedChanges_UsesResolvedPath verifies that WriteProposedChanges +// validates the write path via paths.ValidateWritePath. +func TestDreamer_WriteProposedChanges_UsesResolvedPath(t *testing.T) { + ms := newTestStore(t) + dir := t.TempDir() + proposedChangesPath := filepath.Join(dir, "proposed-changes.md") + + // Use a mock server that returns 404 for /api/tags (unavailable) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.NotFound(w, r) + })) + defer server.Close() + + d, err := NewDreamer(DreamerConfig{ + Store: ms, + OllamaClient: newTestOllamaClient(t, server), + ProposedChangesPath: proposedChangesPath, + }) + if err != nil { + t.Fatalf("NewDreamer: %v", err) + } + + result := &DreamResult{ + Rem: &RemPhaseResult{ + BehavioralInsights: []BehavioralInsight{ + {Category: "ERROR_HANDLING", Count: 3, ContentSnippet: "test"}, + }, + }, + } + + err = d.WriteProposedChanges(result) + if err != nil { + t.Fatalf("WriteProposedChanges: %v", err) + } + + // Verify file was written at the expected path + if _, err := os.Stat(proposedChangesPath); err != nil { + t.Errorf("expected proposed changes file at %s: %v", proposedChangesPath, err) + } +} + +// TestDreamer_WriteProposedChanges_MetadataLink verifies that procedure memories +// created during REM have proposed_changes_link metadata. +func TestDreamer_WriteProposedChanges_MetadataLink(t *testing.T) { + ms := newTestStore(t) + dir := t.TempDir() + proposedChangesPath := filepath.Join(dir, "proposed-changes.md") + + // Add self_assessment memories to exceed the threshold + for i := 0; i < 3; i++ { + addSelfAssessment(t, ms, "ERROR_HANDLING", "Error handling gap "+strings.Repeat("a", 10)) + } + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.NotFound(w, r) + })) + defer server.Close() + + d, err := NewDreamer(DreamerConfig{ + Store: ms, + OllamaClient: newTestOllamaClient(t, server), + ProposedChangesPath: proposedChangesPath, + }) + if err != nil { + t.Fatalf("NewDreamer: %v", err) + } + + result, err := d.Run(context.Background(), true, "rem") + if err != nil { + t.Fatalf("Run: %v", err) + } + + insights := result.Rem.BehavioralInsights + if len(insights) == 0 { + t.Fatal("expected at least one behavioral insight") + } + + // Find the ERROR_HANDLING insight + var insightID string + for _, insight := range insights { + if insight.Category == "ERROR_HANDLING" { + insightID = insight.InsightID + break + } + } + if insightID == "" { + t.Fatal("expected ERROR_HANDLING insight with non-empty InsightID") + } + + // Retrieve the stored procedure memory and verify proposed_changes_link metadata + mem, err := ms.Get(context.Background(), insightID, false) + if err != nil { + t.Fatalf("Get stored procedure: %v", err) + } + + link, ok := mem.Metadata["proposed_changes_link"] + if !ok { + t.Error("expected proposed_changes_link key in procedure memory metadata") + } + if link != proposedChangesPath { + t.Errorf("expected proposed_changes_link=%q, got %q", proposedChangesPath, link) + } +} + +// TestDreamer_WriteProposedChanges_DryRunDoesNotWrite verifies that no file +// is written when apply=false (dry run). +func TestDreamer_WriteProposedChanges_DryRunDoesNotWrite(t *testing.T) { + ms := newTestStore(t) + dir := t.TempDir() + proposedChangesPath := filepath.Join(dir, "proposed-changes.md") + + d, err := NewDreamer(DreamerConfig{ + Store: ms, + ProposedChangesPath: proposedChangesPath, + }) + if err != nil { + t.Fatalf("NewDreamer: %v", err) + } + + // Run with apply=false (dry run) + _, err = d.Run(context.Background(), false, "") + if err != nil { + t.Fatalf("Run: %v", err) + } + + // Verify no file exists at the proposed changes path + if _, err := os.Stat(proposedChangesPath); !os.IsNotExist(err) { + t.Error("expected no proposed-changes.md file during dry run") + } +} + +// TestDreamer_WriteProposedChanges_NoInsightsNoFile verifies that no file +// is created when there are no behavioral insights above the threshold. +func TestDreamer_WriteProposedChanges_NoInsightsNoFile(t *testing.T) { + ms := newTestStore(t) + dir := t.TempDir() + proposedChangesPath := filepath.Join(dir, "proposed-changes.md") + + // Add only 1 self_assessment (below threshold of 3) + addSelfAssessment(t, ms, "ERROR_HANDLING", "A single error handling entry") + + d, err := NewDreamer(DreamerConfig{ + Store: ms, + ProposedChangesPath: proposedChangesPath, + }) + if err != nil { + t.Fatalf("NewDreamer: %v", err) + } + + result, err := d.Run(context.Background(), true, "") + if err != nil { + t.Fatalf("Run: %v", err) + } + + // WriteProposedChanges should be a no-op when no insights + err = d.WriteProposedChanges(result) + if err != nil { + t.Fatalf("WriteProposedChanges: %v", err) + } + + // Verify no file created + if _, err := os.Stat(proposedChangesPath); !os.IsNotExist(err) { + t.Error("expected no proposed-changes.md file when there are no insights") + } +} + +// TestDreamer_WriteProposedChanges_AppendOnlyWithinRun verifies that running +// WriteProposedChanges twice appends both runs' content. +func TestDreamer_WriteProposedChanges_AppendOnlyWithinRun(t *testing.T) { + ms := newTestStore(t) + dir := t.TempDir() + proposedChangesPath := filepath.Join(dir, "proposed-changes.md") + + // Use a mock server that returns 404 (unavailable), so we get fallback content + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.NotFound(w, r) + })) + defer server.Close() + + d, err := NewDreamer(DreamerConfig{ + Store: ms, + OllamaClient: newTestOllamaClient(t, server), + ProposedChangesPath: proposedChangesPath, + }) + if err != nil { + t.Fatalf("NewDreamer: %v", err) + } + + result := &DreamResult{ + Rem: &RemPhaseResult{ + BehavioralInsights: []BehavioralInsight{ + {Category: "ERROR_HANDLING", Count: 3, ContentSnippet: "first run"}, + }, + }, + } + + // First write + err = d.WriteProposedChanges(result) + if err != nil { + t.Fatalf("WriteProposedChanges first: %v", err) + } + + // Second write + result2 := &DreamResult{ + Rem: &RemPhaseResult{ + BehavioralInsights: []BehavioralInsight{ + {Category: "RACE_CONDITION", Count: 5, ContentSnippet: "second run"}, + }, + }, + } + err = d.WriteProposedChanges(result2) + if err != nil { + t.Fatalf("WriteProposedChanges second: %v", err) + } + + data, err := os.ReadFile(proposedChangesPath) + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + content := string(data) + + // Should have two Dream Run headers + dreamRunCount := strings.Count(content, "# Dream Run:") + if dreamRunCount != 2 { + t.Errorf("expected 2 Dream Run headers, got %d", dreamRunCount) + } + + // Should have both categories + if !contains(content, "ERROR_HANDLING") { + t.Error("expected ERROR_HANDLING from first run") + } + if !contains(content, "RACE_CONDITION") { + t.Error("expected RACE_CONDITION from second run") + } +} + +// TestBuildSkillPatchPrompt verifies that the prompt builder includes +// Detection Rule, Checklist, Pitfall, and Verification fields, +// category name, and occurrence count. +func TestBuildSkillPatchPrompt(t *testing.T) { + category := "ERROR_HANDLING" + count := 7 + samples := []string{"Missing try/except in HTTP call", "Swallowed error in database query"} + + prompt := buildSkillPatchPrompt(category, count, 30, samples) + + // Verify category name + if !contains(prompt, "ERROR_HANDLING") { + t.Error("expected prompt to contain category name") + } + // Verify occurrence count + if !contains(prompt, "7") { + t.Error("expected prompt to contain occurrence count") + } + // Verify lookback days + if !contains(prompt, "last 30 days") { + t.Error("expected prompt to contain 'last 30 days'") + } + // Verify Detection Rule field + if !contains(prompt, "Detection Rule") { + t.Error("expected prompt to contain 'Detection Rule'") + } + // Verify Checklist field + if !contains(prompt, "Checklist") { + t.Error("expected prompt to contain 'Checklist'") + } + // Verify Pitfall field + if !contains(prompt, "Pitfall") { + t.Error("expected prompt to contain 'Pitfall'") + } + // Verify Verification field + if !contains(prompt, "Verification") { + t.Error("expected prompt to contain 'Verification'") + } + // Verify samples + for _, s := range samples { + if !contains(prompt, s) { + t.Errorf("expected prompt to contain sample %q", s) + } + } + // Verify taxonomy description + if !contains(prompt, "Missing try/except") { + t.Error("expected prompt to contain taxonomy description for ERROR_HANDLING") + } +} + +// TestDreamerConfig_ProposedChangesPath_Default verifies that when +// DreamerConfig.ProposedChangesPath is empty, the constructor defaults +// to paths.GetProposedChangesPath(). +func TestDreamerConfig_ProposedChangesPath_Default(t *testing.T) { + ms := newTestStore(t) + d, err := NewDreamer(DreamerConfig{ + Store: ms, + // ProposedChangesPath left empty — should default + }) + if err != nil { + t.Fatalf("NewDreamer: %v", err) + } + + expected := paths.GetProposedChangesPath() + if d.proposedChangesPath != expected { + t.Errorf("expected proposedChangesPath=%q, got %q", expected, d.proposedChangesPath) + } + + // Verify explicit value is used as-is + dir := t.TempDir() + explicitPath := filepath.Join(dir, "my-proposed-changes.md") + d2, err := NewDreamer(DreamerConfig{ + Store: ms, + ProposedChangesPath: explicitPath, + }) + if err != nil { + t.Fatalf("NewDreamer with explicit path: %v", err) + } + if d2.proposedChangesPath != explicitPath { + t.Errorf("expected proposedChangesPath=%q, got %q", explicitPath, d2.proposedChangesPath) + } } \ No newline at end of file diff --git a/internal/paths/paths.go b/internal/paths/paths.go index af318c4..9e4420f 100644 --- a/internal/paths/paths.go +++ b/internal/paths/paths.go @@ -102,6 +102,11 @@ func GetDreamReportPath() string { return filepath.Join(GetHomeDir(), "dream_report.html") } +// GetProposedChangesPath returns the path to the proposed-changes file (~/.config/llmem/proposed-changes.md). +func GetProposedChangesPath() string { + return filepath.Join(GetHomeDir(), "proposed-changes.md") +} + // GetContextDir returns the path to the context directory (~/.config/llmem/context/). func GetContextDir() string { return filepath.Join(GetHomeDir(), "context") diff --git a/internal/paths/paths_test.go b/internal/paths/paths_test.go index 664f2d1..3f80cb4 100644 --- a/internal/paths/paths_test.go +++ b/internal/paths/paths_test.go @@ -3,6 +3,7 @@ package paths import ( "os" "path/filepath" + "strings" "testing" ) @@ -189,6 +190,23 @@ func TestGetDreamReportPath(t *testing.T) { } } +func TestGetProposedChangesPath(t *testing.T) { + // Test 1: resolves to ~/.config/llmem/proposed-changes.md by default + path := GetProposedChangesPath() + if !strings.HasSuffix(path, "proposed-changes.md") { + t.Errorf("expected path to end with 'proposed-changes.md', got %q", path) + } + + // Test 2: respects LLMEM_HOME env var + dir := t.TempDir() + t.Setenv("LMEM_HOME", dir) + path = GetProposedChangesPath() + expected := filepath.Join(dir, "proposed-changes.md") + if path != expected { + t.Errorf("expected %q, got %q", expected, path) + } +} + func TestGetContextDir(t *testing.T) { dir := t.TempDir() t.Setenv("LMEM_HOME", dir) From fed125afde5629934cddb38941238355bfa8f8ee Mon Sep 17 00:00:00 2001 From: Lobsterdog Contributors Date: Mon, 11 May 2026 02:21:12 -0600 Subject: [PATCH 2/5] ll-k3pro: fix 3 reviewer issues in REM phase proposed-changes writer 1. Remove O(N) redundant DB queries in WriteProposedChanges by carrying categorySamples on BehavioralInsight struct (populated in extractBehavioralInsights). WriteProposedChanges now uses insight.Samples instead of re-querying the DB for each insight. 2. Fix parseLLMDirectiveResponse fallback using string literal 'category' instead of the actual category name. Added category parameter so the fallback correctly uses e.g. 'ERROR_HANDLING' instead of literal 'category'. 3. Fix silently discarded d.store.Search error in WriteProposedChanges by removing the redundant query entirely (issue 1 fix eliminates the problem at the source). The only Search for self_assessments is now in extractBehavioralInsights where errors ARE logged. Added tests: - TestParseLLMDirectiveResponse_FallbackUsesCategory - TestParseLLMDirectiveResponse_ExtractsDoLines - TestDreamer_RemPhase_BehavioralInsight_SamplesPopulated - TestDreamer_WriteProposedChanges_UsesSamplesFromInsight --- internal/dream/dream.go | 31 ++---- internal/dream/dream_test.go | 178 +++++++++++++++++++++++++++++++++++ 2 files changed, 185 insertions(+), 24 deletions(-) diff --git a/internal/dream/dream.go b/internal/dream/dream.go index 21277a0..548e02a 100644 --- a/internal/dream/dream.go +++ b/internal/dream/dream.go @@ -134,6 +134,7 @@ type BehavioralInsight struct { Count int InsightID string ContentSnippet string + Samples []string } // RemPhaseResult holds the results of the REM (reflect) phase. @@ -706,6 +707,7 @@ func (d *Dreamer) extractBehavioralInsights(ctx context.Context, apply bool) []B Count: count, InsightID: insightID, ContentSnippet: contentSnippet, + Samples: categorySamples[cat], }) } } @@ -910,27 +912,7 @@ func (d *Dreamer) WriteProposedChanges(result *DreamResult) error { } for _, insight := range result.Rem.BehavioralInsights { - // Get category samples for this insight from the DB - cutoff := time.Now().UTC().AddDate(0, 0, -d.behavioralLookbackDays).Format(time.RFC3339) - selfAssessments, _ := d.store.Search(context.Background(), store.SearchParams{ - Type: "self_assessment", - ValidOnly: true, - Limit: 500, - }) - categorySamples := []string{} - for _, m := range selfAssessments { - if m.UpdatedAt != "" && m.UpdatedAt >= cutoff { - if strings.Contains(m.Content, "Category: "+insight.Category) { - snippet := m.Content - if len(snippet) > maxSampleLength { - snippet = snippet[:maxSampleLength] - } - if len(categorySamples) < maxBehavioralSamples { - categorySamples = append(categorySamples, snippet) - } - } - } - } + categorySamples := insight.Samples sb.WriteString(fmt.Sprintf("## %s (×%d)\n\n", insight.Category, insight.Count)) sb.WriteString(fmt.Sprintf("**Category:** %s\n", insight.Category)) @@ -958,7 +940,7 @@ func (d *Dreamer) WriteProposedChanges(result *DreamResult) error { if response != "" { sb.WriteString("**Do:**\n") // Parse Do and Verify from the LLM response - doLines, verifyLine := parseLLMDirectiveResponse(response) + doLines, verifyLine := parseLLMDirectiveResponse(response, insight.Category) for _, line := range doLines { sb.WriteString(fmt.Sprintf("- %s\n", line)) } @@ -1003,7 +985,8 @@ func (d *Dreamer) WriteProposedChanges(result *DreamResult) error { // parseLLMDirectiveResponse extracts Do lines and Verify text from an LLM response. // Returns doLines (behavioral directives) and verifyLine (verification step). -func parseLLMDirectiveResponse(response string) ([]string, string) { +// category is used in the fallback when no Do lines are parsed. +func parseLLMDirectiveResponse(response string, category string) ([]string, string) { var doLines []string var verifyLine string @@ -1042,7 +1025,7 @@ func parseLLMDirectiveResponse(response string) ([]string, string) { } if len(doLines) == 0 { - doLines = []string{fmt.Sprintf("Apply %s checks from taxonomy", "category")} + doLines = []string{fmt.Sprintf("Apply %s checks from taxonomy", category)} } if verifyLine == "" { verifyLine = "Run llmem search to confirm reduction" diff --git a/internal/dream/dream_test.go b/internal/dream/dream_test.go index 569d61b..a23faf7 100644 --- a/internal/dream/dream_test.go +++ b/internal/dream/dream_test.go @@ -1720,4 +1720,182 @@ func TestDreamerConfig_ProposedChangesPath_Default(t *testing.T) { if d2.proposedChangesPath != explicitPath { t.Errorf("expected proposedChangesPath=%q, got %q", explicitPath, d2.proposedChangesPath) } +} + +// TestParseLLMDirectiveResponse_FallbackUsesCategory verifies that when +// parseLLMDirectiveResponse cannot extract Do lines from the LLM response, +// it uses the actual category name in the fallback, not the literal string "category". +func TestParseLLMDirectiveResponse_FallbackUsesCategory(t *testing.T) { + // Empty response should trigger fallback with the category name + doLines, _ := parseLLMDirectiveResponse("", "ERROR_HANDLING") + if len(doLines) != 1 { + t.Fatalf("expected 1 fallback do line, got %d", len(doLines)) + } + if !strings.Contains(doLines[0], "ERROR_HANDLING") { + t.Errorf("expected fallback to contain 'ERROR_HANDLING', got %q", doLines[0]) + } + if !strings.Contains(doLines[0], "checks from taxonomy") { + t.Errorf("expected fallback to contain 'checks from taxonomy', got %q", doLines[0]) + } + + // Response with no recognizable Do/Verify lines should also use category fallback + doLines2, verifyLine2 := parseLLMDirectiveResponse("Some random text without structure", "RACE_CONDITION") + if !strings.Contains(doLines2[0], "RACE_CONDITION") { + t.Errorf("expected fallback to contain 'RACE_CONDITION', got %q", doLines2[0]) + } + // Verify fallback verify line is also sensible + if verifyLine2 == "" { + t.Error("expected non-empty fallback verify line") + } +} + +// TestParseLLMDirectiveResponse_ExtractsDoLines verifies that parseLLMDirectiveResponse +// correctly extracts Do directives and Verify steps from a well-formed LLM response. +func TestParseLLMDirectiveResponse_ExtractsDoLines(t *testing.T) { + response := `**Do:** +- Always check return values from external calls +- Wrap database operations in try/except blocks +- Log errors with context (function name, args) + +**Verify:** Run integration tests with mock failures to confirm error paths execute.` + + doLines, verifyLine := parseLLMDirectiveResponse(response, "ERROR_HANDLING") + + if len(doLines) != 3 { + t.Errorf("expected 3 do lines, got %d: %v", len(doLines), doLines) + } + if len(doLines) > 0 && !strings.Contains(doLines[0], "Always check return values") { + t.Errorf("expected first do line about return values, got %q", doLines[0]) + } + if len(doLines) > 2 && !strings.Contains(doLines[2], "Log errors with context") { + t.Errorf("expected third do line about logging, got %q", doLines[2]) + } + // Verify line should be extracted from the **Verify:** line + // Note: extractAfterColon finds the first ": " — in "**Verify:** Run...", + // the ": " after "Verify" is followed by "**", so extractAfterColon may not + // extract correctly for this format. If it fails to extract, the fallback is used. + if verifyLine == "" { + t.Error("expected non-empty verify line") + } +} + +// TestDreamer_RemPhase_BehavioralInsight_SamplesPopulated verifies that +// extractBehavioralInsights populates the Samples field on BehavioralInsight, +// so that WriteProposedChanges does not need to re-query the database. +func TestDreamer_RemPhase_BehavioralInsight_SamplesPopulated(t *testing.T) { + ms := newTestStore(t) + + // Add self_assessment memories with ERROR_HANDLING category (need >= 3) + for i := 0; i < 4; i++ { + addSelfAssessment(t, ms, "ERROR_HANDLING", "Missing error handling in HTTP call "+strings.Repeat("x", 20)) + } + + // Use a server that returns 404 for /api/tags (unavailable) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.NotFound(w, r) + })) + defer server.Close() + + d, err := NewDreamer(DreamerConfig{ + Store: ms, + OllamaClient: newTestOllamaClient(t, server), + }) + if err != nil { + t.Fatalf("NewDreamer: %v", err) + } + + result, err := d.Run(context.Background(), true, "rem") + if err != nil { + t.Fatalf("Run rem: %v", err) + } + if result.Rem == nil { + t.Fatal("expected Rem result") + } + + insights := result.Rem.BehavioralInsights + if len(insights) == 0 { + t.Fatal("expected at least one behavioral insight") + } + + // Find ERROR_HANDLING insight and verify Samples is populated + var found bool + for _, insight := range insights { + if insight.Category == "ERROR_HANDLING" { + found = true + if len(insight.Samples) == 0 { + t.Error("expected Samples to be populated on BehavioralInsight, but got empty slice") + } + // Each sample should contain the category name + for _, s := range insight.Samples { + if !strings.Contains(s, "ERROR_HANDLING") { + t.Errorf("expected sample to contain 'ERROR_HANDLING', got %q", s) + } + } + break + } + } + if !found { + t.Error("expected ERROR_HANDLING insight not found") + } +} + +// TestDreamer_WriteProposedChanges_UsesSamplesFromInsight verifies that +// WriteProposedChanges uses the Samples field from BehavioralInsight, not +// a separate DB query. This is a regression test for the O(N) redundant +// DB queries that were previously made inside the insight loop. +func TestDreamer_WriteProposedChanges_UsesSamplesFromInsight(t *testing.T) { + ms := newTestStore(t) + dir := t.TempDir() + proposedChangesPath := filepath.Join(dir, "proposed-changes.md") + + // Use an unavailable server (fallback behavior) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.NotFound(w, r) + })) + defer server.Close() + + // Create a Dreamer with a specific OllamaClient that's unavailable + ollamaClient := newTestOllamaClient(t, server) + + d, err := NewDreamer(DreamerConfig{ + Store: ms, + OllamaClient: ollamaClient, + ProposedChangesPath: proposedChangesPath, + }) + if err != nil { + t.Fatalf("NewDreamer: %v", err) + } + + // Create a result with a BehavioralInsight that has Samples populated + result := &DreamResult{ + Rem: &RemPhaseResult{ + BehavioralInsights: []BehavioralInsight{ + { + Category: "ERROR_HANDLING", + Count: 5, + ContentSnippet: "test snippet", + Samples: []string{"Error in HTTP handler", "Missing try/except block"}, + }, + }, + }, + } + + err = d.WriteProposedChanges(result) + if err != nil { + t.Fatalf("WriteProposedChanges: %v", err) + } + + data, err := os.ReadFile(proposedChangesPath) + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + content := string(data) + + // Verify that the samples from the BehavioralInsight appear in the file + if !contains(content, "Error in HTTP handler") { + t.Error("expected 'Error in HTTP handler' sample from insight.Samples in proposed-changes.md") + } + if !contains(content, "Missing try/except block") { + t.Error("expected 'Missing try/except block' sample from insight.Samples in proposed-changes.md") + } } \ No newline at end of file From e5189ab5ff924357766a9b7f2bbc9e1f189eefcd Mon Sep 17 00:00:00 2001 From: Lobsterdog Contributors Date: Mon, 11 May 2026 02:44:52 -0600 Subject: [PATCH 3/5] ll-k3pro: fix parseLLMDirectiveResponse Verify extraction from **Verify:** markdown format The parseLLMDirectiveResponse function could not extract Verify content from the **Verify:** markdown bold format because extractAfterColon looks for ': ' (colon-space), but **Verify:** has the colon followed by '**' not a space. Fix: Strip ** markers before extracting content after colon, so '**Verify:** Run tests' becomes 'Verify: Run tests' which extractAfterColon can parse. Also handle Verify on a separate line after the header. Also removed the dead-code else-if branch (lines 1008-1011) that had an empty body and was intended to capture verify content but did nothing. Added table-driven test TestParseLLMDirectiveResponse_ExtractsVerifyFromBoldFormat covering bold/inline, plain/colon-space, combined bold Do+Verify, and Verify on own line after header. Updated TestParseLLMDirectiveResponse_ExtractsDoLines to assert the actual Verify content is extracted (not just that verifyLine is non-empty). --- internal/dream/dream.go | 27 ++++++++-- internal/dream/dream_test.go | 96 ++++++++++++++++++++++++++++++++++-- 2 files changed, 114 insertions(+), 9 deletions(-) diff --git a/internal/dream/dream.go b/internal/dream/dream.go index 548e02a..b238c56 100644 --- a/internal/dream/dream.go +++ b/internal/dream/dream.go @@ -989,24 +989,44 @@ func (d *Dreamer) WriteProposedChanges(result *DreamResult) error { func parseLLMDirectiveResponse(response string, category string) ([]string, string) { var doLines []string var verifyLine string + var pastVerifyHeader bool lines := strings.Split(response, "\n") var inDoSection bool for _, line := range lines { trimmed := strings.TrimSpace(line) + + // Detect Do section header (handles both "Do:" and "**Do:**" formats) if strings.HasPrefix(trimmed, "Do") || strings.HasPrefix(trimmed, "**Do") { inDoSection = true + pastVerifyHeader = false continue } + + // Detect Verify section header (handles both "Verify:" and "**Verify:**" formats) if strings.HasPrefix(trimmed, "Verify") || strings.HasPrefix(trimmed, "**Verify") { inDoSection = false - // Extract verify content from same or next line - afterColon := extractAfterColon(trimmed) + // Strip markdown bold markers (**) so "**Verify:** text" becomes "Verify: text" + // before extracting content after the colon. + stripped := strings.ReplaceAll(trimmed, "**", "") + afterColon := extractAfterColon(stripped) if afterColon != "" { verifyLine = afterColon + pastVerifyHeader = false + } else { + // Verify header has no inline content; next non-empty line is the verify text + pastVerifyHeader = true } continue } + + // If we just saw a bare Verify header, the next non-empty line is the verify content + if pastVerifyHeader && trimmed != "" { + verifyLine = trimmed + pastVerifyHeader = false + continue + } + if inDoSection && (strings.HasPrefix(trimmed, "-") || strings.HasPrefix(trimmed, "*") || strings.HasPrefix(trimmed, "1.") || strings.HasPrefix(trimmed, "2.") || strings.HasPrefix(trimmed, "3.") || strings.HasPrefix(trimmed, "4.") || strings.HasPrefix(trimmed, "5.")) { cleaned := strings.TrimPrefix(trimmed, "- ") cleaned = strings.TrimPrefix(cleaned, "* ") @@ -1018,9 +1038,6 @@ func parseLLMDirectiveResponse(response string, category string) ([]string, stri if cleaned != "" { doLines = append(doLines, cleaned) } - } else if !inDoSection && verifyLine == "" && trimmed != "" { - // If we didn't find a Verify: prefix, look for the line after "Verify" - // that could be the verification content } } diff --git a/internal/dream/dream_test.go b/internal/dream/dream_test.go index a23faf7..1760269 100644 --- a/internal/dream/dream_test.go +++ b/internal/dream/dream_test.go @@ -1770,13 +1770,101 @@ func TestParseLLMDirectiveResponse_ExtractsDoLines(t *testing.T) { if len(doLines) > 2 && !strings.Contains(doLines[2], "Log errors with context") { t.Errorf("expected third do line about logging, got %q", doLines[2]) } - // Verify line should be extracted from the **Verify:** line - // Note: extractAfterColon finds the first ": " — in "**Verify:** Run...", - // the ": " after "Verify" is followed by "**", so extractAfterColon may not - // extract correctly for this format. If it fails to extract, the fallback is used. + // Verify line must be extracted from the **Verify:** line — not replaced by fallback. + // The markdown bold format **Verify:** has no colon-space pattern for extractAfterColon, + // so the parser must handle stripping the markdown bold markers. if verifyLine == "" { t.Error("expected non-empty verify line") } + if !strings.Contains(verifyLine, "integration tests") { + t.Errorf("expected verify line to contain 'integration tests', got %q (likely fallback used instead of extracted content)", verifyLine) + } +} + +// TestParseLLMDirectiveResponse_ExtractsVerifyFromBoldFormat verifies that +// parseLLMDirectiveResponse correctly extracts Verify text from markdown bold +// format (e.g., **Verify:** some text). The extractAfterColon helper cannot +// find ": " in "**Verify:**" because the colon is followed by "**", so the +// parser must strip bold markers before extracting. +func TestParseLLMDirectiveResponse_ExtractsVerifyFromBoldFormat(t *testing.T) { + tests := []struct { + name string + response string + category string + wantDo []string // substrings that must appear in do lines + wantDoN int // expected number of do lines + wantVer string // substring that must appear in verify line + }{ + { + name: "bold Verify with inline text", + response: `**Do:** +- Always check return values from external calls + +**Verify:** Run integration tests with mock failures to confirm error paths execute.`, + category: "ERROR_HANDLING", + wantDoN: 1, + wantDo: []string{"Always check return values"}, + wantVer: "integration tests", + }, + { + name: "non-bold Verify with colon-space", + response: `Do: +- Wrap every external call in try/except + +Verify: Run llmem search ERROR_HANDLING to confirm rate drops.`, + category: "ERROR_HANDLING", + wantDoN: 1, + wantDo: []string{"Wrap every external call"}, + wantVer: "llmem search", + }, + { + name: "bold Do and bold Verify", + response: `**Do:** +- Check error return values +- Add defensive nil checks +- Log errors with context + +**Verify:** Run automated tests covering error paths.`, + category: "NIL_POINTER", + wantDoN: 3, + wantDo: []string{"Check error return values", "Add defensive nil checks", "Log errors with context"}, + wantVer: "automated tests", + }, + { + name: "Verify on own line after bold header", + response: `**Do:** +- Review recent changes for missing error handling + +**Verify:** +Run llmem search ERROR_HANDLING to confirm reduction in occurrences.`, + category: "ERROR_HANDLING", + wantDoN: 1, + wantDo: []string{"Review recent changes"}, + wantVer: "llmem search", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + doLines, verifyLine := parseLLMDirectiveResponse(tt.response, tt.category) + + if len(doLines) != tt.wantDoN { + t.Errorf("expected %d do lines, got %d: %v", tt.wantDoN, len(doLines), doLines) + } + for i, want := range tt.wantDo { + if i >= len(doLines) { + t.Errorf("missing do line %d: expected to contain %q", i, want) + continue + } + if !strings.Contains(doLines[i], want) { + t.Errorf("do line %d: expected to contain %q, got %q", i, want, doLines[i]) + } + } + if !strings.Contains(verifyLine, tt.wantVer) { + t.Errorf("expected verify line to contain %q, got %q", tt.wantVer, verifyLine) + } + }) + } } // TestDreamer_RemPhase_BehavioralInsight_SamplesPopulated verifies that From 312f6102c572b89fb257270244a9ad6309554f12 Mon Sep 17 00:00:00 2001 From: Lobsterdog Contributors Date: Mon, 11 May 2026 03:12:03 -0600 Subject: [PATCH 4/5] ll-k3pro: fix WriteProposedChanges ctx propagation and eliminate Get+Clone+Update pattern - Add ctx context.Context parameter to WriteProposedChanges, propagate to all context.WithTimeout calls instead of context.Background(). This enables cancellation on shutdown (reviewer issue ll-k3pro-ak7mo). - Include proposed_changes_link in initial AddParams metadata instead of using separate Get+Clone+Update pattern, eliminating 3 redundant DB ops per insight and the silent error path (reviewer issue ll-k3pro-dw4du). - Remove unused 'maps' import after eliminating maps.Clone. - Update all test calls to pass context.Background(). - Add TestDreamer_WriteProposedChanges_AcceptsContext contract test. --- internal/dream/dream.go | 25 ++++----------- internal/dream/dream_test.go | 62 ++++++++++++++++++++++++++++++++---- 2 files changed, 61 insertions(+), 26 deletions(-) diff --git a/internal/dream/dream.go b/internal/dream/dream.go index b238c56..a15be6e 100644 --- a/internal/dream/dream.go +++ b/internal/dream/dream.go @@ -6,7 +6,6 @@ import ( "context" "fmt" "log/slog" - "maps" "net/http" "os" "path/filepath" @@ -334,7 +333,7 @@ func (d *Dreamer) Run(ctx context.Context, apply bool, phase string) (*DreamResu // Write proposed-changes.md if applied and REM phase has behavioral insights if apply && result.Rem != nil && len(result.Rem.BehavioralInsights) > 0 { - if err := d.WriteProposedChanges(result); err != nil { + if err := d.WriteProposedChanges(ctx, result); err != nil { slog.Warn("llmem: dream: failed to write proposed changes", "error", err) } } @@ -682,24 +681,12 @@ func (d *Dreamer) extractBehavioralInsights(ctx context.Context, apply bool) []B Content: contentSnippet, Source: "dream_rem", Confidence: 0.7, - Metadata: map[string]any{"proposed": true, "source": "dream_rem", "category": cat, "occurrences": count}, + Metadata: map[string]any{"proposed": true, "source": "dream_rem", "category": cat, "occurrences": count, "proposed_changes_link": d.proposedChangesPath}, }) if err != nil { slog.Debug("llmem: dream: failed to store REM insight", "error", err) } else { insightID = id - // Link the procedure memory to its proposed-changes.md entry - metadata, getErr := d.store.Get(ctx, id, false) - if getErr == nil && metadata != nil { - mergedMetadata := maps.Clone(metadata.Metadata) - if mergedMetadata == nil { - mergedMetadata = map[string]any{} - } - mergedMetadata["proposed_changes_link"] = d.proposedChangesPath - if _, updateErr := d.store.Update(ctx, store.UpdateParams{ID: id, Metadata: mergedMetadata}); updateErr != nil { - slog.Debug("llmem: dream: failed to update proposed_changes_link metadata", "id", id, "error", updateErr) - } - } } } insights = append(insights, BehavioralInsight{ @@ -864,7 +851,7 @@ func (d *Dreamer) WriteDiary(result *DreamResult) error { // Uses sync.Mutex for concurrency safety within the process. // Only writes when result.Rem has behavioral insights. // Specific to Dreamer; not reusable outside the dream package. -func (d *Dreamer) WriteProposedChanges(result *DreamResult) error { +func (d *Dreamer) WriteProposedChanges(ctx context.Context, result *DreamResult) error { d.mu.Lock() defer d.mu.Unlock() @@ -906,7 +893,7 @@ func (d *Dreamer) WriteProposedChanges(result *DreamResult) error { // Determine if we should try to get LLM-generated SKILL PATCH content useLLM := false if d.ollama != nil { - availCtx, availCancel := context.WithTimeout(context.Background(), 5*time.Second) + availCtx, availCancel := context.WithTimeout(ctx, 5*time.Second) useLLM = d.ollama.IsAvailable(availCtx) availCancel() } @@ -931,7 +918,7 @@ func (d *Dreamer) WriteProposedChanges(result *DreamResult) error { // Generate Do and Verify from the LLM or use fallback if useLLM { prompt := buildBehavioralInsightPrompt(insight.Category, insight.Count, d.behavioralLookbackDays, categorySamples) - llmCtx, llmCancel := context.WithTimeout(context.Background(), defaultDreamModelTimeout) + llmCtx, llmCancel := context.WithTimeout(ctx, defaultDreamModelTimeout) response, llmErr := d.ollama.Generate(llmCtx, prompt, d.model) llmCancel() if llmErr != nil { @@ -959,7 +946,7 @@ func (d *Dreamer) WriteProposedChanges(result *DreamResult) error { if useLLM { skillPrompt := buildSkillPatchPrompt(insight.Category, insight.Count, d.behavioralLookbackDays, categorySamples) - skillCtx, skillCancel := context.WithTimeout(context.Background(), defaultDreamModelTimeout) + skillCtx, skillCancel := context.WithTimeout(ctx, defaultDreamModelTimeout) skillResponse, skillErr := d.ollama.Generate(skillCtx, skillPrompt, d.model) skillCancel() if skillErr != nil { diff --git a/internal/dream/dream_test.go b/internal/dream/dream_test.go index 1760269..089ea8c 100644 --- a/internal/dream/dream_test.go +++ b/internal/dream/dream_test.go @@ -1291,7 +1291,7 @@ func TestDreamer_WriteProposedChanges_AppendsNewSections(t *testing.T) { } // Write proposed changes - err = d.WriteProposedChanges(result) + err = d.WriteProposedChanges(context.Background(), result) if err != nil { t.Fatalf("WriteProposedChanges: %v", err) } @@ -1382,7 +1382,7 @@ func TestDreamer_WriteProposedChanges_PreservesExistingContent(t *testing.T) { t.Fatalf("Run: %v", err) } - err = d.WriteProposedChanges(result) + err = d.WriteProposedChanges(context.Background(), result) if err != nil { t.Fatalf("WriteProposedChanges: %v", err) } @@ -1433,7 +1433,7 @@ func TestDreamer_WriteProposedChanges_UsesResolvedPath(t *testing.T) { }, } - err = d.WriteProposedChanges(result) + err = d.WriteProposedChanges(context.Background(), result) if err != nil { t.Fatalf("WriteProposedChanges: %v", err) } @@ -1558,7 +1558,7 @@ func TestDreamer_WriteProposedChanges_NoInsightsNoFile(t *testing.T) { } // WriteProposedChanges should be a no-op when no insights - err = d.WriteProposedChanges(result) + err = d.WriteProposedChanges(context.Background(), result) if err != nil { t.Fatalf("WriteProposedChanges: %v", err) } @@ -1600,7 +1600,7 @@ func TestDreamer_WriteProposedChanges_AppendOnlyWithinRun(t *testing.T) { } // First write - err = d.WriteProposedChanges(result) + err = d.WriteProposedChanges(context.Background(), result) if err != nil { t.Fatalf("WriteProposedChanges first: %v", err) } @@ -1613,7 +1613,7 @@ func TestDreamer_WriteProposedChanges_AppendOnlyWithinRun(t *testing.T) { }, }, } - err = d.WriteProposedChanges(result2) + err = d.WriteProposedChanges(context.Background(), result2) if err != nil { t.Fatalf("WriteProposedChanges second: %v", err) } @@ -1639,6 +1639,54 @@ func TestDreamer_WriteProposedChanges_AppendOnlyWithinRun(t *testing.T) { } } +// TestDreamer_WriteProposedChanges_AcceptsContext verifies that +// WriteProposedChanges accepts a context.Context parameter and propagates it +// to LLM calls, enabling cancellation on shutdown. Without LLM (fallback path), +// the function completes normally even with a valid context. +func TestDreamer_WriteProposedChanges_AcceptsContext(t *testing.T) { + ms := newTestStore(t) + dir := t.TempDir() + proposedChangesPath := filepath.Join(dir, "proposed-changes.md") + + // Use a server that returns 404 (unavailable), so we get fallback content quickly + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.NotFound(w, r) + })) + defer server.Close() + + d, err := NewDreamer(DreamerConfig{ + Store: ms, + OllamaClient: newTestOllamaClient(t, server), + ProposedChangesPath: proposedChangesPath, + }) + if err != nil { + t.Fatalf("NewDreamer: %v", err) + } + + result := &DreamResult{ + Rem: &RemPhaseResult{ + BehavioralInsights: []BehavioralInsight{ + {Category: "ERROR_HANDLING", Count: 3, ContentSnippet: "test"}, + }, + }, + } + + // Verify the function accepts a context parameter — this is the contract test. + // Using context.Background() ensures the context is valid. + err = d.WriteProposedChanges(context.Background(), result) + if err != nil { + t.Fatalf("WriteProposedChanges with context.Background(): %v", err) + } + + data, err := os.ReadFile(proposedChangesPath) + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + if !contains(string(data), "ERROR_HANDLING") { + t.Error("expected ERROR_HANDLING in proposed-changes.md") + } +} + // TestBuildSkillPatchPrompt verifies that the prompt builder includes // Detection Rule, Checklist, Pitfall, and Verification fields, // category name, and occurrence count. @@ -1968,7 +2016,7 @@ func TestDreamer_WriteProposedChanges_UsesSamplesFromInsight(t *testing.T) { }, } - err = d.WriteProposedChanges(result) + err = d.WriteProposedChanges(context.Background(), result) if err != nil { t.Fatalf("WriteProposedChanges: %v", err) } From 44b2cf0bfcd849de34777535722aea857301643e Mon Sep 17 00:00:00 2001 From: Lobsterdog Contributors Date: Mon, 11 May 2026 03:41:52 -0600 Subject: [PATCH 5/5] ll-k3pro: document proposed-changes.md feature in CLI, config, and dream docs - CONFIGURATION.md: Add proposed_changes_path to config.yaml reference, remove from unwired-fields list (now wired in Go CLI) - DREAM.md: Update REM phase description (skill patches, proposed-changes.md append-only output, proposed_changes_link metadata), add ProposedChangesPath to DreamerConfig table and code example, add Samples field to BehavioralInsight struct, add WriteProposedChanges to Go API section - CLI.md: Update llmem dream REM phase description with proposed-changes.md output details --- docs/CLI.md | 2 +- docs/CONFIGURATION.md | 3 ++- docs/DREAM.md | 8 +++++++- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/docs/CLI.md b/docs/CLI.md index c2a610c..77c04cd 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -189,7 +189,7 @@ Run the dream consolidation cycle, which performs automated memory maintenance i - **Light phase:** Sort and deduplicate near-duplicate memories (cosine similarity ≥ `dream.similarity_threshold`). - **Deep phase:** Score, promote, decay, and merge memories. Also promotes inbox items to long-term memory (items with attention_score ≥ `dream.min_score` become permanent; lower-scored items are evicted). Decays confidence on idle memories. Boosts frequently accessed memories. Performs LLM-assisted merging of similar pairs. Auto-links memories with high cosine similarity (≥ `dream.auto_link_threshold`, default 0.85). -- **REM phase:** Extract themes from memory clusters and write a dream diary (read-only reflection). When Ollama is available, generates actionable behavioral insights via LLM; falls back to count-based summaries when Ollama is unavailable. +- **REM phase:** Extract themes from memory clusters and write a dream diary (read-only reflection). When Ollama is available, generates actionable behavioral insights via LLM with "Do" directives, "Verify" steps, and `[SKILL PATCH]` sections (Detection Rule, Checklist, Pitfall, Verification); falls back to count-based summaries when Ollama is unavailable. When run with `--apply`, also appends behavioral insight and skill patch sections to `proposed-changes.md` at `~/.config/llmem/proposed-changes.md` (or `LMEM_HOME/proposed-changes.md`). Each dream run's entries are separated by a timestamp header. The file is append-only — existing content is preserved. Without `--apply`, the dream cycle runs as a **dry run** — output is prefixed with `[DRY RUN]` and no changes are written to the database. diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 274d23b..ff0b06c 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -45,6 +45,7 @@ dream: boost_amount: 0.05 diary_path: null # Auto-resolved from GetDreamDiaryPath() report_path: null # Auto-resolved from GetDreamReportPath() + proposed_changes_path: null # Auto-resolved from GetProposedChangesPath() behavioral_threshold: 3 behavioral_lookback_days: 30 ollama_url: http://localhost:11434 # Ollama API URL for LLM-generated behavioral insights @@ -62,7 +63,7 @@ session: debounce_seconds: 30 # Idle debounce interval in seconds ``` -> **Note:** The following fields exist in the Python config but are **not wired** in the Go implementation: `min_score`, `min_recall_count`, `min_unique_queries`, `boost_on_promote`, `merge_model`, `calibration_enabled`, `calibration_lookback_days`, `inbox_capacity`, `correction_detection` (top-level), `copilot` (top-level), and `proposed_changes_path`. Setting these in `config.yaml` has no effect when using the Go CLI. +> **Note:** The following fields exist in the Python config but are **not wired** in the Go implementation: `min_score`, `min_recall_count`, `min_unique_queries`, `boost_on_promote`, `merge_model`, `calibration_enabled`, `calibration_lookback_days`, `inbox_capacity`, `correction_detection` (top-level), and `copilot` (top-level). Setting these in `config.yaml` has no effect when using the Go CLI. > **Note:** The Go config resolves `db_path` for OpenCode as `~/.local/share/opencode/opencode.db` using `filepath.Join` with proper path handling. The database is opened in read-only mode (`mode=ro`) using a `file:` URI prefix to ensure the `modernc.org/sqlite` driver enforces the read-only constraint. Without the `file:` prefix, query parameters like `mode=ro` are silently ignored and the driver opens in read-write mode. diff --git a/docs/DREAM.md b/docs/DREAM.md index add4a2e..fa3676f 100644 --- a/docs/DREAM.md +++ b/docs/DREAM.md @@ -8,7 +8,7 @@ The dream cycle performs automated memory maintenance during idle periods. It ca - **Light phase:** Sort and deduplicate near-duplicate memories (cosine similarity ≥ threshold). - **Deep phase:** Score, promote, decay, and merge memories. Decays confidence on idle memories. Boosts frequently accessed memories. Auto-links memories with high cosine similarity (≥ `dream.auto_link_threshold`, default 0.85) by creating `related_to` relations between them. Procedure memories older than `dream.stale_procedure_days` (default 30 days) with no recent access decay at double the normal rate — proposed-but-never-adopted procedures fade faster than confirmed ones. -- **REM phase:** Extract themes from memory clusters and write a dream diary (read-only reflection). Also extracts behavioral insights (patterns exceeding `dream.behavioral_threshold` occurrences within `dream.behavioral_lookback_days` days). When Ollama is available, uses an LLM call to generate specific, actionable procedural rules with "Do" directives and "Verify" steps. Falls back to count-based summaries when Ollama is unavailable. +- **REM phase:** Extract themes from memory clusters and write a dream diary (read-only reflection). Also extracts behavioral insights (patterns exceeding `dream.behavioral_threshold` occurrences within `dream.behavioral_lookback_days` days). When Ollama is available, uses an LLM call to generate specific, actionable procedural rules with "Do" directives and "Verify" steps; also generates `[SKILL PATCH]` sections (Detection Rule, Checklist, Pitfall, Verification). Falls back to count-based summaries when Ollama is unavailable. When run with `--apply`, appends behavioral insight and skill patch sections to `proposed-changes.md`, with a timestamp header separating dream runs. Proposed procedure memories are linked to their `proposed-changes.md` entry via `proposed_changes_link` metadata. Configuration is under the `dream:` key in `config.yaml`. See [Configuration](CONFIGURATION.md) for all dream settings. @@ -48,6 +48,7 @@ dreamer, err := dream.NewDreamer(dream.DreamerConfig{ OllamaClient: nil, // nil → created from BaseURL; takes precedence if provided DiaryPath: "", // defaults from paths.GetDreamDiaryPath() ReportPath: "", // defaults from paths.GetDreamReportPath() + ProposedChangesPath: "", // defaults from paths.GetProposedChangesPath() }) if err != nil { log.Fatal(err) @@ -62,6 +63,9 @@ result, err := dreamer.Run(ctx, true, "deep") // Write dream diary (markdown with sync.Mutex for in-process concurrency) err = dreamer.WriteDiary(result) +// Write proposed-changes.md (behavioral insights + skill patches, append-only) +err = dreamer.WriteProposedChanges(ctx, result) + // Generate HTML dream report err = dreamer.GenerateDreamReport(result, "/path/to/report.html") ``` @@ -88,6 +92,7 @@ err = dreamer.GenerateDreamReport(result, "/path/to/report.html") | Model | string | `"glm-5.1:cloud"` | Ollama model name for behavioral insight generation | | DiaryPath | string | paths.GetDreamDiaryPath() | Path for dream diary markdown | | ReportPath | string | paths.GetDreamReportPath() | Path for HTML dream report | +| ProposedChangesPath | string | paths.GetProposedChangesPath() | Path for proposed-changes.md (behavioral insights and skill patches) | #### DreamResult @@ -117,6 +122,7 @@ type BehavioralInsight struct { Count int InsightID string ContentSnippet string + Samples []string } type RemPhaseResult struct {