CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / integration (push) Successful in 48s
CI & Build / TypeScript typecheck (push) Successful in 54s
CI & Build / Python tests (push) Successful in 1m41s
CI & Build / Build & push image (push) Successful in 30s
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
208 lines
9.1 KiB
Python
208 lines
9.1 KiB
Python
"""Creating a lesson, and tying it to what taught it (milestone 385 step 4).
|
|
|
|
THE TRIGGER IS THE RECORD, so the door refuses a lesson without one.
|
|
|
|
Step 1 could have chosen "flag it visibly" instead. Refusing is the stronger
|
|
answer for the same reason `create_rule` makes enforcement the deciding
|
|
question: a lesson with no trigger is not a weaker lesson, it is a note that
|
|
will never surface, and nothing downstream can tell the difference. It saves,
|
|
it reads correctly in every listing, and it is silently absent from the one
|
|
moment it was written for. A flag would be a warning nobody is present to read
|
|
— the write path is where the writer still is.
|
|
|
|
THE SOURCE IDS ARE A LIST, and `test_a_lesson_keeps_every_incident_that_taught
|
|
_it` is why. The founding example generalised three incidents into one claim;
|
|
`arose_from_id` holds one, and a record that keeps the first and drops two
|
|
reads as complete.
|
|
"""
|
|
from __future__ import annotations
|
|
|
|
from types import SimpleNamespace
|
|
from unittest.mock import AsyncMock, patch
|
|
|
|
import pytest
|
|
|
|
from scribe.mcp._context import _user_id_ctx
|
|
from scribe.mcp.tools.lessons import create_lesson, update_lesson
|
|
from scribe.services import lessons as lessons_svc
|
|
|
|
TRIGGER = "a test fails on code you believe is correct"
|
|
SUBJECT = "Suspect the guard before the code"
|
|
|
|
|
|
def _stub_note(**kw):
|
|
"""A stand-in lesson row — every attribute the tool's _to_dict reads."""
|
|
base = dict(
|
|
id=1, title="t", body="b", tags=[], project_id=None,
|
|
note_type="lesson", data={}, arose_from_id=None,
|
|
created_at=None, updated_at=None,
|
|
)
|
|
base.update(kw)
|
|
return SimpleNamespace(**base)
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_a_lesson_without_a_trigger_is_refused():
|
|
"""THE guard. Falsifiable: it names the parameter, so a door that stopped
|
|
asking for it fails here rather than quietly writing an unfindable record."""
|
|
_user_id_ctx.set(7)
|
|
with pytest.raises(ValueError) as err:
|
|
await create_lesson(what=SUBJECT, when_to_apply="")
|
|
|
|
message = str(err.value)
|
|
assert "when_to_apply" in message
|
|
# It says what to write, not just that something is missing — the writer is
|
|
# here now, and "required" alone produces a topic where a symptom was wanted.
|
|
assert "symptom" in message.lower()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_whitespace_is_not_a_trigger():
|
|
"""The refusal reads the stripped value. A door checking only falsiness
|
|
accepts a space and produces exactly the record it meant to prevent."""
|
|
_user_id_ctx.set(7)
|
|
with pytest.raises(ValueError):
|
|
await create_lesson(what=SUBJECT, when_to_apply=" ")
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_a_lesson_keeps_every_incident_that_taught_it():
|
|
"""Three sources in, three sources stored — and `arose_from_id` left NULL,
|
|
because one of three on a surface that renders it as THE origin would make
|
|
that surface state something false."""
|
|
_user_id_ctx.set(7)
|
|
created = AsyncMock(return_value=_stub_note())
|
|
with patch.object(lessons_svc, "create_lesson", created), \
|
|
patch("scribe.mcp.tools.lessons.dedup_svc.find_duplicate_note",
|
|
AsyncMock(return_value=None)), \
|
|
patch("scribe.mcp.tools.lessons.systems_tools.attach_systems", AsyncMock()):
|
|
await create_lesson(
|
|
what=SUBJECT, when_to_apply=TRIGGER, learned_from=[11, 22, 33],
|
|
)
|
|
|
|
assert created.await_args.kwargs["learned_from"] == [11, 22, 33]
|
|
assert lessons_svc.sole_source([11, 22, 33]) is None
|
|
assert lessons_svc.compose_data(SUBJECT, TRIGGER, [11, 22, 33])["taught_by"] == [
|
|
11, 22, 33,
|
|
]
|
|
|
|
|
|
def test_a_single_source_still_reaches_arose_from_id():
|
|
"""The existing provenance field keeps working for the ordinary case — a
|
|
lesson drawn from one issue is not made less connected by the list."""
|
|
assert lessons_svc.sole_source([11]) == 11
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_the_duplicate_gate_runs_before_anything_is_created():
|
|
"""A near-match returns the existing id and writes nothing: two lessons
|
|
about one failure class want to be one lesson."""
|
|
_user_id_ctx.set(7)
|
|
hit = SimpleNamespace(id=99, title="already recorded", similarity=0.97,
|
|
reason="semantic")
|
|
created = AsyncMock()
|
|
with patch("scribe.mcp.tools.lessons.dedup_svc.find_duplicate_note",
|
|
AsyncMock(return_value=hit)), \
|
|
patch.object(lessons_svc, "create_lesson", created):
|
|
out = await create_lesson(what=SUBJECT, when_to_apply=TRIGGER)
|
|
|
|
assert created.await_count == 0
|
|
assert out["duplicate"] is True
|
|
assert out["existing_id"] == 99
|
|
# The hint names the lesson's own updater, so the caller is pointed at a
|
|
# tool that exists rather than at update_note.
|
|
assert "update_lesson" in out["message"]
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_the_gate_compares_the_composed_document_not_the_raw_fields():
|
|
"""What reaches the gate is the title and body a lesson will actually be
|
|
stored as, plus the `data` its EMBEDDED title is built from (milestone
|
|
427). Comparing `what` alone would miss that the trigger is half the
|
|
document, and would judge two lessons alike that rank nothing alike."""
|
|
_user_id_ctx.set(7)
|
|
gate = AsyncMock(return_value=None)
|
|
with patch("scribe.mcp.tools.lessons.dedup_svc.find_duplicate_note", gate), \
|
|
patch.object(lessons_svc, "create_lesson",
|
|
AsyncMock(return_value=_stub_note())), \
|
|
patch("scribe.mcp.tools.lessons.systems_tools.attach_systems", AsyncMock()):
|
|
await create_lesson(what=SUBJECT, when_to_apply=TRIGGER, insight="Look.")
|
|
|
|
title, body = gate.await_args.args[1], gate.await_args.args[2]
|
|
assert title == SUBJECT
|
|
assert body.startswith(f"**When to apply:** {TRIGGER}")
|
|
assert gate.await_args.kwargs["data"]["when_to_apply"] == TRIGGER
|
|
assert gate.await_args.kwargs["note_type"] == "lesson"
|
|
|
|
|
|
def test_a_lesson_is_judged_at_the_trigger_dominated_bar():
|
|
"""#2518 measured deliberately-parallel siblings at 0.92 on a document that
|
|
is mostly prose ABOUT the thing. A lesson's document is that shape, so the
|
|
bar sits above that band — otherwise two different lessons about one area
|
|
block each other, which is the failure step 4 asked to check for."""
|
|
from scribe.services.dedup import (
|
|
_LESSON_SEMANTIC_THRESHOLD,
|
|
_SEMANTIC_THRESHOLD,
|
|
_semantic_threshold,
|
|
)
|
|
|
|
assert _semantic_threshold("lesson") == _LESSON_SEMANTIC_THRESHOLD
|
|
assert _LESSON_SEMANTIC_THRESHOLD > 0.92, (
|
|
"below the observed sibling band, so two genuinely different lessons "
|
|
"about one area would refuse each other"
|
|
)
|
|
assert _LESSON_SEMANTIC_THRESHOLD > _SEMANTIC_THRESHOLD
|
|
# The carve-out is per kind, not a loosening of the gate: a lesson's bar is
|
|
# its own, apart from the note's copy band (#4306).
|
|
from scribe.services.dedup import _NOTE_COPY_THRESHOLD
|
|
|
|
assert _semantic_threshold("note") == _NOTE_COPY_THRESHOLD
|
|
assert _NOTE_COPY_THRESHOLD != _LESSON_SEMANTIC_THRESHOLD
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_an_update_recomposes_both_halves_of_the_document():
|
|
"""A new trigger has to reach the mirror AND the head of the body — the two
|
|
places the embedded document reads it from (milestone 427). Patching one
|
|
would leave a lesson that reads correctly and ranks on the old situation —
|
|
the failure mode with no symptom. The title stays the subject, even when
|
|
the stored one was an un-migrated composed title."""
|
|
_user_id_ctx.set(7)
|
|
stored = _stub_note(
|
|
title=f"{SUBJECT} — {TRIGGER}",
|
|
body=f"**When to apply:** {TRIGGER}\n\nLook at the guard.",
|
|
data={"what": SUBJECT, "when_to_apply": TRIGGER},
|
|
)
|
|
updated = AsyncMock(return_value=_stub_note())
|
|
with patch.object(lessons_svc, "get_lesson", AsyncMock(return_value=stored)), \
|
|
patch("scribe.services.notes.update_note", updated):
|
|
await lessons_svc.update_lesson(7, 1, when_to_apply="a guard goes red")
|
|
|
|
fields = updated.await_args.kwargs
|
|
assert fields["title"] == SUBJECT
|
|
assert fields["body"].startswith("**When to apply:** a guard goes red")
|
|
assert fields["data"]["when_to_apply"] == "a guard goes red"
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_an_update_does_not_stack_the_composed_lines():
|
|
"""The insight is handed back to `compose_body`, which re-adds the trigger
|
|
and provenance lines. Without stripping them first they accumulate a copy
|
|
per edit — and because the trigger line is half of what makes the document
|
|
rank, the duplicates would look like the shape working."""
|
|
_user_id_ctx.set(7)
|
|
stored = _stub_note(
|
|
body=f"**When to apply:** {TRIGGER}\n\nLook at the guard."
|
|
"\n\n**Learned from:** #5",
|
|
data={"what": SUBJECT, "when_to_apply": TRIGGER, "taught_by": [5]},
|
|
)
|
|
updated = AsyncMock(return_value=_stub_note())
|
|
with patch.object(lessons_svc, "get_lesson", AsyncMock(return_value=stored)), \
|
|
patch("scribe.services.notes.update_note", updated):
|
|
await lessons_svc.update_lesson(7, 1, what="Suspect the guard")
|
|
|
|
body = updated.await_args.kwargs["body"]
|
|
assert body.count("**When to apply:**") == 1
|
|
assert body.count("**Learned from:**") == 1
|
|
assert "Look at the guard." in body
|