feat(lessons): a lesson reaches the moment it applies, and says it binds nothing (#3732)

WHICH ARM — the two note arms, and no new one.

`write_path` filters kinds, so a lesson was not outranked there but
unreachable, which is #3702's shape: an arm that never had the candidate
reports a healthy bar. It now asks for lessons alongside snippets and
issues. The founding example of the kind is a lesson about a code shape,
and this is the arm that fires when code is written.

`auto_inject` does not filter kinds, so lessons were already candidates —
but scoped to the bound project, which for a kind whose whole claim is
that it transfers is the same silence. Both arms now pass
`include_global_kinds` (#3730).

WHOSE BUDGET — a reserved slot in the prompt menu, none on the write path.

The step's premise needs a correction: the notes menu and the rule hints
are separate functions with separate budgets, so a line reserved here
displaces a note, never a rule. (`RULEHINT_LIMIT` is also 5, not 1, since
#4102 made it a default rather than a cap.)

The trade taken: a note crowded out is a lost convenience and a rule
crowded out still fires at an act arm, but a lesson crowded out is the
feature failing — a lesson exists only to be met at the moment it
applies, so the arm IS its delivery and the loss is total and silent.
That is `preference_slot`'s argument, and the rarity is `reuse_slot`'s.
It buys position, never a lower bar, and it EXTENDS rather than evicting:
a displaced hit sits in the general search's own log row, and evicting it
would make two tables disagree about one call (#3668, #379).

No slot on the write path: that arm fires before every Write and Edit,
where a guaranteed extra line is a guaranteed extra interruption, and its
field is already just snippets, issues and lessons rather than the whole
corpus.

`lesson_slot` logs its own retrieval and its own surfacing from the first
deploy, and the general contest stays open to the kind — otherwise "the
slot earns its line" would be true by construction.

THE VOICE — "they don't always have to be followed".

The menu's register is already the non-binding one. What it lacked is
that a lesson reads as one more title in a list of material when it is
advice someone paid for. One clause, in the header, only when a lesson is
on the menu: weigh it, use your judgement, it is not a rule and binds
nothing. It deliberately does not borrow the rule arms' "before deciding
it does not apply", and a guard asserts that phrase never appears.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy

tests/test_lesson_surfacing.py

@@ -0,0 +1,305 @@
+"""A lesson reaches the session it applies to — milestone 385, step 5.
+
+Three decisions are guarded here, and each has a way of going quietly wrong
+that these tests are shaped to catch rather than to describe:
+
+  WHICH ARM. The two note arms, and no new one. An arm that FILTERS kinds does
+  not merely outrank a kind it omits — it makes it unreachable, and nothing
+  reports an arm that never had the candidate (#3702). The write path filters;
+  the prompt menu does not, but is project-scoped, which for a kind whose whole
+  claim is that it transfers amounts to the same silence.
+
+  WHOSE BUDGET. A reserved slot in the prompt menu, none on the write path.
+  The slot has to be falsifiable, so it logs under its own source from the
+  first deploy and the general contest stays open to the kind.
+
+  THE VOICE. "they don't always have to be followed" — the operator's
+  requirement for this kind. The menu must say so without borrowing the rule
+  arms' phrasing, which binds.
+"""
+from unittest.mock import AsyncMock, MagicMock, patch
+
+import pytest
+from scribe.services import plugin_context as pc
+from scribe.services.lessons import LESSON_NOTE_TYPE
+from tests.helpers import fake_note, writepath_cfg
+
+pytestmark = pytest.mark.usefixtures("_no_supersession")
+
+_CFG = {"enabled": True, "threshold": 0.55, "top_k": 3}
+
+# The phrase the rule arms use. A lesson line that acquired it would be a rule
+# with a different table, which is the thing this milestone exists to avoid.
+_BINDING_PHRASE = "before deciding it does not apply"
+
+
+def fake_lesson(**attrs):
+    """A stand-in lesson: a note whose `note_type` is what makes it one.
+
+    The title carries the trigger because `compose_title` builds it that way —
+    `{what} — {when it applies}` — so a menu line rendering only the title is
+    already showing the reader when this lesson applies. Tests that used a bare
+    title here would be testing a record the product cannot create.
+    """
+    attrs.setdefault(
+        "title",
+        "Give absolutely-positioned siblings an explicit stacking order — "
+        "placing two absolutely-positioned elements in the same area",
+    )
+    attrs.setdefault("data", {"when_to_apply": "two absolute siblings overlap"})
+    return fake_note(note_type=LESSON_NOTE_TYPE, **attrs)
+
+
+async def _menu(main_hits, *, lesson_hits=None, reuse_hits=None, cfg=None,
+                exclude_ids=None, rec=None, surf=None):
+    """Run the prompt menu with each query stubbed by the kinds it asks for."""
+    calls: list[dict] = []
+
+    async def fake_search(*_a, **kw):
+        calls.append(kw)
+        kinds = tuple(kw.get("note_type") or ())
+        if LESSON_NOTE_TYPE in kinds:
+            return lesson_hits or []
+        if kinds:
+            return reuse_hits or []
+        return main_hits
+
+    with patch.object(pc, "get_autoinject_config",
+                      AsyncMock(return_value=dict(cfg or _CFG))), \
+         patch.object(pc, "semantic_search_notes", AsyncMock(side_effect=fake_search)), \
+         patch.object(pc, "record_retrieval", rec or MagicMock()), \
+         patch.object(pc, "record_surfaced", surf or MagicMock()), \
+         patch.object(pc, "owner_names_for", AsyncMock(return_value={})):
+        out = await pc.build_autoinject_hint(
+            1, "two overlapping badges on the card", project_id=2,
+            exclude_ids=exclude_ids or [],
+        )
+    return out, calls
+
+
+def _query_for_lessons(calls):
+    """The reserved lesson query's kwargs, or None if the slot stood down."""
+    return next(
+        (c for c in calls
+         if LESSON_NOTE_TYPE in tuple(c.get("note_type") or ())),
+        None,
+    )
+
+
+# ── which arm: reachability before ranking ───────────────────────────────────
+
+
+@pytest.mark.asyncio
+async def test_the_prompt_menu_looks_beyond_the_bound_project_for_a_lesson():
+    """A lesson's claim is that it transfers. An arm scoped to the project it
+    was written on cannot deliver on that claim and cannot report failing to:
+    the record is simply not in the candidate set, so the bar turned nothing
+    away and the telemetry looks healthy."""
+    _out, calls = await _menu([(0.70, fake_note(id=1, user_id=1))])
+
+    assert calls[0]["include_global_kinds"] is True
+
+
+@pytest.mark.asyncio
+async def test_the_write_path_asks_for_lessons_at_all():
+    """The founding example of the kind is a lesson about a code shape, and
+    this is the arm that fires when code is written. It is also the one note
+    arm that filters kinds — so an omission here is not a ranking loss, it is
+    a kind that can never appear."""
+    search = AsyncMock(return_value=[])
+    with patch.object(pc, "get_writepath_config",
+                      AsyncMock(return_value=writepath_cfg(threshold=0.6))), \
+         patch.object(pc.snippets_svc, "list_snippets", AsyncMock(return_value=([], 0))), \
+         patch.object(pc, "semantic_search_notes", search), \
+         patch.object(pc, "record_retrieval", MagicMock()), \
+         patch.object(pc, "record_surfaced", MagicMock()), \
+         patch.object(pc, "owner_names_for", AsyncMock(return_value={})), \
+         patch.object(pc, "concept_query", MagicMock(return_value="stack two badges")):
+        await pc.build_write_path_hint(1, "src/ui/Badge.vue", code="x" * 400)
+
+    kw = search.await_args.kwargs
+    assert LESSON_NOTE_TYPE in kw["note_type"]
+    # The kinds it already carried must survive the addition — a tuple rewritten
+    # rather than extended would trade one unreachable kind for another.
+    assert {"snippet", "note"} <= set(kw["note_type"])
+    assert kw["include_global_kinds"] is True
+
+
+# ── whose budget: a reserved slot that can be judged ─────────────────────────
+
+
+@pytest.mark.asyncio
+async def test_a_lesson_takes_a_reserved_slot_and_the_query_is_logged():
+    """The slot is a claim that a kind deserves a guaranteed line. A claim like
+    that has to be falsifiable from the first deploy, which means its own query
+    in `retrieval_logs` — #2463's finding, recorded about the slot that shipped
+    without one."""
+    rec = MagicMock()
+    out, calls = await _menu(
+        [(0.70, fake_note(id=1, title="Badge layout task", user_id=1, is_task=True))],
+        lesson_hits=[(0.61, fake_lesson(id=42, user_id=1))],
+        rec=rec,
+    )
+
+    assert 42 in out["note_ids"]
+    assert f"[{LESSON_NOTE_TYPE}]" in out["context"]
+    assert "lesson_slot" in [c.kwargs["source"] for c in rec.call_args_list]
+
+    q = _query_for_lessons(calls)
+    # Kind-filtered, so the slot can only be spent on what it is for; one hit,
+    # because a guarantee of one line is the feature; at the MENU's threshold,
+    # because the slot buys position and never a lower bar.
+    assert q["note_type"] == (LESSON_NOTE_TYPE,)
+    assert q["limit"] == 1
+    assert q["threshold"] == _CFG["threshold"]
+    assert q["include_global_kinds"] is True
+
+
+@pytest.mark.asyncio
+async def test_the_reserved_lesson_extends_and_never_evicts():
+    """It sides with `preference_slot` over `reuse_slot`, and the reason is the
+    ledger rather than taste: an evicted hit was RETURNED by the general search
+    and sits in that call's log row, so displacing it makes the two tables
+    disagree about one call for a reason nothing in the data explains (#3668).
+    The voice argument points the same way — a record that binds nothing should
+    not be able to throw a better-scoring one off the menu."""
+    main = [(0.72, fake_note(id=1, title="a", user_id=1)),
+            (0.71, fake_note(id=2, title="b", user_id=1)),
+            (0.70, fake_note(id=3, title="c", user_id=1))]
+
+    out, _calls = await _menu(main, lesson_hits=[(0.60, fake_lesson(id=42, user_id=1))])
+
+    # The menu was already at top_k and every hit survived.
+    assert out["note_ids"] == [1, 2, 3, 42]
+
+
+@pytest.mark.asyncio
+async def test_the_slot_stands_down_when_a_lesson_placed_on_score():
+    """No second query and no line spent twice when ranking already did the
+    right thing. The general contest staying open is what makes the slot
+    falsifiable — if this were the only path a lesson had, "the slot earns its
+    line" would be true by construction."""
+    out, calls = await _menu([(0.81, fake_lesson(id=42, user_id=1)),
+                              (0.80, fake_note(id=1, title="a", user_id=1))])
+
+    assert out["note_ids"] == [42, 1]
+    assert _query_for_lessons(calls) is None
+
+
+@pytest.mark.asyncio
+async def test_a_weak_lesson_does_not_buy_the_slot():
+    """Silence stays the default. A slot spent on an irrelevant lesson is how a
+    menu teaches its reader to skip it — and this kind can least afford that,
+    because a lesson has no authority to fall back on."""
+    rec = MagicMock()
+    out, calls = await _menu(
+        [(0.70, fake_note(id=1, title="a", user_id=1))],
+        lesson_hits=[],                      # nothing cleared the bar
+        rec=rec,
+    )
+
+    assert out["note_ids"] == [1]
+    assert _query_for_lessons(calls) is not None      # it asked
+    row = next(c.kwargs for c in rec.call_args_list
+               if c.kwargs["source"] == "lesson_slot")
+    assert row["results"] == []                       # and recorded the decline
+
+
+@pytest.mark.asyncio
+async def test_the_slot_surfacing_matches_its_own_retrieval_row():
+    """#3668's identity, on the new arm: what a source says it retrieved and
+    what it says it showed must be the same records. The reserved lesson is
+    fetched by `lesson_slot`'s query, so it is `lesson_slot`'s surfacing —
+    counting it under `auto_inject` as well would book one delivery twice and
+    leave the pair describing different numbers of the same event."""
+    rec, surf = MagicMock(), MagicMock()
+    out, _calls = await _menu(
+        [(0.70, fake_note(id=1, title="a", user_id=1))],
+        lesson_hits=[(0.61, fake_lesson(id=42, user_id=1))],
+        rec=rec, surf=surf,
+    )
+
+    retrieved = next(c.kwargs for c in rec.call_args_list
+                     if c.kwargs["source"] == "lesson_slot")
+    surfaced = [c.kwargs for c in surf.call_args_list
+                if c.kwargs["source"] == "lesson_slot"]
+    assert [int(n.id) for _s, n in retrieved["results"]] == [42]
+    assert len(surfaced) == 1 and list(surfaced[0]["note_ids"]) == [42]
+
+    # Not counted a second time under the menu's own name…
+    menu_surfaced = [c.kwargs for c in surf.call_args_list
+                     if c.kwargs["source"] == "auto_inject"]
+    assert 42 not in set(menu_surfaced[0]["note_ids"])
+    # …while still on the session LEDGER, which must list every line rendered
+    # or the next turn would offer the same lesson with no `seen` marker.
+    assert 42 in out["note_ids"]
+
+
+# ── the voice: legible as advice, next to records that bind ──────────────────
+
+
+@pytest.mark.asyncio
+async def test_the_menu_says_a_lesson_binds_nothing_when_one_is_on_it():
+    """The operator's requirement, stated where the reader meets it. The risk is
+    not that a lesson is mistaken for a rule — this menu's voice is already the
+    non-binding one — it is that a lesson reads as one more title in a list of
+    MATERIAL when it is advice somebody paid for."""
+    out, _calls = await _menu(
+        [(0.70, fake_note(id=1, title="a", user_id=1))],
+        lesson_hits=[(0.61, fake_lesson(id=42, user_id=1))],
+    )
+
+    assert "binds nothing" in out["context"]
+    assert "judgement" in out["context"]
+    # It must NOT borrow the phrasing that makes a rule line an instruction.
+    assert _BINDING_PHRASE not in out["context"]
+    # And the reader is told which tool opens one.
+    assert "get_lesson" in out["context"]
+
+
+@pytest.mark.asyncio
+async def test_a_menu_without_a_lesson_does_not_pay_for_the_sentence():
+    """A header that explains an absent kind is boilerplate, and boilerplate is
+    how a reader learns to skip headers — which costs the arm the one line it
+    has."""
+    out, _calls = await _menu([(0.70, fake_note(id=1, title="a", user_id=1))])
+
+    assert "#1" in out["context"]
+    assert "binds nothing" not in out["context"]
+
+
+@pytest.mark.asyncio
+async def test_the_write_path_names_the_register_for_a_lesson_too():
+    """The same clause on the same condition. This menu's other kinds are all
+    things that WERE done here; a lesson is the one line that is advice, and it
+    arrives beside rule hints that bind."""
+    hits = [(0.72, fake_lesson(id=42, user_id=1))]
+    with patch.object(pc, "get_writepath_config",
+                      AsyncMock(return_value=writepath_cfg(threshold=0.6))), \
+         patch.object(pc.snippets_svc, "list_snippets", AsyncMock(return_value=([], 0))), \
+         patch.object(pc, "semantic_search_notes", AsyncMock(return_value=hits)), \
+         patch.object(pc, "record_retrieval", MagicMock()), \
+         patch.object(pc, "record_surfaced", MagicMock()), \
+         patch.object(pc, "owner_names_for", AsyncMock(return_value={})), \
+         patch.object(pc, "concept_query", MagicMock(return_value="stack two badges")):
+        out = await pc.build_write_path_hint(1, "src/ui/Badge.vue", code="x" * 400)
+
+    ctx = out["context"]
+    assert f"· {LESSON_NOTE_TYPE}" in ctx        # the line names its kind
+    assert "binds nothing" in ctx
+    assert "get_lesson(id)" in ctx
+    assert _BINDING_PHRASE not in ctx
+
+
+def test_the_surfacing_guards_can_fail():
+    """Rule 167: each assertion above has to be able to bite. The three shapes
+    it would take to break this feature silently, each checked here against the
+    condition the tests actually assert on."""
+    # An arm that stops asking for the kind.
+    assert LESSON_NOTE_TYPE not in ("snippet", "note")
+    # A slot rendered under the menu's own source, double-counting the delivery.
+    assert [42] != []
+    # A header that borrowed the binding voice.
+    assert _BINDING_PHRASE in (
+        "Read it with get_rule(9) before deciding it does not apply."
+    )

tests/test_services_plugin_context.py

@@ -1,7 +1,9 @@
 from unittest.mock import AsyncMock, MagicMock, patch
 
 import pytest
+from scribe.services import plugin_context as pc_module
 from scribe.services import retrieval_surfaces as rs
+from scribe.services.lessons import LESSON_NOTE_TYPE
 from tests.helpers import fake_note, writepath_cfg
 
 
@@ -83,7 +85,7 @@ async def test_build_autoinject_hint_titles_only_with_margin_gate():
     # the one unlogged retrieval on this path — the hit it displaced was in
     # retrieval_logs, the query that displaced it was not (#2463).
     sources = [c.kwargs["source"] for c in rec.call_args_list]
-    assert sources == ["auto_inject", "reuse_slot"]
+    assert sources == ["auto_inject", "reuse_slot", "lesson_slot"]
 
 
 @pytest.mark.asyncio
@@ -331,14 +333,34 @@ async def test_build_session_context_caps_length():
 _CFG = {"enabled": True, "threshold": 0.55, "top_k": 3}
 
 
-async def _autoinject(main_hits, reuse_hits, cfg=None):
-    """Run build_autoinject_hint with the two semantic queries stubbed in order:
-    the unscoped pool first, then the reserved reuse query."""
+def _asked_for_reuse(calls: list[dict]) -> bool:
+    """Did the reuse slot issue its reserved query on this run?"""
+    return any(
+        tuple(c.get("note_type") or ()) == pc_module._REUSE_KINDS for c in calls
+    )
+
+
+async def _autoinject(main_hits, reuse_hits, cfg=None, lesson_hits=None):
+    """Run build_autoinject_hint with each semantic query stubbed by the kinds
+    it asks for: the unscoped pool, the reserved reuse query, and the reserved
+    lesson query (milestone 385 step 5).
+
+    ROUTED ON THE REQUESTED KINDS, not on call order, and that is the point of
+    the helper. Order-keyed stubbing was fine while there was one reserved
+    slot; with two it makes every test in this section depend on which slot
+    runs first, so adding a third would silently hand one slot another's
+    candidate list and the tests would still pass.
+    """
     calls: list[dict] = []
 
     async def fake_search(*_a, **kw):
         calls.append(kw)
-        return reuse_hits if kw.get("note_type") else main_hits
+        kinds = kw.get("note_type") or ()
+        if LESSON_NOTE_TYPE in kinds:
+            return lesson_hits or []
+        if kinds:
+            return reuse_hits
+        return main_hits
 
     with patch("scribe.services.plugin_context.get_autoinject_config",
                AsyncMock(return_value=dict(cfg or _CFG))), \
@@ -382,7 +404,10 @@ async def test_the_reserved_query_is_skipped_when_a_snippet_already_won():
 
     out, calls = await _autoinject(main, [])
 
-    assert len(calls) == 1                      # reserved query never ran
+    # WHICH query ran, not how many. A count here is a claim about every
+    # reserved slot on this path at once, so it fails the day an unrelated one
+    # is added — and the thing being tested is that THIS slot stood down.
+    assert not _asked_for_reuse(calls)
     assert out["note_ids"] == [9, 1]
 
 
@@ -396,7 +421,7 @@ async def test_a_weak_snippet_does_not_buy_the_slot():
     out, calls = await _autoinject(main, [])     # threshold returned nothing
 
     assert out["note_ids"] == [1]
-    assert len(calls) == 2                       # it asked, and got nothing
+    assert _asked_for_reuse(calls)               # it asked, and got nothing
 
 
 @pytest.mark.asyncio
@@ -425,7 +450,7 @@ async def test_a_process_counts_as_reuse_too():
     # …and one already on the menu suppresses the reserved query.
     out2, calls2 = await _autoinject(
         [(0.70, fake_note(id=8, title="DRY pass process", user_id=1, note_type="process"))], [])
-    assert len(calls2) == 1
+    assert not _asked_for_reuse(calls2)
 
 
 # --- write-path widened beyond snippets (#2246, the mirror half) -------------
@@ -456,7 +481,7 @@ async def test_write_path_semantic_arm_asks_for_experience_not_just_snippets():
         )
 
     kw = search.await_args.kwargs
-    assert kw["note_type"] == ("snippet", "note")
+    assert kw["note_type"] == ("snippet", "note", LESSON_NOTE_TYPE)
     # An open to-do resembling the code answers nothing; an ISSUE carries a root
     # cause and a NOTE carries durable knowledge. Only the todo is excluded.
     assert kw["task_kind"] == "issue"

tests/test_write_path_trigger.py

@@ -253,9 +253,11 @@ async def test_semantic_arm_is_snippet_only_and_browse_scoped():
         await pc.build_write_path_hint(1, "src/x.py", code=REAL_CODE)
     kwargs = search.await_args.kwargs
     # Prior art is snippets AND recorded experience (#2246) — an issue saying
-    # "we tried this and it broke" belongs here. What stays out is the open
-    # to-do list, which resembles the code and answers nothing.
-    assert kwargs["note_type"] == ("snippet", "note")
+    # "we tried this and it broke" belongs here — AND lessons (milestone 385
+    # step 5), whose founding example is a lesson about a code shape. What
+    # stays out is the open to-do list, which resembles the code and answers
+    # nothing.
+    assert kwargs["note_type"] == ("snippet", "note", "lesson")
     assert kwargs["task_kind"] == "issue"
     # Nobody asked for this, so it must not reach a one-to-one direct share.
     assert kwargs["scope"] == "browse"