bvandeusen/FabledScribe
1
0
Fork 0
Code Issues 1 Pull Requests 1 Actions Packages Projects Releases 53 Wiki Activity

docs(6205): lessons comments state the reason, not the instance that taught it

Browse Source 
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 <noreply@anthropic.com>
This commit is contained in:
bvandeusen
2026-10-21 20:21:57 -04:00
co-authored by Claude Opus 5.5
parent 7e9d33d9d9
commit d1b442c572
1 changed files with 71 additions and 84 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/scribe/services/lessons.py
+71 -84
View File
@@ -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.