From d1b442c5722959d31d01270714b604f39f92c2ba Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 21 Oct 2026 20:21:57 -0400 Subject: [PATCH] docs(6205): lessons comments state the reason, not the instance that taught it The module docstring keeps why the kind exists, where its trigger lives, what it inherits and why it needs no migration, without milestone, decision and note ids or this instance's snippet counts. The body-strip docstring states why taking the note matters instead of retelling the defect. Co-Authored-By: Claude Opus 5.5 --- src/scribe/services/lessons.py | 155 +++++++++++++++------------------ 1 file changed, 71 insertions(+), 84 deletions(-) diff --git a/src/scribe/services/lessons.py b/src/scribe/services/lessons.py index a2bda83f..88c15dca 100644 --- a/src/scribe/services/lessons.py +++ b/src/scribe/services/lessons.py @@ -4,12 +4,11 @@ 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) +WHY THE KIND EXISTS -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. +Without it, rules are the only surface that is global AND situation-keyed, so +an agent holding a transferable insight has one door, and that door binds — +and sessions propose rules for things that should not be rules. The gap is a document-shape fact, not a threshold: @@ -23,31 +22,26 @@ 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) +WHERE THE TRIGGER LIVES In ``notes.data`` under ``when_to_apply``, written through a named parameter and -mirrored into the head of the body — the shape snippets already use for -``when_to_use``. Not a column on ``notes``. Since milestone 427 it is NOT in the -stored title: the title is the lesson's subject, and the trigger joins it only -in the embedded document (``embeddings.document_title``). +mirrored into the head of the body — the shape snippets use for +``when_to_use``. Not a column on ``notes``. It is NOT in the stored title: the +title is the lesson's subject, and the trigger joins it only in the embedded +document (``embeddings.document_title``). -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: the service composed 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. +A named structured field rather than a column: snippets show such a field gets +filled reliably without a guard, and a column would decide enforceability for +every note kind at once. The trigger in the document is what makes the vector sharp. It reaches it twice — in the embedded title, joined at embed time, and in the body's first -line — which is the text these kinds were always embedded as, so nothing -re-embeds and ``CHUNKER_VERSION`` does not move. +line. -WHAT A LESSON INHERITS, AND THE CELLS LEFT EMPTY ON PURPOSE (#3163) +WHAT A LESSON INHERITS, AND THE CELLS LEFT EMPTY ON PURPOSE -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. +A new kind inherits the note machinery wholesale. Which parts it should NOT get +is stated here, so that an empty cell is a decision rather than an oversight. Inherited, all deliberately: @@ -67,25 +61,23 @@ NOT inherited, and each for a stated reason: 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 + fields, but not part of a lesson's contract and not asked for on create. + 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 +WHY A KIND NEEDS 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. +(``notes_task_kind_check``). It is plain ``Text`` with a server default, so a +value the database refuses on an un-migrated instance 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. +single edit. """ from __future__ import annotations @@ -142,18 +134,18 @@ def normalize_sources(entries: list | None) -> list[int]: 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 + answer and the wrong one. A good lesson often generalises several + incidents — a badge collision, an un-backfilled column, a duplicated const + — into one claim about a failure class. That 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 + It lives in `notes.data` rather than a join table for the reason the + trigger lives there: a join table would settle, for every note kind at + once, whether provenance is multi-valued. `data` is JSONB with a GIN + index, 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 @@ -217,10 +209,10 @@ def compose_body( The trigger's home in the text of the document, and half of what makes a lesson findable: `chunk_document` joins `{embedded title}\\n{body}`, and - the embedded title is `what — when it applies` (`embeddings.document_title`, - milestone 427), so the document 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. + the embedded title is `what — when it applies` (`embeddings.document_title`), + so the document states WHEN IT APPLIES in the title and again in the first + line of the body. That twice-in-a-short-document shape is what measures + sharp in the corpus. `**When to apply:**` rather than plain text: the body is the READABLE form, `data` is the queryable mirror, and `_BODY_TRIGGER_RE` reads this @@ -229,11 +221,11 @@ def compose_body( 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 + `rule_document` excludes a rule's `why` because long narrative in ONE + vector lands on the centroid of "development" — but a body over the + budget is split into several chunks, EACH prefixed with the title, so a + lesson's story does not average 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. @@ -255,11 +247,10 @@ def compose_body( return "\n\n".join(lines) -# A lesson's NAME is its claim, and the claim is one line (#4797). `what` is -# the title every listing, menu and search row prints, so a story written there -# instead of in `insight` turns each of those lines into a document — the menu -# line for one such lesson ran to 1,416 characters, and the claim it existed to -# transfer sat somewhere in the middle. It also ranks badly: the embedded title +# A lesson's NAME is its claim, and the claim is one line. `what` is the title +# every listing, menu and search row prints, so a story written there instead +# of in `insight` turns each of those lines into a document, with the claim it +# exists to transfer somewhere in the middle. It also ranks badly: the embedded title # is `what — when_to_apply`, so a narrative crowds out the trigger. # # 240 is a long sentence. The bound is on what a NAME can be, not on what a @@ -320,12 +311,10 @@ def lesson_document( it: the record would look right in every listing and simply never be retrieved at the moment it applies. - The title is the SUBJECT alone (milestone 427). It used to carry the - trigger too, so the stored record was itself the sharp document — and every - listing, menu and search row then showed a title that ran to kilobytes. - The trigger now joins the title at embed time (`embeddings.document_title`, - reading `data`), producing the same text as before, so nothing re-embeds - and `CHUNKER_VERSION` does not move. + The title is the SUBJECT alone. A stored title carrying the trigger would + make every listing, menu and search row show a title that runs to + kilobytes. The trigger joins the title at embed time + (`embeddings.document_title`, reading `data`). """ return ( (what or "").strip(), @@ -368,7 +357,7 @@ def recompose_data(note) -> dict: 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 + one it now names. The trigger is the entire retrieval story, so that is not a degraded record; it is a record that fires at the wrong moment and looks right when it does. @@ -410,14 +399,12 @@ async def create_lesson( """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. + parameters rather than asked of the caller. That is the design, not a + convenience: a named structured field that a service composes is filled + reliably, where a convention an agent must type correctly is not. `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 + every project. 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 @@ -440,9 +427,10 @@ async def create_lesson( 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). + Share-aware (services/access.py): 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. """ from scribe.services import notes as notes_svc @@ -494,7 +482,7 @@ async def source_records(user_id: int, ids: list[int]) -> list[dict]: 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 + Share-aware (services/access.py). Order follows `ids`, because that order is the writer's: the first source is the one they reached for first. """ from sqlalchemy import select @@ -535,11 +523,11 @@ async def lessons_taught_by(user_id: int, record_id: int, limit: int = 20): 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 + mirror is JSONB with a GIN index, which is the whole reason the list + lives 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 + Share-aware 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. """ @@ -630,13 +618,12 @@ def lesson_insight(note) -> str: added; anything it leaves behind is re-prepended and the body grows by that much on every edit. - TAKES THE NOTE, NOT THE BODY, and that is the whole fix (#4272). The - previous shape filtered out lines matching `_BODY_TRIGGER_RE`, which is - `re.M` and therefore matches ONE LINE. `when_to_apply` is free text and is - routinely several paragraphs — `compose_body` writes all of it after the - marker, so paragraphs 2..n were ordinary lines the filter never touched. - They came back as part of "the insight", the full trigger was prepended - again, and the record gained (n-1) paragraphs per update. Silent, and + TAKES THE NOTE, NOT THE BODY. Filtering out lines matching + `_BODY_TRIGGER_RE` is not enough: that pattern is `re.M` and matches ONE + LINE, while `when_to_apply` is free text and routinely several paragraphs + — `compose_body` writes all of it after the marker. Paragraphs 2..n would + come back as part of "the insight", the full trigger would be prepended + again, and the record would gain (n-1) paragraphs per update. Silent, and compounding. What makes an exact strip possible is that the trigger is stored: the @@ -645,9 +632,9 @@ def lesson_insight(note) -> str: subtraction rather than pattern-matching. The fallback is not dead code. A row whose mirror is missing degrades to - the old line-wise removal, which is right for a one-paragraph trigger and - is the best that can be done without knowing where the trigger ends — - there is no marker for its last line, which is why guessing was wrong. + line-wise removal, which is right for a one-paragraph trigger and is the + best that can be done without knowing where the trigger ends — there is + no marker for its last line. The provenance line needs no such care: `compose_body` builds it by joining ids with ", ", so it cannot contain a newline.