Files
FabledScribe/src/scribe/services/lessons.py
T
bvandeusenandClaude Opus 5 95dc25eaab
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 11s
CI & Build / TypeScript typecheck (push) Failing after 31s
CI & Build / integration (push) Successful in 48s
CI & Build / Python tests (push) Successful in 1m33s
CI & Build / Build & push image (push) Skipped
feat(lessons): a lesson is readable, writable and browsable by a human (#3734)
Step 7's actual UI. Before this the frontend had zero lesson code — the kind
existed for agents only, which is rule 27 failing.

THE EDITOR ASKS FOR THE TRIGGER BY NAME, and leads with it. Three fields —
the trigger, the claim, the detail — never one markdown box. That is the
design step 1 settled, and the evidence is blunt: the snippet corpus carries
a trigger on every record with no guard anywhere, because a service composes
the title from a named parameter. What is at 100% is a named structured
field, not a writer remembering a convention. The trigger gets the most room,
its own explanation, and a save button that refuses without it and says why.

The form shows the composed title live, so the writer is agreeing to a
document they can read rather than one assembled out of sight. A 409 from the
duplicate gate is rendered as the record that already covers the moment, with
a link to improve it and an explicit override — not as a failure.

THE BROWSE VOCABULARY GAINS THE KIND, which #3161 warned this step not to get
wrong: a facet chip, a badge label, and routing to `/lessons/:id` rather than
the note editor, which cannot edit a trigger. The badge is neutral alongside
snippet and process — a hue would make the softest record in the corpus look
like the loudest, next to a rule that actually binds.

BOTH DIRECTIONS OF THE PROVENANCE. The detail page resolves `learned_from` to
titles rather than bare ids, because "#4181" tells a reader nothing about
whether it is worth opening. And `LessonsTaughtPanel` answers the reverse on
the record's own page — the direction the task body calls the one that gets
forgotten. It has no author to type it, which is exactly why it tends never
to get built. A component, not markup in the task editor, so the same panel
mounts on any record a lesson can cite instead of being written a second time
(#3207). Silent when empty: most records taught no lesson, and a panel that
says "None yet" everywhere is one people learn to skip.

GLOBAL-BY-DEFAULT IS MADE LEGIBLE. A lesson meeting you on a project it was
not written on reads as a bug unless the page says otherwise, so the origin
line says it as a property of the kind rather than as an apology.

Design system tokens throughout; no new raw hex. `--fs-error` rather than
`--fs-danger` — 31 uses against 1.

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

602 lines
26 KiB
Python

"""Lesson service — a transferable insight, retrievable by situation.
A *lesson* is a Note with ``note_type='lesson'``: a better way to think about a
problem, or a solution that transfers, recorded so a later session meets it at
the moment it applies — and **without binding the reader**.
WHY THE KIND EXISTS (milestone 385, from note #3727)
Rules were the only surface that is global AND situation-keyed, so an agent
holding a transferable insight had one door, and that door binds. The observed
symptom was sessions offering rule proposals for things that should not be
rules.
The gap is a document-shape fact, not a threshold:
- a note is embedded as ``title\\nbody`` and is findable by **what it is
about**;
- a rule is embedded as ``{title}{trigger}`` with ``When to apply:``
repeated at the head of the body, so the trigger appears twice in a short
document and dominates the vector — findable by **when it applies**.
No tuning reaches across that: the field the query would match on is simply not
in a note's document. So a lesson carries a trigger and is embedded like a rule,
while staying a note in every other respect.
WHERE THE TRIGGER LIVES (decision #4157, milestone 385 step 1)
In ``notes.data`` under ``when_to_apply``, written through a named parameter and
mirrored into the title and the head of the body — the shape snippets already
use for ``when_to_use``. Not a column on ``notes``.
That decision was measured rather than assumed. The whole snippet corpus —
164 of 164 — carries a ``when_to_use`` with **no guard anywhere**, which refutes
the premise that an unenforced field gets skipped. What it does NOT show is that
an agent types a title convention correctly: ``compose_title`` builds the title
from the parameter, so what is at 100% is a named structured field. A column
would have bought enforceability at the price of deciding, for every note kind
at once, a question nothing had measured.
The mirror is what makes the vector sharp, and it is why nothing re-embeds:
``chunk_document`` is untouched, so ``CHUNKER_VERSION`` does not move. The
trigger reaches the document by being in the text, exactly as a snippet's is.
WHAT A LESSON INHERITS, AND THE CELLS LEFT EMPTY ON PURPOSE (#3163)
A new kind inherits the note machinery wholesale, and #3163 asks which parts it
should NOT get — so that an empty cell is a decision rather than an oversight.
Inherited, all deliberately:
- **versions** — a lesson is reworded as understanding improves, and what it
used to say is worth as much as any note's history.
- **supersession** — the event this most needs. A lesson replaced by a better
lesson is precisely what ``note_supersessions`` models, and the demotion
penalty already exists.
- **trash**, **the share ACL**, **tags**, **project and System tagging**,
**chunked embeddings**, **the near-duplicate gate**.
NOT inherited, and each for a stated reason:
- **status / task_kind / milestone_id** — a lesson is not work. ``is_task`` is
``status is not None``, so a lesson that acquired a status would become a
task and appear in open-work listings. This is the one cell where filling it
in by accident silently changes what the record IS.
- **recurrence** — task-only, and a lesson does not recur.
- **verify_with / expires_when** — available, because they are generic note
fields, but not part of a lesson's contract and not asked for on create. The
milestone-312 distinction is why: those mark a record that asserts a FACT
about someone else's software and can go false unwatched. A lesson is closer
to a norm — "a better way to think about this" has no truth value that rots
on its own. A lesson that does assert such a fact can still carry them.
WHY THERE IS NO MIGRATION
``note_type`` carries **no CHECK constraint** — only ``task_kind`` does
(``notes_task_kind_check``, migrations 0056 / 0065). Migration 0036 added
``note_type`` as plain ``Text`` with a server default and nothing has gated it
since. So rule 36 has nothing to expand here, and the failure it guards against
— a value the database refuses on an instance predating its migration — cannot
arise for this column.
The real vocabulary is ``services.knowledge._FACETS``, which is where a kind
becomes reachable on the browse surface and validated at the door. That is one
table feeding both dialects of the type filter, so adding a kind there is a
single edit — a property #3161 recommended and that landed before this.
"""
from __future__ import annotations
import re
from scribe.models import async_session
from scribe.models.note import Note
LESSON_NOTE_TYPE = "lesson"
# The key in `notes.data`. Named for the field it mirrors on `rules`, because it
# answers the same question and a reader who knows one should not have to learn
# a second word for it.
TRIGGER_KEY = "when_to_apply"
# What taught this lesson: the ids of the issues, tasks or notes it was drawn
# from. A LIST, and that is the whole decision — see `normalize_sources`.
SOURCES_KEY = "taught_by"
# The body's trigger line, and the pattern that reads it back. The body is the
# readable form and the thing that gets embedded; `data` is the queryable
# mirror. Reads prefer the mirror and fall back to this, which is the discipline
# `snippet_fields` follows and the reason a row written before the mirror
# existed is still readable.
_BODY_TRIGGER_RE = re.compile(r"^\*\*When to apply:\*\*\s*(.+?)\s*$", re.M)
# The readable mirror of `data[SOURCES_KEY]`, and the pattern that reads it
# back — the shape snippets use for `**Merged from:** #ids`, for the same
# reason: the body is what a human sees and what survives a row with no `data`.
_BODY_SOURCES_RE = re.compile(r"^\*\*Learned from:\*\*\s*(.+?)\s*$", re.M)
_ID_RE = re.compile(r"#(\d+)")
def lesson_trigger(note) -> str:
"""When this lesson applies, or "" — the mirror first, then the body.
Prefers `data` for the same reason every snippet read does: it is indexed,
and parsing a body to answer a question the database can answer is how a
hot path ends up regexing markdown. The fallback is not dead code — it is
what makes a lesson readable if the mirror is ever absent, and an absent
mirror must degrade to the right answer rather than to silence.
"""
data = getattr(note, "data", None) or {}
from_mirror = (data.get(TRIGGER_KEY) or "").strip() if isinstance(data, dict) else ""
if from_mirror:
return from_mirror
match = _BODY_TRIGGER_RE.search(getattr(note, "body", None) or "")
return match.group(1).strip() if match else ""
def normalize_sources(entries: list | None) -> list[int]:
"""The ids that taught this lesson — ints, de-duplicated, in the order given.
THE CARDINALITY DECISION, and why it is a list.
`arose_from_id` already exists and holds ONE id, which is the obvious first
answer and the wrong one. The lesson that started this milestone generalised
THREE incidents — a badge collision, an un-backfilled column, a duplicated
const — into one claim about failure classes no CI lane can see. Generalising
across incidents is the shape a good lesson HAS, not an edge case. A single
id would keep the first and silently drop the rest, and a record that drops
two of its three sources is worse than one that names none, because it reads
as complete.
It lives in `notes.data` rather than a join table for exactly the reason
decision #4157 put the trigger there: a join table would settle, for every
note kind at once, whether provenance is multi-valued — a question nothing
has measured. `data` is JSONB with a GIN index (0070), so the list is
queryable today and a table can be migrated to later if the need is shown.
Order is history, not sorting: the incidents stay in the sequence the writer
named them, which is the order they were learned in.
"""
out: list[int] = []
seen: set[int] = set()
for raw in entries or []:
ident = raw.get("id") if isinstance(raw, dict) else raw
try:
i = int(ident)
except (TypeError, ValueError):
continue
if i > 0 and i not in seen:
seen.add(i)
out.append(i)
return out
def lesson_sources(note) -> list[int]:
"""What taught this lesson — the mirror first, then the body, then
`arose_from_id`.
Three fallbacks rather than two, because the third is what keeps this
honest on a record written before the kind existed: a note carrying only
`arose_from_id` has exactly one source and this returns it, so a caller
never has to ask which field to read.
"""
data = getattr(note, "data", None) or {}
if isinstance(data, dict):
from_mirror = normalize_sources(data.get(SOURCES_KEY))
if from_mirror:
return from_mirror
match = _BODY_SOURCES_RE.search(getattr(note, "body", None) or "")
if match:
found = normalize_sources(_ID_RE.findall(match.group(1)))
if found:
return found
single = getattr(note, "arose_from_id", None)
return normalize_sources([single]) if single else []
def sole_source(sources: list[int] | None) -> int | None:
"""`arose_from_id` for this lesson: the id when there is exactly ONE.
Left NULL for a lesson drawn from several, deliberately. Every existing
surface that renders provenance reads `arose_from_id` and renders it as
THE origin; handing it one of three would make those surfaces state
something false. Showing nothing there is accurate — there is no single
origin — and `data[SOURCES_KEY]` carries all of them for the surfaces that
know to ask.
"""
ids = normalize_sources(sources)
return ids[0] if len(ids) == 1 else None
def compose_title(what: str, when_to_apply: str = "") -> str:
"""`{what} — {when it applies}`, the half of the document that ranks.
Built HERE rather than asked of the caller, and that distinction is the
whole evidence base for this design: the snippet corpus is at 100% on its
trigger because a service composes the title from a named parameter, not
because agents type separators reliably. A caller made to spell the
convention is the option milestone 385 step 1 rejected.
The join is `embeddings.trigger_title` — shared with rules and snippets, so
the three kinds that rank on a trigger cannot drift apart in how they say
so.
"""
from scribe.services.embeddings import trigger_title
return trigger_title(what, when_to_apply)
def compose_body(
insight: str, when_to_apply: str = "", learned_from: list[int] | None = None,
) -> str:
"""The lesson body — the trigger line first, the insight after.
The mirror of `compose_title` on the other half of the document, and the
reason the pair is what makes a lesson findable: `chunk_document` joins
them as `{title}\\n{body}`, so a lesson composed here states WHEN IT
APPLIES in the title and again in the first line of the body. That is the
twice-in-a-short-document shape note #2485 measured as the only sharp one
in the corpus, reached the way a snippet reaches it — by being in the text
— rather than by a second document builder at embed time.
`**When to apply:**` rather than plain text: the body is the READABLE
form, `data` is the queryable mirror, and `_BODY_TRIGGER_RE` reads this
line back when the mirror is missing. Its markdown must therefore match
what that pattern expects, which is why neither is written by hand
anywhere else.
The insight goes in the body rather than being held out of the document.
`rule_document` excludes a rule's `why` because long dated narrative made
sixteen dev-logs land on the centroid of "development" — but that finding
predates chunking (#280). A body over the budget is now split into several
chunks, EACH prefixed with the title, so a lesson's story no longer
averages itself into its trigger: it occupies its own vectors, and every
one of them still carries the trigger in its prefix. Holding it out would
cost the reader the only part that explains the insight and would buy a
sharpness the chunker already provides.
"""
lines = []
trigger = (when_to_apply or "").strip()
if trigger:
lines.append(f"**When to apply:** {trigger}")
insight = (insight or "").strip()
if insight:
lines.append(insight)
sources = normalize_sources(learned_from)
if sources:
# LAST, not beside the trigger. The first line has to be what this
# lesson is FOR; a provenance line above the insight would push the
# thing the reader came for below a list of ids, and would put
# numbers where the trigger's second appearance does its work.
lines.append("**Learned from:** " + ", ".join(f"#{i}" for i in sources))
return "\n\n".join(lines)
def lesson_document(
what: str, when_to_apply: str = "", insight: str = "",
learned_from: list[int] | None = None,
) -> tuple[str, str]:
"""The (title, body) a lesson is STORED — and therefore embedded — as.
One call so the two halves cannot be composed apart. A lesson whose title
carried the trigger and whose body did not would embed as an ordinary
note wearing a label, and nothing would report it: the record would look
right in every listing and simply never be retrieved at the moment it
applies.
Deliberately returns what is STORED, not a separate embed-time shape.
Rules need `rule_document` because a rule keeps its trigger in a column
and its title is a plain name, so the sharp document has to be synthesised
for the ranker and exists nowhere else. A lesson follows the snippet
instead — the stored record IS the sharp document — which is why nothing
re-embeds and `CHUNKER_VERSION` does not move.
"""
return (
compose_title(what, when_to_apply),
compose_body(insight, when_to_apply, learned_from),
)
def compose_data(
what: str, when_to_apply: str = "", learned_from: list[int] | None = None,
) -> dict:
"""The indexed mirror of the same fields the body renders (0070).
Written together with the body by the one caller that composes both, so the
two can never describe different things — the discipline `compose_data`
follows for snippets, and the reason `lesson_trigger` can prefer `data`
without checking whether it agrees with the prose.
Empty values are omitted so the column stays sparse: a lesson with no
sources has no `taught_by` key rather than an empty list, which keeps a
`?` containment query honest.
"""
data: dict = {"what": what.strip()} if what and what.strip() else {}
trigger = (when_to_apply or "").strip()
if trigger:
data[TRIGGER_KEY] = trigger
sources = normalize_sources(learned_from)
if sources:
data[SOURCES_KEY] = sources
return data
def recompose_data(note) -> dict:
"""Rebuild a lesson's `data` mirror from its own title and body.
For the GENERIC note door. `update_lesson` composes the mirror itself from
the merged field set and never needs this; a plain `update_note(body=...)`
has no idea the mirror exists and would leave it behind.
THE COST OF LEAVING IT BEHIND IS HIGHER HERE THAN FOR A SNIPPET. A stale
snippet mirror reports the wrong path. A stale lesson mirror reports the
wrong TRIGGER — and `lesson_trigger` prefers the mirror, so the lesson goes
on being retrieved for the situation it used to name while displaying the
one it now names. The trigger is the entire retrieval story (step 3), so
that is not a degraded record; it is a record that fires at the wrong
moment and looks right when it does.
The body is the authority and the mirror is derived — already this file's
rule. This is its enforcement on the path that bypasses `update_lesson`.
The subject comes back out of the title through `untrigger_title`, the
inverse of the join that composed it, rather than by splitting on a
separator spelled a second time here.
"""
from scribe.services.embeddings import untrigger_title
body = getattr(note, "body", None) or ""
trigger_match = _BODY_TRIGGER_RE.search(body)
trigger = trigger_match.group(1).strip() if trigger_match else ""
what = untrigger_title(getattr(note, "title", None), trigger)
# Sources through the normal read, which already falls back body →
# arose_from_id. A body edit that drops the provenance line should drop
# the mirror's copy too: the body is the authority, and carrying a value
# the reader just deleted is the failure this function exists to prevent.
sources_match = _BODY_SOURCES_RE.search(body)
sources = (
normalize_sources(_ID_RE.findall(sources_match.group(1)))
if sources_match else []
)
return compose_data(what, trigger, sources)
async def create_lesson(
user_id: int,
*,
what: str,
when_to_apply: str = "",
insight: str = "",
learned_from: list[int] | None = None,
tags: list[str] | None = None,
project_id: int | None = None,
):
"""Create a lesson note. Returns the created Note.
The title, the body and the `data` mirror are composed HERE from named
parameters rather than asked of the caller. That is the whole evidence base
for this design and not a convenience: the snippet corpus carries a trigger
on 164 of 164 records with no guard anywhere, because a service builds the
title from a parameter — what is at 100% is a named structured field, not an
agent typing a convention correctly.
`project_id` is accepted and kept, even though a lesson is reachable from
every project (step 3). Where it was learned is a fact worth keeping; it
simply stops being the limit of where it can be found.
"""
from scribe.services import notes as notes_svc
sources = normalize_sources(learned_from)
title, body = lesson_document(what, when_to_apply, insight, sources)
return await notes_svc.create_note(
user_id,
title=title,
body=body,
note_type=LESSON_NOTE_TYPE,
tags=tags,
project_id=project_id,
# NULL unless there is exactly one source — see `sole_source`.
arose_from_id=sole_source(sources),
data=compose_data(what, when_to_apply, sources),
)
async def get_lesson(user_id: int, lesson_id: int):
"""Fetch a lesson by id, or None if it isn't one / isn't readable.
Share-aware (rule 78): a fetch by id is an explicit act, so it resolves the
caller's full read scope rather than ownership alone — without this, a
lesson a search legitimately surfaced could not then be opened (#2093).
"""
from scribe.services import notes as notes_svc
result = await notes_svc.get_note_for_user(user_id, lesson_id)
if result is None:
return None
note, _permission = result
if note.note_type != LESSON_NOTE_TYPE or note.deleted_at is not None:
return None
return note
def lesson_to_dict(note) -> dict:
"""A lesson as either door returns it — the composed fields read back out,
not the raw row, so a caller sees the same vocabulary it wrote with.
In the SERVICE rather than in each door, on the `snippet_to_dict`
precedent: the REST route feeds the web UI and the MCP tools feed an
agent, and a shape spelled once per door is a shape that answers the two
of them differently the first time a field is added.
"""
return {
"id": note.id,
"title": note.title,
"body": note.body,
# The composed vocabulary, not the storage: a caller that wrote
# `when_to_apply` reads `when_to_apply` back.
"what": (note.data or {}).get("what", "") if isinstance(note.data, dict) else "",
"when_to_apply": lesson_trigger(note),
"learned_from": lesson_sources(note),
"insight": _strip_composed_lines(note.body),
"tags": list(note.tags or []),
"project_id": note.project_id,
"note_type": note.note_type,
"created_at": note.created_at.isoformat() if note.created_at else None,
"updated_at": note.updated_at.isoformat() if note.updated_at else None,
}
async def source_records(user_id: int, ids: list[int]) -> list[dict]:
"""The records that taught a lesson, resolved to something linkable.
`learned_from` is a list of bare ids, which is right for storage and
useless on a page: "#4181" tells a reader nothing about whether it is
worth opening. This resolves each to its title and kind so the UI can
label the link, and so a source that has been deleted simply drops out
rather than rendering a link to nothing.
One query for the whole list, not one per id — a lesson with six sources
would otherwise be six round trips to draw one panel.
Share-aware (rule 78). Order follows `ids`, because that order is the
writer's: the first source is the one they reached for first.
"""
from sqlalchemy import select
from scribe.services.access import readable_notes_clause
wanted = normalize_sources(ids)
if not wanted:
return []
async with async_session() as session:
result = await session.execute(
select(Note)
.where(Note.id.in_(wanted))
.where(Note.deleted_at.is_(None))
.where(readable_notes_clause(user_id))
)
found = {n.id: n for n in result.scalars().all()}
return [
{
"id": n.id,
"title": n.title,
"note_type": n.note_type,
"is_task": n.status is not None,
"task_kind": n.task_kind,
"status": n.status,
}
for i in wanted
if (n := found.get(i)) is not None
]
async def lessons_taught_by(user_id: int, record_id: int, limit: int = 20):
"""The lessons drawn FROM one record — the reverse of `learned_from`.
THE DIRECTION THAT GETS FORGOTTEN, and arguably the more useful one: a
reader opening an old issue wants to know what was learned from it, and
without this the relation is only navigable from the lesson's side. A
record that taught something should say so on its own page.
Queried through `data[SOURCES_KEY]` rather than by scanning bodies: the
mirror is JSONB with a GIN index (0070), which is the whole reason step 4
put the list there. `path_exists` is the same dialect the snippet location
lookup uses, so both reverse lookups read the index the same way.
Share-aware (rule 78) via `readable_notes_clause`: this renders beside a
record the caller can already see, and a lesson someone shared with them
belongs in that list exactly as their own does.
"""
from sqlalchemy import select
from scribe.services.access import readable_notes_clause
try:
wanted = int(record_id)
except (TypeError, ValueError):
return []
if wanted <= 0:
return []
# The id is an int we just validated, never caller text, so it cannot
# break out of the expression — the same guarantee `location_jsonpath`
# gets from JSON-quoting its values.
jsonpath = f"$.{SOURCES_KEY}[*] ? (@ == {wanted})"
async with async_session() as session:
result = await session.execute(
select(Note)
.where(Note.note_type == LESSON_NOTE_TYPE)
.where(Note.deleted_at.is_(None))
.where(Note.data.path_exists(jsonpath))
.where(readable_notes_clause(user_id))
.order_by(Note.updated_at.desc())
.limit(max(1, min(limit, 100)))
)
return list(result.scalars().all())
async def update_lesson(
user_id: int,
lesson_id: int,
*,
what: str | None = None,
when_to_apply: str | None = None,
insight: str | None = None,
learned_from: list[int] | None = None,
tags: list[str] | None = None,
):
"""Update a lesson, re-composing title, body and mirror from the merged
fields. Returns the updated Note, or None if it isn't a readable lesson.
READ-MODIFY-WRITE over the whole record rather than patching one half.
The three fields are not independent: the trigger appears in the title AND
at the head of the body, so editing it in place would need two edits that
a caller could do one of. Re-composing from the merged values means a
partial update cannot leave the halves disagreeing — which, because the
document is what ranks, would be a lesson that still reads correctly and
quietly stops being retrievable.
"""
from scribe.services import notes as notes_svc
note = await get_lesson(user_id, lesson_id)
if note is None:
return None
current = note.data if isinstance(note.data, dict) else {}
merged_what = current.get("what") or "" if what is None else what
merged_trigger = lesson_trigger(note) if when_to_apply is None else when_to_apply
merged_sources = (
lesson_sources(note) if learned_from is None
else normalize_sources(learned_from)
)
if insight is None:
insight = _strip_composed_lines(note.body)
title, body = lesson_document(
merged_what, merged_trigger, insight, merged_sources,
)
fields: dict = {
"title": title,
"body": body,
"arose_from_id": sole_source(merged_sources),
"data": compose_data(merged_what, merged_trigger, merged_sources),
}
if tags is not None:
fields["tags"] = tags
return await notes_svc.update_note(user_id, lesson_id, **fields)
def _strip_composed_lines(body: str | None) -> str:
"""The insight alone — the body with the lines `compose_body` wrote removed.
An update that keeps the insight has to hand it back to `compose_body`,
which will re-add the trigger and provenance lines. Without this the two
composed lines accumulate a copy per edit, and since the trigger line is
half of what makes the document rank, the duplicates would look like the
shape working rather than a bug.
"""
kept = [
line for line in (body or "").splitlines()
if not _BODY_TRIGGER_RE.match(line) and not _BODY_SOURCES_RE.match(line)
]
return "\n".join(kept).strip()