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
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>
612 lines
27 KiB
Python
612 lines
27 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 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``).
|
|
|
|
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.
|
|
|
|
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.
|
|
|
|
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_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 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.
|
|
|
|
`**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 as.
|
|
|
|
One call so the two halves cannot be composed apart. The body's first line
|
|
carries the trigger; a lesson without it (and without the `data` mirror)
|
|
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.
|
|
|
|
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.
|
|
"""
|
|
return (
|
|
(what or "").strip(),
|
|
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": lesson_insight(note),
|
|
"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 = lesson_insight(note)
|
|
|
|
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 lesson_insight(note) -> str:
|
|
"""The authored insight alone — the body with what `compose_body` added removed.
|
|
|
|
An update that keeps the insight hands it back to `compose_body`, which
|
|
re-adds the trigger and provenance. So this has to remove EXACTLY what was
|
|
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
|
|
compounding.
|
|
|
|
What makes an exact strip possible is that the trigger is stored: the
|
|
mirror in `data` holds its full text, so `composed` below is
|
|
byte-for-byte what `compose_body` put at the head, and removing it is
|
|
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.
|
|
|
|
The provenance line needs no such care: `compose_body` builds it by
|
|
joining ids with ", ", so it cannot contain a newline.
|
|
"""
|
|
body = getattr(note, "body", None) or ""
|
|
trigger = lesson_trigger(note).strip()
|
|
head = body.lstrip("\n")
|
|
composed = f"**When to apply:** {trigger}" if trigger else ""
|
|
if composed and head.startswith(composed):
|
|
lines = head[len(composed):].splitlines()
|
|
else:
|
|
lines = [ln for ln in body.splitlines() if not _BODY_TRIGGER_RE.match(ln)]
|
|
kept = [ln for ln in lines if not _BODY_SOURCES_RE.match(ln)]
|
|
return "\n".join(kept).strip()
|