Files
FabledScribe/tests/test_lesson_rest_door.py
T
bvandeusenandClaude Opus 5.5 66e21a6c60
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 14s
CI & Build / integration (push) Successful in 52s
CI & Build / TypeScript typecheck (push) Successful in 53s
CI & Build / Python tests (push) Successful in 1m35s
CI & Build / Build & push image (push) Successful in 32s
refactor(notes): a snippet's and lesson's stored title is its name; the trigger joins it only in the embedded document (milestone 427)
The title was `subject — trigger` because the stored title WAS the
embedded one, and the join is what makes these kinds rank on the
situation they apply to (#2485). Every surface that shows a title then
showed the trigger too -- menus, lists and search rows ran to kilobytes.

- embeddings.document_title(title, note_type, data, body) joins the
  trigger from `data` (body fallback) at embed time. Idempotent: an
  un-migrated composed title comes out the same, never doubled. The
  embed path, the startup backfill and the dedup gate's semantic signal
  all use it, so the embedded text -- and every vector -- is unchanged.
- Writers store the subject: snippet create/update (service, REST, MCP)
  and lesson_document. Both compose_title helpers are removed.
- Readers: dedup takes `data`; the menus strip the embedded title from a
  passage; list rows project `when_to_use`, which SnippetListView reads.
- 0108 rewrites existing rows on an exact `' — ' || <own trigger>`
  suffix with raw SQL, leaving updated_at alone so the backfill does not
  re-embed the corpus for identical vectors. Downgrade recomposes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-09-23 16:48:28 -04:00

247 lines
9.4 KiB
Python

"""The REST door for lessons — the half the web UI can actually reach (#3734).
WHY THIS EXISTS AT ALL
Milestone 385 built the lesson kind through the MCP tools, which is the surface
an agent uses. The Vue app speaks REST, so until this blueprint existed a lesson
was a record a person could not create, read, edit or retire from the UI — rule
27's "no UI, no ship" failing at the door rather than in the view.
WHAT THESE PIN
Three things a second door tends to get wrong, and one that is specific to this
kind:
- PARITY. Both doors go through services/lessons.py, so the composed
document is identical whichever one wrote it. A REST door that composed its
own title would produce lessons that rank differently from the agent's, and
the document IS what ranks.
- ACL (rule 78). Share-aware resolve, write as the owner — the pattern
routes/snippets.py sets — so a shared editor isn't rejected by the
owner-scoped service.
- THE TRIGGER IS REFUSED WHEN EMPTY. This is the kind-specific one and the
reason the door is not a thin wrapper. The service will happily store a
lesson with no trigger: it saves, it reads correctly in every listing, and
it never surfaces. There is nothing to notice afterwards — it looks exactly
like a lesson that works. So the door refuses it at both create and update
rather than handing back a record that looks finished.
- THE REVERSE DIRECTION. `taught-by/<id>` answers "what was learned from this
record", which the task body calls the direction that gets forgotten and
arguably the more useful one.
"""
import inspect
import pytest
# ── parity: one composer, two doors ──────────────────────────────────────────
def test_both_doors_share_one_serializer():
"""A payload shape spelled once per door answers the two of them
differently the first time a field is added."""
from scribe.mcp.tools import lessons as mcp_lessons
from scribe.services import lessons as lessons_svc
assert mcp_lessons._to_dict is lessons_svc.lesson_to_dict
def test_the_rest_door_composes_nothing_itself():
"""Asserted on structure (rule 167). The document is what ranks, so a door
that built its own title would produce lessons that rank differently from
the ones the agent writes."""
from scribe.routes import lessons as routes
src = inspect.getsource(routes)
# It may CALL the service's composer (the dedup gate needs the document),
# but it must not assemble a title or a trigger line itself.
assert "trigger_title" not in src
assert "**When to apply:**" not in src
assert "lessons_svc.lesson_document" in src, (
"the dedup gate must hash the same document the service will store"
)
def test_the_door_is_registered():
"""A blueprint nobody registers is a file, not a door."""
from scribe import app as app_module
src = inspect.getsource(app_module)
assert "from scribe.routes.lessons import lessons_bp" in src
assert "app.register_blueprint(lessons_bp)" in src
def test_the_blueprint_is_mounted_where_the_client_looks():
from scribe.routes.lessons import lessons_bp
assert lessons_bp.url_prefix == "/api/lessons"
def test_the_reverse_lookup_is_registered_before_the_id_route():
"""Quart matches in registration order. The int converter protects
`taught-by` today, but the ordering is what keeps that true if the
converter is ever widened — the same care snippets' `/duplicates` takes."""
from scribe.routes import lessons as routes
src = inspect.getsource(routes)
assert src.index('"/taught-by/<int:record_id>"') < src.index(
'"/<int:lesson_id>"'
)
# ── the trigger is not optional at this door ─────────────────────────────────
def test_create_refuses_a_lesson_with_no_trigger():
"""The kind-specific guard. A triggerless lesson saves and never surfaces,
and nothing about the stored record shows it."""
from scribe.routes import lessons as routes
src = inspect.getsource(routes.create_lesson_route)
assert "when_to_apply is required" in src
assert "never reaches anyone" in src, (
"the refusal must say WHY, or the next person reads it as a nag and "
"removes it"
)
def test_update_refuses_to_clear_the_trigger():
"""The other half. Creating without one is refused; emptying one later
would reach the same broken state by a different path."""
from scribe.routes import lessons as routes
src = inspect.getsource(routes.update_lesson_route)
assert "cannot be cleared" in src
def test_the_subject_is_required_too():
from scribe.routes import lessons as routes
src = inspect.getsource(routes.create_lesson_route)
assert "what is required" in src
# ── ACL: rule 78's pattern, not a bare owner filter ──────────────────────────
@pytest.mark.parametrize(
"handler", ["update_lesson_route", "delete_lesson_route"]
)
def test_writes_check_permission_and_act_as_the_owner(handler):
"""Resolve share-aware, then write as the owner — otherwise a shared
editor is rejected by the owner-scoped service."""
from scribe.routes import lessons as routes
src = inspect.getsource(getattr(routes, handler))
assert "can_write_note" in src, f"{handler} does not check write permission"
assert "note.user_id" in src, (
f"{handler} writes as the caller rather than as the owner, which "
f"rejects a legitimately shared editor (rule 78)"
)
def test_no_handler_builds_its_own_owner_filter():
"""Rule 78's actual failure mode: a route assembling its own
`Note.user_id == uid` clause instead of going through the service and the
access helpers. Passing `user_id=uid` INTO a service is the correct call
and is not what this looks for."""
from scribe.routes import lessons as routes
src = inspect.getsource(routes)
assert "Note.user_id" not in src
assert "select(" not in src, (
"a route composing its own query has bypassed the access helpers"
)
def test_delete_trashes_recoverably():
"""Every kind's delete is a trash, and the batch id is what restores it."""
from scribe.routes import lessons as routes
src = inspect.getsource(routes.delete_lesson_route)
assert "trash_svc.delete" in src
assert "deleted_batch_id" in src
# ── the reverse direction ────────────────────────────────────────────────────
def test_the_reverse_lookup_reads_the_indexed_mirror():
"""Not by scanning bodies: `data[taught_by]` is JSONB with a GIN index
(0070), which is the whole reason step 4 put the list there."""
from scribe.services import lessons as lessons_svc
src = inspect.getsource(lessons_svc.lessons_taught_by)
assert "path_exists" in src
assert "SOURCES_KEY" in src
def test_the_reverse_lookup_is_share_aware():
"""It renders beside a record the caller can already see, so a lesson
someone shared with them belongs in the list exactly as their own does."""
from scribe.services import lessons as lessons_svc
src = inspect.getsource(lessons_svc.lessons_taught_by)
assert "readable_notes_clause" in src
assert "deleted_at" in src
def test_the_reverse_lookup_refuses_a_nonsense_id_rather_than_interpolating():
"""The jsonpath is built by formatting, so the id has to be an int before
it gets near the expression."""
from scribe.services import lessons as lessons_svc
src = inspect.getsource(lessons_svc.lessons_taught_by)
assert "int(record_id)" in src
@pytest.mark.asyncio
async def test_a_bad_id_returns_nothing_and_raises_nothing():
from scribe.services.lessons import lessons_taught_by
assert await lessons_taught_by(1, 0) == []
assert await lessons_taught_by(1, -3) == []
assert await lessons_taught_by(1, "not a number") == []
# ── the serializer speaks the vocabulary the caller wrote with ───────────────
def test_the_payload_reads_back_the_composed_fields():
"""A caller that wrote `when_to_apply` reads `when_to_apply` back, not a
body it has to parse."""
from tests.helpers import fake_lesson
from scribe.services.lessons import compose_body, lesson_to_dict
what = "Read the job log before waiting longer"
trigger = "a CI run has sat in_progress longer than its suite takes"
note = fake_lesson(
id=7,
title=what,
body=compose_body("The work is usually done.", trigger, [4181]),
data={"what": what, "when_to_apply": trigger, "taught_by": [4181]},
project_id=None,
created_at=None,
updated_at=None,
)
out = lesson_to_dict(note)
assert out["what"] == what
assert out["when_to_apply"] == trigger
assert out["learned_from"] == [4181]
# The insight comes back WITHOUT the lines compose_body added, so an edit
# form round-trips instead of accumulating a copy of them per save.
assert out["insight"] == "The work is usually done."
assert "**When to apply:**" not in out["insight"]
def test_the_rest_guards_can_fail():
"""Rule 167: shown turning red once."""
from scribe.routes import lessons as routes
src = inspect.getsource(routes)
assert "a phrase that is definitely not in this module" not in src
assert "lessons_bp" in src