fix(lessons): strip a lesson's trigger by what was stored, not by matching a line (#4272)
CI & Build / Python lint (push) Successful in 2s
CI & Build / Plugin hooks (push) Successful in 13s
CI & Build / integration (push) Successful in 51s
CI & Build / TypeScript typecheck (push) Successful in 53s
CI & Build / Python tests (push) Failing after 1m3s
CI & Build / Build & push image (push) Skipped
CI & Build / Python lint (push) Successful in 2s
CI & Build / Plugin hooks (push) Successful in 13s
CI & Build / integration (push) Successful in 51s
CI & Build / TypeScript typecheck (push) Successful in 53s
CI & Build / Python tests (push) Failing after 1m3s
CI & Build / Build & push image (push) Skipped
`update_lesson` is field-wise: an edit that touches only `system_ids`, or only the title, passes `insight=None`. The service then derives the insight from the stored body and hands it straight back to `compose_body`, which re-adds the trigger and the provenance line. The derivation and the composition have to be inverses, and they were not. `_strip_composed_lines` filtered out body LINES matching the trigger marker's pattern. That pattern is `re.M`, so it 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 had no reason to touch. They came back as part of "the insight", the whole trigger was prepended again on top of them, and the record gained (n-1) paragraphs per update. Silent, compounding, and in the half of the document that does the retrieving: a lesson edited a few times ends up stating when it applies several times over and burying the insight below it. No marker could fix this. There is no end-of-trigger marker in the body, and adding one would not repair a row already written without it. What makes an exact strip possible is that the trigger is mirrored in `data[TRIGGER_KEY]`, so the composed head can be rebuilt byte for byte and removed by subtraction rather than matched by pattern. Hence `lesson_insight(note)` rather than `_strip_composed_lines(body)` — it takes the record because the body alone does not say where the trigger ends. The line-wise removal survives as the fallback for a row whose mirror is missing, where it is right for a one-paragraph trigger and is the best available without one; a mirror that disagrees with the body falls back rather than cutting into the insight. tests/test_lesson_insight_round_trip.py asserts the property rather than the strip: `lesson_document(what, trigger(n), insight(n), sources(n)) == (n.title, n.body)` across six shapes, and again over ten consecutive no-op edits. Falsified against the old code before committing: 11 of 23 fail on HEAD, the headline one reporting a body grown by 3230 characters over ten edits. The one damaged row in the corpus (#4207, two paragraphs duplicated by a `system_ids` edit) was repaired by hand against its authored text. #4221 is the only other lesson with a multi-paragraph trigger and has never been updated, so its body is intact — its derived `insight` was wrong, its storage was not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy
This commit is contained in:
@@ -436,7 +436,7 @@ def lesson_to_dict(note) -> dict:
|
||||
"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),
|
||||
"insight": lesson_insight(note),
|
||||
"tags": list(note.tags or []),
|
||||
"project_id": note.project_id,
|
||||
"note_type": note.note_type,
|
||||
@@ -569,7 +569,7 @@ async def update_lesson(
|
||||
else normalize_sources(learned_from)
|
||||
)
|
||||
if insight is None:
|
||||
insight = _strip_composed_lines(note.body)
|
||||
insight = lesson_insight(note)
|
||||
|
||||
title, body = lesson_document(
|
||||
merged_what, merged_trigger, insight, merged_sources,
|
||||
@@ -585,17 +585,43 @@ async def update_lesson(
|
||||
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.
|
||||
def lesson_insight(note) -> str:
|
||||
"""The authored insight alone — the body with what `compose_body` added 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.
|
||||
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.
|
||||
"""
|
||||
kept = [
|
||||
line for line in (body or "").splitlines()
|
||||
if not _BODY_TRIGGER_RE.match(line) and not _BODY_SOURCES_RE.match(line)
|
||||
]
|
||||
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()
|
||||
|
||||
Reference in New Issue
Block a user