CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 11s
CI & Build / integration (push) Successful in 45s
CI & Build / TypeScript typecheck (push) Successful in 53s
CI & Build / Python tests (push) Failing after 1m5s
CI & Build / Build & push image (push) Skipped
The module docstring said the defect grew the worst real body by 520 characters over ten edits. That number was recalled, not measured, and it is wrong — as was the 2500 I then got by deriving it on paper instead of running it. Replayed lesson #4207's actual stored shape through the old code: 607 characters per edit, exactly the length of the trigger's paragraphs 2 and 3 with their separators, taking the body from 1291 to 7361 over ten edits. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy
223 lines
11 KiB
Python
223 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_note
|
|
|
|
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."""
|
|
title, body = lessons_svc.lesson_document(WHAT, trigger, insight, sources)
|
|
data = lessons_svc.compose_data(WHAT, trigger, sources)
|
|
return fake_note(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_note(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_note(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_note(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_note(
|
|
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)
|