feat(memory): make effectiveness ranking and three fields reachable

internal/cmd/cmd_test.go

@@ -385,3 +385,166 @@ func TestIntegration_FactStalenessComesFromConfig(t *testing.T) {
 		t.Errorf("stale_after = %v, want created+7d = %v", fm.StaleAfter, want)
 	}
 }
+
+// A decision is only reopenable if the reader can see what would reopen it.
+// The rules brAIn ships tell a model to "reopen one only when its revisit
+// condition has actually been met", so the condition has to reach every path
+// a consumer reads: the frontmatter, the JSON, and the text.
+func TestRemember_revisitIfReachesEveryReader(t *testing.T) {
+	dir := setupBrainDir(t)
+
+	if code, out := run(t, dir, "remember", "camelCase for API responses",
+		"--domain", "api", "--layer", "decision", "--revisit-if", "GraphQL adoption"); code != 0 {
+		t.Fatalf("remember: exit %d, out=%s", code, out)
+	}
+
+	t.Run("frontmatter", func(t *testing.T) {
+		matches, err := filepath.Glob(filepath.Join(dir, "decisions", "*.md"))
+		if err != nil || len(matches) != 1 {
+			t.Fatalf("want one decision file, got %v (err=%v)", matches, err)
+		}
+		raw, err := os.ReadFile(matches[0])
+		if err != nil {
+			t.Fatalf("reading decision: %v", err)
+		}
+		if !strings.Contains(string(raw), "revisit_if: GraphQL adoption") {
+			t.Errorf("revisit_if missing from frontmatter:\n%s", raw)
+		}
+	})
+
+	t.Run("recall --json", func(t *testing.T) {
+		code, out := run(t, dir, "--json", "recall", "--domain", "api")
+		if code != 0 {
+			t.Fatalf("recall: exit %d, out=%s", code, out)
+		}
+		var res RecallResult
+		if err := json.Unmarshal([]byte(out), &res); err != nil {
+			t.Fatalf("unmarshal: %v (out=%s)", err, out)
+		}
+		if len(res.Memories) != 1 || res.Memories[0].RevisitIf != "GraphQL adoption" {
+			t.Errorf("revisit_if missing from JSON: %+v", res.Memories)
+		}
+	})
+
+	t.Run("recall text", func(t *testing.T) {
+		code, out := run(t, dir, "recall", "--domain", "api")
+		if code != 0 {
+			t.Fatalf("recall: exit %d, out=%s", code, out)
+		}
+		if !strings.Contains(out, "revisit if: GraphQL adoption") {
+			t.Errorf("revisit_if missing from text output:\n%s", out)
+		}
+	})
+
+	t.Run("session-start injection", func(t *testing.T) {
+		code, out := run(t, dir, "hooks", "session-start")
+		if code != 0 {
+			t.Fatalf("session-start: exit %d, out=%s", code, out)
+		}
+		if !strings.Contains(out, "revisit if: GraphQL adoption") {
+			t.Errorf("revisit_if missing from the injected context:\n%s", out)
+		}
+	})
+}
+
+// A memory with no revisit condition must not grow an empty line for it.
+func TestRecall_noRevisitLineWhenUnset(t *testing.T) {
+	dir := setupBrainDir(t)
+
+	if code, out := run(t, dir, "remember", "The users table has 12M rows",
+		"--domain", "database", "--layer", "fact"); code != 0 {
+		t.Fatalf("remember: exit %d, out=%s", code, out)
+	}
+	code, out := run(t, dir, "recall", "--domain", "database")
+	if code != 0 {
+		t.Fatalf("recall: exit %d, out=%s", code, out)
+	}
+	if strings.Contains(out, "revisit if:") {
+		t.Errorf("printed an empty revisit line:\n%s", out)
+	}
+}
+
+// Superseding is how one decision replaces another. The replaced memory is
+// retired, so recall stops surfacing it, and the replacement stands alone.
+func TestRemember_supersedesRetiresTheOldDecision(t *testing.T) {
+	dir := setupBrainDir(t)
+
+	code, out := run(t, dir, "--json", "remember", "snake_case for API responses",
+		"--domain", "api", "--layer", "decision")
+	if code != 0 {
+		t.Fatalf("remember old: exit %d, out=%s", code, out)
+	}
+	var first RememberResult
+	if err := json.Unmarshal([]byte(out), &first); err != nil {
+		t.Fatalf("unmarshal: %v (out=%s)", err, out)
+	}
+
+	if code, out := run(t, dir, "remember", "camelCase for API responses",
+		"--domain", "api", "--layer", "decision", "--supersedes", first.Path); code != 0 {
+		t.Fatalf("remember new: exit %d, out=%s", code, out)
+	}
+
+	code, out = run(t, dir, "recall", "--domain", "api")
+	if code != 0 {
+		t.Fatalf("recall: exit %d, out=%s", code, out)
+	}
+	if !strings.Contains(out, "camelCase") {
+		t.Errorf("replacement missing from recall:\n%s", out)
+	}
+	if strings.Contains(out, "snake_case") {
+		t.Errorf("superseded decision still surfaces in recall:\n%s", out)
+	}
+}
+
+// A path that does not resolve is almost always a typo, and a typo must not
+// buy two live decisions that contradict each other.
+func TestRemember_supersedesRejectsAPathThatIsNotThere(t *testing.T) {
+	dir := setupBrainDir(t)
+
+	code, _ := run(t, dir, "remember", "camelCase for API responses",
+		"--domain", "api", "--layer", "decision", "--supersedes", "decisions/typo.md")
+	if code != 3 {
+		t.Errorf("exit %d, want 3 (invalid input)", code)
+	}
+
+	// Nothing may have been written on the way to that rejection.
+	listCode, listOut := run(t, dir, "list")
+	if listCode != 0 {
+		t.Fatalf("list: exit %d", listCode)
+	}
+	if !strings.Contains(listOut, "0 memories") {
+		t.Errorf("a rejected remember left something behind:\n%s", listOut)
+	}
+}
+
+// confidence was removed from the model. Files already carrying it — every
+// correction brain trust override ever wrote — must keep loading, because
+// frontmatter parsing ignores keys it does not know. That leniency is what
+// made the removal free, and it is worth a test rather than an assumption.
+func TestRemovedFields_doNotBreakExistingFiles(t *testing.T) {
+	dir := setupBrainDir(t)
+	if err := os.MkdirAll(filepath.Join(dir, "corrections"), 0o755); err != nil {
+		t.Fatalf("mkdir: %v", err)
+	}
+
+	legacy := "---\n" +
+		"layer: correction\n" +
+		"domain: testing\n" +
+		"created: 2026-04-01T00:00:00Z\n" +
+		"source: human\n" +
+		"confidence: high\n" +
+		"immutable: true\n" +
+		"---\n" +
+		"Stop flagging nullable email columns\n"
+	if err := os.WriteFile(filepath.Join(dir, "corrections", "legacy.md"), []byte(legacy), 0o644); err != nil {
+		t.Fatalf("writing legacy correction: %v", err)
+	}
+
+	code, out := run(t, dir, "recall", "--domain", "testing")
+	if code != 0 {
+		t.Fatalf("recall: exit %d, out=%s", code, out)
+	}
+	if !strings.Contains(out, "Stop flagging nullable email columns") {
+		t.Errorf("a file carrying the removed confidence field no longer loads:\n%s", out)
+	}
+}

internal/cmd/hooks.go

@@ -348,7 +348,11 @@ func renderSessionStart(memories []memory.Memory) string {
 	b.WriteString(sessionStartHeader)
 	for _, m := range memories {
 		fmt.Fprintf(&b, "[%s] %s\n", m.Layer, m.Title())
-		fmt.Fprintf(&b, "  domain: %s  path: %s\n\n", m.Domain, m.Path)
+		fmt.Fprintf(&b, "  domain: %s  path: %s\n", m.Domain, m.Path)
+		if m.RevisitIf != "" {
+			fmt.Fprintf(&b, "  revisit if: %s\n", m.RevisitIf)
+		}
+		b.WriteString("\n")
 	}
 	b.WriteString(sessionStartFooter)
 	return b.String()

internal/cmd/recall.go

@@ -50,12 +50,13 @@ func recallCmd() *cobra.Command {
 			jr := RecallResult{}
 			for _, m := range memories {
 				jr.Memories = append(jr.Memories, RecallMemory{
-					Path:   m.Path,
-					Layer:  string(m.Layer),
-					Domain: m.Domain,
-					Title:  m.Title(),
-					Body:   m.Body,
-					Tags:   m.Tags,
+					Path:      m.Path,
+					Layer:     string(m.Layer),
+					Domain:    m.Domain,
+					Title:     m.Title(),
+					Body:      m.Body,
+					Tags:      m.Tags,
+					RevisitIf: m.RevisitIf,
 				})
 			}
 
@@ -67,6 +68,13 @@ func recallCmd() *cobra.Command {
 					}
 					fmt.Fprintf(&sb, "[%s] %s\n", m.Layer, m.Title())
 					fmt.Fprintf(&sb, "  domain: %s  path: %s\n", m.Domain, m.Path)
+					// A settled decision is only reopenable if the reader can
+					// see what would reopen it. Printing the condition only in
+					// --json would hide it from every model that reads the
+					// text, which is the path the rules point at.
+					if m.RevisitIf != "" {
+						fmt.Fprintf(&sb, "  revisit if: %s\n", m.RevisitIf)
+					}
 				}
 				return sb.String()
 			})

internal/cmd/remember.go

@@ -1,6 +1,7 @@
 package cmd
 
 import (
+	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -9,14 +10,18 @@ import (
 
 	"github.com/spf13/cobra"
 
+	"github.com/luuuc/brain/internal/engine"
 	"github.com/luuuc/brain/internal/memory"
 )
 
 func rememberCmd() *cobra.Command {
 	var (
-		domain string
-		layer  string
-		tags   []string
+		domain     string
+		layer      string
+		tags       []string
+		persona    string
+		revisitIf  string
+		supersedes string
 	)
 
 	cmd := &cobra.Command{
@@ -37,11 +42,14 @@ func rememberCmd() *cobra.Command {
 			}
 
 			m := memory.Memory{
-				Domain:  domain,
-				Body:    content,
-				Created: time.Now(),
-				Source:  memory.SourceHuman,
-				Tags:    tags,
+				Domain:     domain,
+				Body:       content,
+				Created:    time.Now(),
+				Source:     memory.SourceHuman,
+				Tags:       tags,
+				Persona:    persona,
+				RevisitIf:  revisitIf,
+				Supersedes: supersedes,
 			}
 
 			if layer != "" {
@@ -55,6 +63,9 @@ func rememberCmd() *cobra.Command {
 			eng := engineFrom(cmd)
 			result, err := eng.Remember(cmd.Context(), m)
 			if err != nil {
+				if errors.Is(err, engine.ErrInvalidArgs) {
+					return &ExitError{Code: 3, Err: err}
+				}
 				return err
 			}
 
@@ -79,6 +90,12 @@ func rememberCmd() *cobra.Command {
 	cmd.Flags().StringVar(&domain, "domain", "", "memory domain (required)")
 	cmd.Flags().StringVar(&layer, "layer", "", "memory layer (fact, lesson, decision, effectiveness, correction)")
 	cmd.Flags().StringSliceVar(&tags, "tags", nil, "comma-separated tags")
+	cmd.Flags().StringVar(&supersedes, "supersedes", "",
+		"path of the memory this one replaces (e.g. decisions/old-choice.md); it is retired")
+	cmd.Flags().StringVar(&revisitIf, "revisit-if", "",
+		"the condition that would make this memory worth reconsidering (e.g. \"GraphQL adoption\")")
+	cmd.Flags().StringVar(&persona, "persona", "",
+		"persona this memory came from (e.g. kent-beck); ranks it by that persona's acceptance rate")
 	return cmd
 }
 

internal/cmd/result.go

@@ -17,12 +17,13 @@ type RecallResult struct {
 
 // RecallMemory is a single memory in recall output.
 type RecallMemory struct {
-	Path   string   `json:"path"`
-	Layer  string   `json:"layer"`
-	Domain string   `json:"domain"`
-	Title  string   `json:"title"`
-	Body   string   `json:"body"`
-	Tags   []string `json:"tags,omitempty"`
+	Path      string   `json:"path"`
+	Layer     string   `json:"layer"`
+	Domain    string   `json:"domain"`
+	Title     string   `json:"title"`
+	Body      string   `json:"body"`
+	Tags      []string `json:"tags,omitempty"`
+	RevisitIf string   `json:"revisit_if,omitempty"`
 }
 
 // ListResult is the JSON output of brain list.

internal/cmd/track_test.go

@@ -167,3 +167,62 @@ func TestTrackIntegration_ViewTextHasRate(t *testing.T) {
 		t.Errorf("view text missing rate:\n%s", out)
 	}
 }
+
+// The loop 01-06 built, driven entirely through the CLI: record outcomes for
+// two personas, attribute a memory to each, and watch recall rank the
+// well-regarded persona's memory above the newer one.
+//
+// Before --persona existed this was provable only from Go, because nothing
+// any user could run put a persona on a lesson. Effectiveness ranking was
+// switched on and had nothing to score.
+func TestRemember_personaMakesEffectivenessRankingReachable(t *testing.T) {
+	target := setupBrainDir(t)
+
+	// kent-beck is accepted every time; rich-hickey is overridden every time.
+	for range 5 {
+		if code, out := run(t, target, "track", "--domain", "testing",
+			"--persona", "kent-beck", "--outcome", "accepted"); code != 0 {
+			t.Fatalf("track beck: exit %d, out=%s", code, out)
+		}
+		if code, out := run(t, target, "track", "--domain", "testing",
+			"--persona", "rich-hickey", "--outcome", "overridden"); code != 0 {
+			t.Fatalf("track hickey: exit %d, out=%s", code, out)
+		}
+	}
+
+	// Beck's lesson is written first, so hickey's is newer and would lead on
+	// recency alone.
+	if code, out := run(t, target, "remember", "Check for concurrent writes",
+		"--domain", "testing", "--layer", "lesson", "--persona", "kent-beck"); code != 0 {
+		t.Fatalf("remember beck: exit %d, out=%s", code, out)
+	}
+	if code, out := run(t, target, "remember", "Prefer values over places",
+		"--domain", "testing", "--layer", "lesson", "--persona", "rich-hickey"); code != 0 {
+		t.Fatalf("remember hickey: exit %d, out=%s", code, out)
+	}
+
+	code, out := run(t, target, "recall", "--domain", "testing", "--layer", "lesson")
+	if code != 0 {
+		t.Fatalf("recall: exit %d, out=%s", code, out)
+	}
+	beck := strings.Index(out, "Check for concurrent writes")
+	hickey := strings.Index(out, "Prefer values over places")
+	if beck < 0 || hickey < 0 {
+		t.Fatalf("both lessons should be recalled:\n%s", out)
+	}
+	if beck > hickey {
+		t.Errorf("the overridden persona's newer lesson outranked the accepted one:\n%s", out)
+	}
+}
+
+// A persona that is not a slug would be stored and then never match a score,
+// failing silently. It is rejected instead.
+func TestRemember_rejectsAPersonaThatCouldNeverScore(t *testing.T) {
+	target := setupBrainDir(t)
+
+	code, _ := run(t, target, "remember", "A lesson", "--domain", "testing",
+		"--layer", "lesson", "--persona", "Kent Beck")
+	if code != 3 {
+		t.Errorf("exit %d, want 3 (invalid input)", code)
+	}
+}