Files
FabledScribe/tests/test_lesson_insight_round_trip.py
T
bvandeusenandClaude Opus 5 b47dc97ca8
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 12s
CI & Build / integration (push) Successful in 45s
CI & Build / TypeScript typecheck (push) Successful in 52s
CI & Build / Python tests (push) Successful in 1m34s
CI & Build / Build & push image (push) Successful in 21s
fix(tests): the round-trip guard needs fake_lesson, not fake_note (#4272)
Run 7208 failed 5 of 23 in the new file, all with a `**Learned from:** #1`
that no shape asked for.

`lesson_sources` falls back to `arose_from_id` when neither the mirror nor
the body names a source. `fake_note` leaves that field to MagicMock, which
auto-creates it truthy and coerces to 1 — so every no-sources shape
recomposed with a provenance line and the round trip failed for a reason
with nothing to do with the trigger. `fake_lesson` pins `arose_from_id` to
None and its docstring says why; it is also the truthful stand-in, since
these are lessons.

Worth recording separately: the local harness that cleared this change used
a plain stub class, so it was kinder than the real fixture and could not
reproduce the truthiness. The stub has been given the same None. A
stand-in that is more forgiving than the one the suite uses will verify a
change that CI then rejects — note #2109's point, arriving from the other
direction.

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

231 lines
11 KiB
Python

"""Reading a lesson's insight back and re-composing it returns the same body (#4272).
THE PROPERTY. `update_lesson` is field-wise: an edit that touches only
`system_ids`, or only `what`, passes `insight=None`, and the service then
derives the insight from the stored body and hands it back to
`compose_body`, which re-adds the trigger and the provenance line. So the
derivation and the composition are inverses, and if they are not, EVERY edit
that does not restate the insight rewrites the record slightly wrong.
lesson_document(what, lesson_trigger(n), lesson_insight(n), lesson_sources(n))
== (n.title, n.body)
That is the whole of what these tests assert, and it is the shape of guard
worth having wherever a record is stored composed and edited in parts: not
"does the strip remove the marker" — it did — but "does an edit that changes
nothing change nothing".
WHAT WENT WRONG. The derivation used to filter out body LINES matching the
trigger marker's pattern, which is `re.M` and therefore matches one line.
`when_to_apply` is free text and is routinely several paragraphs; the
composer writes all of it after the marker, so paragraphs 2..n were ordinary
lines that the filter had no reason to touch. They came back as "the
insight", the full trigger was prepended again, and the stored lesson gained
(n-1) paragraphs per update — silently, compounding, and in the half of the
document that does the retrieving.
Measured on the one real row this damaged (lesson #4207, a three-paragraph
trigger): the body grew by exactly 607 characters per edit — the length of
paragraphs 2 and 3 with their separators — taking it from 1291 characters to
7361 over ten edits, at which point the trigger appears eleven times and the
insight is 17% of the record.
WHY A MARKER COULD NOT HAVE FIXED IT. 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 reconstructed byte for byte
and removed by subtraction rather than matched by pattern. The line-wise
removal survives as the fallback for a row with no mirror, where it is right
for a one-paragraph trigger and is the best available without one — see
`test_a_stale_mirror_falls_back_rather_than_cutting_the_wrong_thing`.
KNOWN AND UNCHANGED: a line inside the insight that itself begins
`**Learned from:**` is absorbed into the provenance line, as it always was.
That is a different defect from this one and is not addressed here.
"""
from __future__ import annotations
import pytest
from scribe.services import lessons as lessons_svc
from tests.helpers import fake_lesson
WHAT = "A column written once at create time is a snapshot, not a field"
ONE_LINE = "whenever a stored value is written only on the insert path"
# The shape that broke, reduced. Three paragraphs, ordinary prose, nothing
# unusual about it — which is the point: this is what a real trigger looks
# like once someone writes a good one.
MANY_PARAGRAPHS = (
"Whenever a stored field is written only on the insert path — the tell is "
"a write guarded by `if isNew` with no setter anywhere.\n\n"
"Also whenever the value is captured BEFORE the thing it describes is "
"fully known: an async metadata fetch, a webhook that fills in details "
"later, a placeholder from a search result.\n\n"
"Finally: whenever a sync reports a steady bucket of records it \"can't "
"find\", and the records turn out to be healthy when you look at them."
)
INSIGHT = (
"A write-once column is a snapshot of a moment. A field is a current "
"fact.\n\n"
"## The lesson\n\n"
"Decide which you meant, and if it is a field, give it a writer that runs "
"when the fact changes."
)
# (name, trigger, insight, sources)
SHAPES = [
("one-line trigger", ONE_LINE, INSIGHT, [4219, 4211]),
("multi-paragraph trigger", MANY_PARAGRAPHS, INSIGHT, [4219, 4211, 4199]),
("multi-paragraph trigger, no sources", MANY_PARAGRAPHS, INSIGHT, []),
("no trigger at all", "", INSIGHT, [4219]),
("single-line insight", ONE_LINE, "Check the inputs agree.", [4199]),
# The boundary case: the insight OPENS with the same words the trigger
# CLOSES with. An exact prefix removal is right here and a "cut up to the
# last matching paragraph" heuristic is not.
("insight echoes the trigger's tail", "alpha\n\nbeta", "beta, and more besides.", []),
]
IDS = [s[0] for s in SHAPES]
def _stored(trigger: str, insight: str, sources: list[int]):
"""A lesson exactly as `create_lesson` writes it — title, body and mirror
composed together, which is the only way a real row is ever written.
`fake_lesson` rather than `fake_note`, and the difference is load-bearing
here: `lesson_sources` falls back to `arose_from_id`, which on a bare
MagicMock auto-creates truthy and coerces to `1`. A no-sources shape then
recomposes with `**Learned from:** #1` and the round trip fails for a
reason that has nothing to do with the trigger. `fake_lesson` pins that
field to None — note #2109's point, which is why the fixture exists.
"""
title, body = lessons_svc.lesson_document(WHAT, trigger, insight, sources)
data = lessons_svc.compose_data(WHAT, trigger, sources)
return fake_lesson(title=title, body=body, data=data)
def _edit(note):
"""One no-op edit, along `update_lesson`'s own path when `insight is None`
and no other field is given: every part is read back from the row and
re-composed."""
return lessons_svc.lesson_document(
(note.data or {}).get("what", WHAT),
lessons_svc.lesson_trigger(note),
lessons_svc.lesson_insight(note),
lessons_svc.lesson_sources(note),
)
@pytest.mark.parametrize("_name,trigger,insight,sources", SHAPES, ids=IDS)
def test_the_derived_insight_is_the_authored_insight(_name, trigger, insight, sources):
"""The direct claim, before any round trip: what comes back out is what
went in. Asserted separately from the round trip because a derivation can
be self-consistently wrong — strip too much and re-composing too little
still balances."""
note = _stored(trigger, insight, sources)
assert lessons_svc.lesson_insight(note) == insight.strip()
@pytest.mark.parametrize("_name,trigger,insight,sources", SHAPES, ids=IDS)
def test_an_edit_that_changes_nothing_changes_nothing(_name, trigger, insight, sources):
"""THE guard. Not about markers or patterns — about whether the pair of
functions that every partial edit runs through is actually an identity."""
note = _stored(trigger, insight, sources)
title, body = _edit(note)
# Reported as a size delta rather than left to a diff of two multi-kilobyte
# strings: the failure this catches is duplication, and the number of
# characters gained says which paragraph came back far faster than an
# equality dump does (rule 167 — a guard has to name what broke).
assert (title, body) == (note.title, note.body), (
f"a no-op edit rewrote the lesson: title {len(title) - len(note.title):+d} "
f"chars, body {len(body) - len(note.body):+d} chars"
)
@pytest.mark.parametrize("_name,trigger,insight,sources", SHAPES, ids=IDS)
def test_ten_edits_do_not_grow_the_record(_name, trigger, insight, sources):
"""Because the damage compounds rather than appearing once.
A single round trip off by one paragraph looks like a formatting wobble
in a diff. Ten of them is a lesson whose trigger appears eleven times in
its own body, which is both unreadable and — since the trigger is what
ranks — a document that has drowned out its own insight. Tagging a lesson
to a System is one such edit; so is fixing a typo in its title."""
note = _stored(trigger, insight, sources)
first = note.body
for _ in range(10):
title, body = _edit(note)
note = fake_lesson(title=title, body=body, data=note.data)
assert note.body == first, (
f"body grew by {len(note.body) - len(first)} characters over ten no-op edits"
)
def test_no_paragraph_of_the_trigger_survives_into_the_insight():
"""Stated on the paragraphs themselves, so the failure names the defect.
An equality assertion reports "these two long strings differ"; this one
reports which paragraph of the trigger leaked, which is the sentence
somebody needs in order to understand the bug at all."""
note = _stored(MANY_PARAGRAPHS, INSIGHT, [4219])
derived = lessons_svc.lesson_insight(note)
for n, para in enumerate(MANY_PARAGRAPHS.split("\n\n"), start=1):
assert para not in derived, (
f"paragraph {n} of the trigger came back as part of the insight; "
f"the next update will prepend the whole trigger again on top of it"
)
def test_the_insight_is_derived_from_the_row_not_from_the_body_alone():
"""Why the function takes a note.
The body on its own does not say where the trigger ends — that is the
information the old shape did not have and guessed at. The mirror in
`data` is where it comes from, so a caller holding only the body string
cannot be served, and this pins that the argument is the record."""
note = _stored(MANY_PARAGRAPHS, INSIGHT, [])
assert lessons_svc.lesson_insight(note) == INSIGHT.strip()
# The same body with the mirror removed cannot reach the same answer.
bare = fake_lesson(title=note.title, body=note.body, data=None)
assert lessons_svc.lesson_insight(bare) != INSIGHT.strip()
def test_without_a_mirror_a_one_paragraph_trigger_still_strips():
"""The fallback doing its job. A row written before the mirror existed,
or one whose `data` was dropped by a generic update, still reads back
correctly for the common shape — degrading to the old behaviour rather
than to nothing."""
_, body = lessons_svc.lesson_document(WHAT, ONE_LINE, INSIGHT, [4199])
bare = fake_lesson(title="t", body=body, data=None)
assert lessons_svc.lesson_insight(bare) == INSIGHT.strip()
def test_a_stale_mirror_falls_back_rather_than_cutting_the_wrong_thing():
"""If the mirror and the body disagree, the body wins by default.
`recompose_data` exists because a generic `update_note(body=...)` can
leave the mirror describing a trigger the body no longer has. Removing a
stored prefix that is not actually at the head of the body would cut into
the insight, so the removal happens only on an exact match and the
line-wise fallback takes over otherwise. Nothing authored is lost."""
_, body = lessons_svc.lesson_document(WHAT, ONE_LINE, INSIGHT, [])
stale = fake_lesson(
title="t", body=body,
data={"what": WHAT, lessons_svc.TRIGGER_KEY: "a trigger that is not in this body"},
)
derived = lessons_svc.lesson_insight(stale)
assert derived == INSIGHT.strip()
assert "a trigger that is not in this body" not in derived
def test_a_lesson_with_neither_trigger_nor_sources_round_trips():
"""The degenerate row — body is the insight and nothing else. Pinned
because the removal builds its prefix from the trigger, and an empty
trigger must mean "remove nothing", not "remove the marker text"."""
note = _stored("", INSIGHT, [])
assert note.body == INSIGHT.strip()
assert lessons_svc.lesson_insight(note) == INSIGHT.strip()
assert _edit(note) == (note.title, note.body)