Files
FabledScribe/tests/test_snippet_live_body.py
T
bvandeusenandClaude Fable 5 446d6da0d7
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 11s
CI & Build / TypeScript typecheck (push) Successful in 34s
CI & Build / integration (push) Successful in 29s
CI & Build / Python tests (push) Successful in 1m12s
CI & Build / Build & push image (push) Successful in 25s
fix(snippets): an annotated record is not a diverged one — a standing verdict vouches at its commit (#2782)
Pull-time freshness confirms a cached body by containment: normalised cached
code must appear inside the fetched file. That is right for a record kept
verbatim and permanently wrong for a deliberately annotated one. A record
whose job is to say WHY the shape is what it is carries commentary the source
does not, so containment fails on every pull, forever — #2508 was reading
`diverged` although its declarations match the source exactly, and always
would. Annotation is a sanctioned record style, so this was two deliberate
designs colliding, and it was quietly poisoning the one honest drift signal:
decision #2707's scoreboard watches body_freshness for `diverged` and was
accruing false positives it could never age out.

The escape hatch is the verdict itself. verify_snippet is exactly where a
human or agent already judged this body a faithful rendering of that source,
and `verification.commit_sha` records the repo commit they judged it at — a
field whose own docstring (#2688) anticipated this: "makes 'the REPO moved on
since the check' computable, once the forge integration can compare it against
the current head." This is that comparison. When containment fails, a standing
verdict can still vouch, on four conditions and no fewer:

  - the verdict says `ok`;
  - it has not EXPIRED — verification_view recomputes code_sha against the
    record's current body, so editing the record retires the verdict;
  - it was not INVALIDATED by a push touching the location (#2691);
  - the file just fetched is at the very commit the verdict was stamped at.

That last one is what keeps it honest: the hatch vouches for a body against
ONE known commit, never against whatever the source became since. The moment
the file moves, containment resumes as the authority and the record reads
`diverged` until someone re-verifies — correct, because at that point nobody
has looked. The first three are checked by reusing verification_view rather
than restating its rule, so "expired" keeps meaning one thing.

Nothing is rewritten and no new freshness value is minted; `verification`
already travels in the same payload, so a reader can see the basis rather
than take "current" on faith. Verdicts predating commit stamping carry no
commit to compare and therefore do not vouch — they fall through to
containment rather than passing on age alone.

Tests pin the fix and, more usefully, every condition that switches it back
off: moved commit, expired verdict, push-invalidated verdict, non-ok verdict,
no verdict, and a pre-#2688 verdict with no commit_sha. Plus a regression
that a verbatim record still takes the containment path untouched.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-23 22:36:04 -04:00

361 lines
14 KiB
Python

"""Pull-time freshness (#2690) — attach_live_body against a mocked forge.
The properties that must hold, each with its own way of rotting:
- A no-forge instance's pull response is BYTE-IDENTICAL to today's (rule
#115 — the baseline, not a degraded mode).
- The probe never rewrites the body: "current" refreshes provenance,
"diverged" reports, and neither clobbers the record mid-read.
- A 404 stamps the mechanically-true 'missing' verdict — once, not on
every pull of an already-flagged record.
- The pull is never slower than the budget: a hung forge costs bounded
time and then the cache serves.
"""
import asyncio
import base64
import time
from types import SimpleNamespace
from unittest.mock import AsyncMock, patch
import httpx
from scribe.services import background
from scribe.services import snippets as svc
from scribe.services.forge import GiteaForge
BASE = "https://git.example.com"
CODE = "def helper(x):\n return x + 1\n"
SHA = "f" * 40
def _note():
return SimpleNamespace(id=7, user_id=3, title="", body="", tags=[], data=None)
def _data(*, repo=f"{BASE}/alice/widget", code=CODE, verification=None, provenance=None):
snippet = {
"code": code,
"locations": [{"repo": repo, "path": "src/helper.py", "symbol": "helper"}],
}
if verification:
snippet["verification"] = verification
if provenance:
snippet["provenance"] = provenance
return {"snippet": snippet}
def _forge_with(handler) -> GiteaForge:
return GiteaForge(BASE, "tok", transport=httpx.MockTransport(handler))
def _file_response(content: str, commit_sha: str = SHA) -> httpx.Response:
return httpx.Response(200, json={
"type": "file", "encoding": "base64",
"content": base64.b64encode(content.encode()).decode(),
"last_commit_sha": commit_sha, "path": "src/helper.py",
})
def _patched(forge):
"""Stub the owner-keyring lookup (#2778): None → an empty selector, i.e.
the owner has no connections — the old 'no forge configured' state."""
from scribe.services.forge import ForgeSelector
selector = ForgeSelector(() if forge is None else (forge,))
return patch(
"scribe.services.forge.get_forges", AsyncMock(return_value=selector)
)
async def test_no_forge_attaches_nothing():
data = _data()
before = repr(data)
with _patched(None):
await svc.attach_live_body(_note(), data)
assert repr(data) == before
assert "body_source" not in data
async def test_current_code_confirms_and_refreshes_provenance():
# The file wraps the cached code with extra context and trailing spaces —
# containment is judged after the same normalization the verdict hash uses.
file_content = "import os\n\n" + CODE.replace(" + 1", " + 1 ").rstrip() + "\n\n# eof\n"
forge = _forge_with(lambda r: _file_response(file_content))
saved = {}
async def fake_update(uid, nid, **fields):
saved.update(fields)
data = _data()
with _patched(forge), patch.object(svc.notes_svc, "update_note", fake_update):
await svc.attach_live_body(_note(), data)
await background.drain()
assert data["body_source"] == "forge"
assert data["body_freshness"] == "current"
# Reflected in the response...
assert data["snippet"]["provenance"]["commit_sha"] == SHA
# ...and persisted, without touching the body.
assert saved["data"]["provenance"]["commit_sha"] == SHA
assert "body" not in saved
async def test_current_with_same_stored_sha_skips_the_write():
forge = _forge_with(lambda r: _file_response("prefix\n" + CODE))
update = AsyncMock()
data = _data(provenance={"commit_sha": SHA, "fetched_at": "t"})
with _patched(forge), patch.object(svc.notes_svc, "update_note", update):
await svc.attach_live_body(_note(), data)
await background.drain()
assert data["body_freshness"] == "current"
update.assert_not_called()
async def test_stored_sha_short_circuit_skips_the_content_fetch():
"""#2693: when provenance already names a commit and the forge reports no
newer commit touching the path, the pull is confirmed current WITHOUT a
content transfer — the economy that fits pull-time freshness inside
GitHub's rate limits."""
calls = []
def handler(request):
calls.append(request.url.path)
if request.url.path.endswith("/commits"):
return httpx.Response(200, json=[{"sha": SHA}])
raise AssertionError("the content fetch should have been skipped")
update = AsyncMock()
data = _data(provenance={"commit_sha": SHA, "fetched_at": "t"})
with _patched(_forge_with(handler)), patch.object(svc.notes_svc, "update_note", update):
await svc.attach_live_body(_note(), data)
await background.drain()
assert data["body_source"] == "forge"
assert data["body_freshness"] == "current"
assert len(calls) == 1
update.assert_not_called() # same stamp — nothing to persist
async def test_moved_file_falls_through_to_the_full_fetch():
new_sha = "0" * 40
def handler(request):
if request.url.path.endswith("/commits"):
return httpx.Response(200, json=[{"sha": new_sha}])
return _file_response("prefix\n" + CODE, commit_sha=new_sha)
saved = {}
async def fake_update(uid, nid, **fields):
saved.update(fields)
data = _data(provenance={"commit_sha": SHA, "fetched_at": "t"})
with _patched(_forge_with(handler)), patch.object(
svc.notes_svc, "update_note", fake_update
):
await svc.attach_live_body(_note(), data)
await background.drain()
# The file moved but still contains the code — current, with the stamp
# advanced by the authoritative full fetch.
assert data["body_freshness"] == "current"
assert saved["data"]["provenance"]["commit_sha"] == new_sha
async def test_short_circuit_failure_degrades_to_the_full_fetch():
"""A forge whose commits endpoint errors must cost nothing: the full
fetch stays the authoritative path and the pull behaves as before."""
def handler(request):
if request.url.path.endswith("/commits"):
return httpx.Response(500)
return _file_response("prefix\n" + CODE)
update = AsyncMock()
data = _data(provenance={"commit_sha": SHA, "fetched_at": "t"})
with _patched(_forge_with(handler)), patch.object(svc.notes_svc, "update_note", update):
await svc.attach_live_body(_note(), data)
await background.drain()
assert data["body_freshness"] == "current"
update.assert_not_called() # same sha via the full fetch → same-sha skip
async def test_diverged_reports_without_clobbering():
forge = _forge_with(lambda r: _file_response("def helper(x):\n return x - 1\n"))
update = AsyncMock()
data = _data()
with _patched(forge), patch.object(svc.notes_svc, "update_note", update):
await svc.attach_live_body(_note(), data)
await background.drain()
assert data["body_source"] == "cache"
assert data["body_freshness"] == "diverged"
assert data["snippet"]["code"] == CODE
update.assert_not_called()
async def test_missing_stamps_the_verdict_once():
forge = _forge_with(lambda r: httpx.Response(404, json={}))
data = _data()
with _patched(forge), patch.object(
svc, "record_verification", AsyncMock()
) as verdict:
await svc.attach_live_body(_note(), data)
await background.drain()
assert data["body_freshness"] == "missing"
verdict.assert_awaited_once()
assert verdict.await_args.kwargs["status"] == svc.VERIFY_MISSING
# Already stamped missing → no re-stamp on the next pull.
data2 = _data(verification={"status": svc.VERIFY_MISSING, "code_sha": "x"})
with _patched(forge), patch.object(
svc, "record_verification", AsyncMock()
) as verdict2:
await svc.attach_live_body(_note(), data2)
await background.drain()
assert data2["body_freshness"] == "missing"
verdict2.assert_not_awaited()
async def test_unreachable_falls_back_to_cache():
def handler(request):
raise httpx.ConnectError("down", request=request)
data = _data()
with _patched(_forge_with(handler)):
await svc.attach_live_body(_note(), data)
assert data["body_source"] == "cache"
assert data["body_freshness"] == "unreachable"
async def test_hung_forge_costs_bounded_time(monkeypatch):
async def slow_handler(request):
await asyncio.sleep(30)
return _file_response(CODE)
monkeypatch.setattr(svc, "PULL_FETCH_BUDGET_S", 0.2)
data = _data()
start = time.monotonic()
with _patched(_forge_with(slow_handler)):
await svc.attach_live_body(_note(), data)
assert time.monotonic() - start < 2.0
assert data["body_freshness"] == "unreachable"
async def test_foreign_repo_and_placeless_records_read_as_cache():
forge = GiteaForge(BASE, "tok")
data = _data(repo="https://github.com/alice/widget")
with _patched(forge):
await svc.attach_live_body(_note(), data)
assert data["body_freshness"] == "repo-not-on-this-forge"
placeless = {"snippet": {"code": CODE, "locations": []}}
with _patched(forge):
await svc.attach_live_body(_note(), placeless)
assert placeless["body_freshness"] == "no-recorded-location"
def test_both_pull_surfaces_attach_freshness():
"""Source-inspection guard (the CI convention for wiring assertions): the
MCP pull and the REST detail route both decorate — a surface that forgets
is a surface whose readers silently lose freshness."""
import pathlib
root = pathlib.Path(__file__).resolve().parents[1] / "src" / "scribe"
mcp_src = (root / "mcp" / "tools" / "snippets.py").read_text()
rest_src = (root / "routes" / "snippets.py").read_text()
assert "attach_live_body" in mcp_src
assert "attach_live_body" in rest_src
async def test_forge_failure_inside_lookup_never_breaks_the_pull():
with patch(
"scribe.services.forge.get_forges", AsyncMock(side_effect=RuntimeError("cfg"))
):
data = _data()
await svc.attach_live_body(_note(), data)
assert "body_source" not in data
# --- #2782: an annotated record is not a diverged one ------------------------
# Containment is right for a verbatim record and wrong for a deliberately
# annotated one: the commentary that makes the record worth reading is exactly
# what makes `cached in fetched` false, forever. These pin the escape hatch —
# a standing `ok` verdict stamped at the commit we just fetched — and, just as
# importantly, every condition that must switch it back off.
ANNOTATED = "# Membership is the contract — this record says WHY, the source can't.\n" + CODE
def _ok_verdict(code=ANNOTATED, commit=SHA, **extra):
verdict = svc.compose_verification(
status=svc.VERIFY_OK, checked_code_sha=svc.code_sha(code), commit_sha=commit
)
verdict.update(extra)
return verdict
async def _freshness(data, *, file_commit=SHA, content=CODE):
forge = _forge_with(lambda r: _file_response(content, commit_sha=file_commit))
with _patched(forge), patch.object(svc.notes_svc, "update_note", AsyncMock()):
await svc.attach_live_body(_note(), data)
await background.drain()
return data["body_source"], data["body_freshness"]
async def test_annotated_record_with_a_standing_verdict_reads_current():
"""The bug: the record's commentary is absent from the source, so
containment fails and every pull said `diverged`. A verdict that already
judged this body faithful, at this very commit, outranks the substring."""
data = _data(code=ANNOTATED, verification=_ok_verdict())
assert await _freshness(data) == ("forge", "current")
assert data["snippet"]["code"] == ANNOTATED # still never rewritten
async def test_the_verdict_vouches_for_one_commit_only():
"""The guard that keeps the hatch honest. The file has moved past the
commit the verdict was stamped at, so nobody has judged what is there
now — containment resumes as the authority and the record reads diverged
until someone re-verifies."""
data = _data(code=ANNOTATED, verification=_ok_verdict(commit="a" * 40))
assert await _freshness(data) == ("cache", "diverged")
async def test_an_expired_verdict_does_not_vouch():
"""The record was edited after the check, so `code_sha` no longer matches
and the verdict describes a body that is not this one."""
data = _data(code=ANNOTATED, verification=_ok_verdict(code="def other(): pass"))
assert await _freshness(data) == ("cache", "diverged")
async def test_a_push_invalidated_verdict_does_not_vouch():
"""A push touched the recorded location since the check (#2691) — the repo
moved under the verdict even though the record didn't."""
data = _data(code=ANNOTATED, verification=_ok_verdict(invalidated_by="c" * 40))
assert await _freshness(data) == ("cache", "diverged")
async def test_only_an_ok_verdict_vouches():
"""A drifted verdict is evidence AGAINST the body, not for it."""
verdict = svc.compose_verification(
status=svc.VERIFY_CHANGED, checked_code_sha=svc.code_sha(ANNOTATED), commit_sha=SHA
)
data = _data(code=ANNOTATED, verification=verdict)
assert await _freshness(data) == ("cache", "diverged")
async def test_a_verbatim_record_still_takes_the_containment_path():
"""No regression: the happy path does not route through the hatch, and an
unverified verbatim record is still confirmed by containment alone."""
data = _data(code=CODE)
assert await _freshness(data) == ("forge", "current")
async def test_a_verdict_predating_commit_stamping_does_not_vouch():
"""Verdicts recorded before `commit_sha` existed (#2688) carry no commit to
compare, so they cannot tie the body to a known state of the source. They
fall through to containment rather than vouching on age alone."""
verdict = svc.compose_verification(
status=svc.VERIFY_OK, checked_code_sha=svc.code_sha(ANNOTATED)
)
assert "commit_sha" not in verdict
data = _data(code=ANNOTATED, verification=verdict)
assert await _freshness(data) == ("cache", "diverged")