CI & Build / Python lint (push) Successful in 2s
CI & Build / Plugin hooks (push) Successful in 14s
CI & Build / TypeScript typecheck (push) Successful in 54s
CI & Build / integration (push) Successful in 1m3s
CI & Build / Python tests (push) Successful in 1m55s
CI & Build / Build & push image (push) Successful in 13s
Six more test files state their pinned properties without the issue and milestone history: the pull-telemetry guard explains enumerating kinds instead of listing two getter incidents, cited-record status explains why a hint's id needs its status instead of recounting one misreported step, and the trigger-shape guard keeps its reasoning about why prose heuristics fail while dropping the dated corpus audit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
197 lines
8.5 KiB
Python
197 lines
8.5 KiB
Python
"""A lesson's surfaced-vs-opened counts reach a reader.
|
|
|
|
Every door — the REST list and detail, both MCP doors, and the view — carries
|
|
the counts, because the one kind whose central question is "is this trigger
|
|
firing on the right situation" needs that answer readable.
|
|
|
|
WHY THE COUNTS MATTER MORE HERE THAN ON A SNIPPET. Whether a lesson repeatedly
|
|
followed should become a rule has a trap: raw frequency
|
|
cannot tell "this should bind" from "this trigger is too broad", which is the
|
|
commoner reading. Surfaced-AND-opened can. Neither question is answerable by
|
|
anything that cannot see the numbers, which is why this is the first step and
|
|
not the threshold.
|
|
|
|
NO THRESHOLD IS ASSERTED ANYWHERE HERE, deliberately. A promotion rule fitted
|
|
to a small corpus would be fitting noise, and `UsageBadge` already declines
|
|
to render a verdict on fewer than three surfacings for the same reason.
|
|
"""
|
|
import inspect
|
|
from pathlib import Path
|
|
from unittest.mock import AsyncMock, MagicMock, patch
|
|
|
|
import pytest
|
|
|
|
from scribe.mcp.tools import lessons as lesson_tools
|
|
from scribe.services import note_usage
|
|
from scribe.services.note_usage import empty_usage
|
|
|
|
|
|
def _rows():
|
|
return [
|
|
{"id": 7, "title": "a", "tags": [], "snippet": "", "when_to_apply": "x"},
|
|
{"id": 9, "title": "b", "tags": [], "snippet": "", "when_to_apply": "y"},
|
|
]
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_the_mcp_listing_carries_usage_for_every_row():
|
|
"""Zero-filled, not omitted. A missing key would make the reader treat
|
|
"never surfaced" and "not reported" as the same thing, which is the
|
|
distinction the whole readout exists to keep."""
|
|
used = {7: {**empty_usage(), "surfaced_count": 5, "pull_count": 2}}
|
|
with (
|
|
patch.object(lesson_tools, "current_user_id", return_value=1),
|
|
patch.object(lesson_tools.knowledge_svc, "query_knowledge",
|
|
new=AsyncMock(return_value=(_rows(), 2))),
|
|
patch.object(lesson_tools.access_svc, "label_shared_items",
|
|
new=AsyncMock(side_effect=lambda _uid, items: items)),
|
|
patch.object(note_usage, "usage_for_notes",
|
|
new=AsyncMock(return_value=used)),
|
|
):
|
|
out = await lesson_tools.list_lessons()
|
|
|
|
by_id = {r["id"]: r for r in out["lessons"]}
|
|
assert by_id[7]["usage"]["surfaced_count"] == 5
|
|
assert by_id[7]["usage"]["pull_count"] == 2
|
|
# The row nobody has surfaced still carries the key.
|
|
assert by_id[9]["usage"] == empty_usage()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_the_listing_asks_for_usage_once_for_the_whole_page():
|
|
"""One aggregate, not one lookup per row — the snippet listing's own
|
|
comment calls a per-row read N+1 by construction, and a lesson list is the
|
|
same shape."""
|
|
reader = AsyncMock(return_value={})
|
|
with (
|
|
patch.object(lesson_tools, "current_user_id", return_value=1),
|
|
patch.object(lesson_tools.knowledge_svc, "query_knowledge",
|
|
new=AsyncMock(return_value=(_rows(), 2))),
|
|
patch.object(lesson_tools.access_svc, "label_shared_items",
|
|
new=AsyncMock(side_effect=lambda _uid, items: items)),
|
|
patch.object(note_usage, "usage_for_notes", new=reader),
|
|
):
|
|
await lesson_tools.list_lessons()
|
|
|
|
assert reader.await_count == 1
|
|
assert sorted(reader.await_args.args[0]) == [7, 9]
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_get_lesson_reads_the_count_before_recording_its_own_pull():
|
|
"""Otherwise the first read of a lesson reports a pull that is its own.
|
|
|
|
`get_lesson` records a pull on every open — it has to, or the kind sits
|
|
permanently at zero and reads as dead weight. That makes the ORDER load
|
|
bearing in a way it is not for kinds that only count.
|
|
"""
|
|
order: list[str] = []
|
|
note = MagicMock(id=7, user_id=1)
|
|
|
|
async def _usage_read(ids):
|
|
order.append("read")
|
|
return {7: {**empty_usage(), "surfaced_count": 4, "pull_count": 1}}
|
|
|
|
with (
|
|
patch.object(lesson_tools, "current_user_id", return_value=1),
|
|
patch.object(lesson_tools.lessons_svc, "get_lesson",
|
|
new=AsyncMock(return_value=note)),
|
|
patch.object(lesson_tools, "_to_dict", return_value={"id": 7}),
|
|
patch.object(lesson_tools.access_svc, "describe_provenance",
|
|
new=AsyncMock(return_value={})),
|
|
patch.object(note_usage, "usage_for_notes", new=_usage_read),
|
|
patch.object(lesson_tools, "record_pulled",
|
|
side_effect=lambda **_kw: order.append("pull")),
|
|
):
|
|
out = await lesson_tools.get_lesson(7)
|
|
|
|
assert order == ["read", "pull"], (
|
|
"the pull was recorded before the count was read, so the number the "
|
|
"caller is shown includes the read that produced it"
|
|
)
|
|
assert out["usage"]["pull_count"] == 1
|
|
|
|
|
|
# ── the REST door, on structure ──────────────────────────────────────────────
|
|
#
|
|
# `usage_for_notes(` and `empty_usage()` live in ONE seam,
|
|
# `note_usage.attach_usage`, which seven doors share — so the
|
|
# zero-fill and the single aggregate are pinned once, against the seam, in
|
|
# tests/test_usage_attach_seam.py. What stays HERE is what only this route can
|
|
# get wrong: that it calls the seam at all, once, and before it records a pull.
|
|
#
|
|
# Its siblings in test_lesson_rest_door.py are source guards for the same
|
|
# reason: the route is decorated and returns a Quart response, so driving it
|
|
# means standing up the app. What matters here is reachable from the source
|
|
# and falsifiable from it — an aggregate rather than a per-row read, and the
|
|
# same read-then-record order the MCP door is pinned to above.
|
|
|
|
|
|
def _route_source(name: str) -> str:
|
|
from scribe.routes import lessons as routes
|
|
|
|
src = inspect.getsource(routes)
|
|
start = src.index(f"async def {name}(")
|
|
nxt = src.find("\n@lessons_bp.route", start)
|
|
return src[start:nxt if nxt > 0 else len(src)]
|
|
|
|
|
|
def test_the_rest_listing_reads_usage_once_for_the_page():
|
|
"""A per-row lookup would be N+1 by construction — the listing's own
|
|
comment says so, and this is what makes that comment checkable."""
|
|
src = _route_source("list_lessons_route")
|
|
assert src.count("attach_usage(") == 1
|
|
assert "attach_usage(items)" in src, "the seam must get the page, not a row"
|
|
|
|
|
|
def test_every_rest_row_carries_the_key_even_at_zero():
|
|
""""Never surfaced" is a state the UI renders; a missing field is not."""
|
|
src = _route_source("list_lessons_route")
|
|
assert "attach_usage(" in src
|
|
assert 'it["usage"] =' not in src, (
|
|
"the route re-spells the attach by hand, so a row with no recorded "
|
|
"usage can come back without the key again"
|
|
)
|
|
|
|
|
|
def test_the_rest_detail_door_also_reads_before_it_records():
|
|
"""Two doors that disagree about what the number counts are worse than
|
|
one door that is wrong, because only one of them looks wrong."""
|
|
src = _route_source("get_lesson_route")
|
|
assert src.index("attach_usage(") < src.index("record_pulled(")
|
|
|
|
|
|
def test_these_guards_can_fail():
|
|
"""Falsify the shape they assert against, so a guard that has
|
|
quietly stopped describing anything cannot pass by describing nothing."""
|
|
src = _route_source("list_lessons_route")
|
|
assert "def list_lessons_route" in src
|
|
assert "def get_lesson_route" not in src, "the slice ran past its route"
|
|
|
|
|
|
def test_the_detail_view_renders_the_badge_rather_than_respelling_it():
|
|
"""Reach for UsageBadge and pass the kind's own advice; a view's scoped
|
|
re-spelling of `.usage-tag` is what that component exists to prevent.
|
|
|
|
The advice is the kind-specific half. For a lesson it points at the
|
|
trigger, not at deletion — a lesson nobody opens is usually keyed to a
|
|
situation nobody is in, which is `update_lesson`'s own words and the
|
|
commoner reading.
|
|
"""
|
|
view = (Path(__file__).resolve().parents[1]
|
|
/ "frontend/src/views/LessonDetailView.vue").read_text()
|
|
assert "UsageBadge" in view
|
|
assert "usage-tag" not in view, "re-spelled the chip instead of reusing it"
|
|
|
|
# The sentence lives in the shared table, not this template. Assert BOTH
|
|
# halves, so the guard cannot pass by the
|
|
# view pointing at an entry that no longer says the right thing.
|
|
assert "DEAD_WEIGHT_ADVICE.lesson" in view, "the view no longer reads the lesson advice"
|
|
table = (Path(__file__).resolve().parents[1]
|
|
/ "frontend/src/utils/deadWeight.ts").read_text()
|
|
start = table.index(" lesson:")
|
|
advice = table[start:table.index(",\n", start)]
|
|
assert "when_to_apply" in advice, (
|
|
f"the dead-weight advice does not point at the trigger: {advice!r}"
|
|
)
|