fix(recall): exclude superseded lessons from lessons.jsonl retrieval

.agent/memory/render_lessons.py

@@ -132,19 +132,31 @@ def _bullet_for(lesson, superseded_by):
     return f"- {claim}  <!-- {ann} -->"
 
 
-def _build_auto_section(lessons):
-    # Only accepted supersessions flip the old lesson to strikethrough.
-    # A provisional --supersedes would otherwise blank the active lesson
-    # before its replacement has been accepted, leaving no active guidance
-    # on that topic at all (retrieval skips both provisional and
-    # strikethrough).
+def superseded_by_map(lessons):
+    """Old lesson id -> id of the accepted lesson that supersedes it.
+
+    Only accepted supersessions retire the old lesson. A provisional
+    --supersedes would otherwise blank the active lesson before its
+    replacement has itself been accepted, leaving no active guidance on
+    that topic at all -- callers that skip provisional/strikethrough
+    entries would then surface nothing for that topic.
+
+    Shared with recall.py so retrieval and rendering never disagree on
+    which lessons are currently retired -- see its docstring for the bug
+    this fixed (recall returning both the stale and replacement guidance).
+    """
     superseded_by = {}
     for L in lessons:
         if L.get("status") != "accepted":
             continue
         sup = L.get("supersedes")
         if sup:
             superseded_by[sup] = L.get("id")
+    return superseded_by
+
+
+def _build_auto_section(lessons):
+    superseded_by = superseded_by_map(lessons)
 
     groups = defaultdict(list)
     for L in lessons:

.agent/tools/recall.py

@@ -25,7 +25,9 @@
 
 BASE = os.path.abspath(os.path.join(os.path.dirname(__file__), ".."))
 sys.path.insert(0, os.path.join(BASE, "harness"))
+sys.path.insert(0, os.path.join(BASE, "memory"))
 from text import word_set  # noqa: E402
+from render_lessons import superseded_by_map  # noqa: E402
 
 LESSONS_JSONL = os.path.join(BASE, "memory/semantic/lessons.jsonl")
 LESSONS_MD = os.path.join(BASE, "memory/semantic/LESSONS.md")
@@ -68,8 +70,17 @@ def _load_structured():
             order.append(lid)
         latest[lid] = row
 
+    # A lesson can stay status="accepted" on its own row even after being
+    # superseded -- supersession creates a NEW id, it never edits the old
+    # row. Exclude ids an accepted supersession retires, mirroring
+    # render_lessons.py's rendering rule, so recall doesn't return both the
+    # stale and replacement guidance for the same topic.
+    retired = superseded_by_map(latest.values())
+
     out = []
     for lid in order:
+        if lid in retired:
+            continue
         lesson = latest[lid]
         if lesson.get("status") != "accepted":
             continue

tests/test_recall_supersession.py

@@ -0,0 +1,161 @@
+"""recall.py must not surface superseded lessons alongside their replacement.
+
+lessons.jsonl is append-only: supersession writes a NEW id whose
+`supersedes` field points at the old one -- it never edits the old row.
+The old row keeps `status: "accepted"` forever, so a naive per-row
+`status == "accepted"` filter (recall's own dedupe already handles same-id
+retraction, but not this different-id case) returns both the stale and
+the replacement guidance for the same topic. Fixed by sharing
+`render_lessons.superseded_by_map` between rendering and retrieval so
+they never disagree about which lessons are currently retired.
+"""
+import importlib.util
+import json
+import tempfile
+import unittest
+from pathlib import Path
+
+ROOT = Path(__file__).resolve().parents[1]
+RECALL = ROOT / ".agent" / "tools" / "recall.py"
+RENDER_LESSONS = ROOT / ".agent" / "memory" / "render_lessons.py"
+
+
+def load_module(path: Path, module_name: str):
+    spec = importlib.util.spec_from_file_location(module_name, path)
+    module = importlib.util.module_from_spec(spec)
+    spec.loader.exec_module(module)
+    return module
+
+
+def _lesson(lesson_id, claim, conditions, status="accepted", supersedes=None):
+    return {
+        "id": lesson_id,
+        "claim": claim,
+        "conditions": conditions,
+        "status": status,
+        "supersedes": supersedes,
+        "accepted_at": "2026-08-01T00:00:00+00:00",
+    }
+
+
+class RecallSupersessionTest(unittest.TestCase):
+    def with_semantic_dir(self):
+        tmp = tempfile.TemporaryDirectory()
+        semantic_dir = Path(tmp.name) / ".agent" / "memory" / "semantic"
+        semantic_dir.mkdir(parents=True)
+        return tmp, semantic_dir
+
+    def write_lessons(self, semantic_dir: Path, lessons):
+        path = semantic_dir / "lessons.jsonl"
+        with path.open("w", encoding="utf-8") as f:
+            for lesson in lessons:
+                f.write(json.dumps(lesson) + "\n")
+
+    def load_recall(self, semantic_dir: Path, module_name: str):
+        recall = load_module(RECALL, module_name)
+        recall.LESSONS_JSONL = str(semantic_dir / "lessons.jsonl")
+        recall.LESSONS_MD = str(semantic_dir / "missing.md")
+        return recall
+
+    def test_superseded_lesson_is_excluded_from_structured_load(self):
+        tmp, semantic_dir = self.with_semantic_dir()
+        self.addCleanup(tmp.cleanup)
+        old_id, new_id = "lesson_old_git_tip", "lesson_new_git_tip"
+        self.write_lessons(
+            semantic_dir,
+            [
+                _lesson(old_id, "git branch -f silently no-ops on checked-out branch",
+                        ["git branch -f"]),
+                _lesson(new_id, "git branch -f refuses with exit 128 on checked-out branch",
+                        ["git branch -f", "rollback"], supersedes=old_id),
+            ],
+        )
+        recall = self.load_recall(semantic_dir, "recall_1")
+
+        loaded = recall._load_structured()
+        ids = [row["id"] for row in loaded]
+
+        self.assertIn(new_id, ids)
+        self.assertNotIn(old_id, ids)
+
+    def test_recall_ranks_replacement_not_the_superseded_lesson(self):
+        tmp, semantic_dir = self.with_semantic_dir()
+        self.addCleanup(tmp.cleanup)
+        old_id, new_id = "lesson_old_git_tip", "lesson_new_git_tip"
+        self.write_lessons(
+            semantic_dir,
+            [
+                _lesson(
+                    old_id,
+                    "git branch -f <name> <sha> silently no-ops when checked out",
+                    ["git branch -f", "cherry-pick loop"],
+                ),
+                _lesson(
+                    new_id,
+                    "git branch -f <name> <sha> refuses to move the checked-out "
+                    "branch; use git update-ref then git restore",
+                    ["git branch -f", "git update-ref", "rollback"],
+                    supersedes=old_id,
+                ),
+            ],
+        )
+        recall = self.load_recall(semantic_dir, "recall_2")
+
+        result, meta = recall.recall(
+            "git branch -f cherry-pick rollback update-ref", top_k=5, min_score=0.01
+        )
+        returned_ids = [row.get("id") for row in result if row.get("id")]
+
+        self.assertIn(new_id, returned_ids)
+        self.assertNotIn(old_id, returned_ids)
+        self.assertEqual(meta["considered"], 1)
+
+    def test_provisional_supersession_does_not_retire_old_lesson(self):
+        tmp, semantic_dir = self.with_semantic_dir()
+        self.addCleanup(tmp.cleanup)
+        old_id, provisional_id = "lesson_active", "lesson_provisional"
+        self.write_lessons(
+            semantic_dir,
+            [
+                _lesson(old_id, "Keep using the established rollback procedure",
+                        ["rollback"]),
+                _lesson(
+                    provisional_id,
+                    "Experimental rollback procedure under review",
+                    ["rollback"],
+                    status="provisional",
+                    supersedes=old_id,
+                ),
+            ],
+        )
+        recall = self.load_recall(semantic_dir, "recall_3")
+
+        loaded = recall._load_structured()
+        ids = [row["id"] for row in loaded]
+
+        self.assertIn(old_id, ids)
+        self.assertNotIn(provisional_id, ids)
+
+
+class SupersededByMapTest(unittest.TestCase):
+    """render_lessons.superseded_by_map is the single source of truth both
+    rendering and recall share -- exercised directly so drift between the
+    two consumers is caught here, not just at the recall integration level.
+    """
+
+    def test_only_accepted_supersessions_are_mapped(self):
+        render_lessons = load_module(RENDER_LESSONS, "render_lessons_map_test")
+        lessons = [
+            _lesson("lesson_a", "a", []),
+            _lesson("lesson_b", "b", [], supersedes="lesson_a"),
+            _lesson("lesson_c", "c", []),
+            _lesson("lesson_d", "d", [], status="provisional", supersedes="lesson_c"),
+        ]
+
+        mapping = render_lessons.superseded_by_map(lessons)
+
+        self.assertEqual(mapping, {"lesson_a": "lesson_b"})
+
+
+if __name__ == "__main__":
+    unittest.main()