Files
FabledScribe/tests/test_lesson_surfacing.py
T
bvandeusenandClaude Opus 5 1252d0e305
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / integration (push) Successful in 49s
CI & Build / TypeScript typecheck (push) Successful in 53s
CI & Build / Python tests (push) Successful in 1m31s
CI & Build / Build & push image (push) Successful in 23s
fix(lessons): a derived mirror survives the generic note door, by kind not by name (#3734)
Groundwork for step 7, and a data-integrity fix in its own right.

Two kinds keep a queryable mirror in `notes.data` derived from their body:
snippets and, since milestone 385, lessons. Every read prefers the mirror —
deliberately, because parsing markdown to answer what an index can answer is
how a hot path rots. So a write that moves the body must move the mirror.

#3128 found that hole for snippets and plugged it with a hard-coded
`if note.note_type == SNIPPET_NOTE_TYPE`. The plug was correct and did not
generalise: lessons arrived with the same design and none of the protection,
which is precisely the "don't add a fourth instance" defect #3734 was told to
avoid.

The cost is higher for a lesson. A stale snippet mirror reports the wrong
path. A stale lesson mirror reports the wrong TRIGGER, and the trigger is the
whole retrieval story — the lesson goes on firing for the situation it used
to name while displaying the one it now names. Silent, and confident.

So `update_note` now dispatches through `_mirror_recomposers()`, a
note_type -> recomposer table. A kind with a derived mirror is covered by
registering it, not by someone remembering to widen an if.

`lessons.recompose_data` is the lesson's entry. It recovers the subject with
`embeddings.untrigger_title` — new, and deliberately placed beside the join it
inverts rather than in the caller that wanted it, because a separator spelled
in two files is a separator that will one day be changed in one of them
(#3207). `TRIGGER_SEP` is now the one spelling, and `parse_snippet_fields`
uses it too; it had the third copy inline.

The two inverses stay distinct on purpose: a snippet partitions at the first
separator (its name is a symbol), a lesson strips an exact known suffix (its
subject may legitimately contain a dash). Different algorithms, one constant,
so they cannot disagree about where the seam is.

Provenance is DROPPED when the body drops it, which is the opposite call from
a snippet's `verification` — that is carried because it was never in the body
to delete. The body is the authority; carrying a value the reader just removed
is the failure the recompose exists to prevent.

Tests: test_snippet_mirror_generic_door.py becomes
test_derived_mirror_generic_door.py, since the concern is now plural. The
registry property is asserted directly (every kind with a mirror is in the
table; the dispatch names no kind inline), plus the lesson cases and the
join/inverse round-trip. `fake_lesson` moves to tests/helpers.py — it existed
in test_lesson_surfacing.py and a second copy was about to be written — and
gains the explicit `None`s `fake_snippet` carries, because update_note reads
`verify_with` and a MagicMock is truthy.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy
2026-09-19 13:58:54 -04:00

289 lines
13 KiB
Python

"""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_lesson, 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"
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."
)