fix(snippets): an annotated record is not a diverged one — a standing verdict vouches at its commit (#2782)
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
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
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>
This commit is contained in:
@@ -207,6 +207,13 @@ async def get_snippet(snippet_id: int) -> dict:
|
|||||||
the source moved on — trust the location over the cached body and
|
the source moved on — trust the location over the cached body and
|
||||||
consider verify_snippet after you look.
|
consider verify_snippet after you look.
|
||||||
|
|
||||||
|
A record kept VERBATIM is confirmed by containment. A deliberately
|
||||||
|
ANNOTATED one — commentary the source does not carry — cannot be, so it
|
||||||
|
reads "current" on the authority of a standing `ok` verdict stamped at
|
||||||
|
the very commit just fetched (#2782); `verification` in the same payload
|
||||||
|
shows that basis. Edit the record, or let the file move past that commit,
|
||||||
|
and it reads "diverged" again until someone re-runs verify_snippet.
|
||||||
|
|
||||||
When the shape ledger has judgments against this snippet, the response
|
When the shape ledger has judgments against this snippet, the response
|
||||||
carries `instances` (shapes classified as conforming to it — the
|
carries `instances` (shapes classified as conforming to it — the
|
||||||
structured consumer map) and/or `variants` (named departures, each with
|
structured consumer map) and/or `variants` (named departures, each with
|
||||||
|
|||||||
@@ -987,6 +987,46 @@ async def _refresh_provenance(note, commit_sha: str) -> None:
|
|||||||
await notes_svc.update_note(note.user_id, note.id, data=data)
|
await notes_svc.update_note(note.user_id, note.id, data=data)
|
||||||
|
|
||||||
|
|
||||||
|
def _verdict_still_vouches(note, fields: dict, fetched_commit_sha: str) -> bool:
|
||||||
|
"""Does a standing `ok` verdict still speak for this body, at this commit?
|
||||||
|
|
||||||
|
Containment (cached code ∈ fetched file) is the fast path, and it is right
|
||||||
|
for a record kept verbatim. It is WRONG for a deliberately annotated one
|
||||||
|
(#2782): a record whose job is to say why the shape is what it is carries
|
||||||
|
commentary the source does not, so containment fails forever and the record
|
||||||
|
reads `diverged` on every pull. That turns the one honest drift signal into
|
||||||
|
a permanent false positive — and annotation is a sanctioned record style,
|
||||||
|
so this is two deliberate designs colliding, not a malformed record.
|
||||||
|
|
||||||
|
The escape hatch is the verdict itself. `verify_snippet` is precisely 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 use: "makes
|
||||||
|
'the REPO moved on since the check' computable, once the forge integration
|
||||||
|
can compare it against the current head." This is that comparison.
|
||||||
|
|
||||||
|
All four conditions, and none is optional:
|
||||||
|
- 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 we just fetched is at the very commit the verdict was stamped
|
||||||
|
at. Any later commit means nobody has judged what is there now.
|
||||||
|
|
||||||
|
The last one is what keeps this honest: it vouches for a body against ONE
|
||||||
|
known commit, never against whatever the source has become since. The
|
||||||
|
moment the file moves, containment resumes as the authority and the record
|
||||||
|
reads `diverged` until someone re-verifies — which is the correct outcome,
|
||||||
|
because at that point nobody has looked.
|
||||||
|
"""
|
||||||
|
if not fetched_commit_sha:
|
||||||
|
return False
|
||||||
|
view = verification_view(note, fields)
|
||||||
|
if view.get("status") != VERIFY_OK or view.get("needs_attention"):
|
||||||
|
return False
|
||||||
|
return view.get("commit_sha") == fetched_commit_sha
|
||||||
|
|
||||||
|
|
||||||
async def attach_live_body(note, data: dict) -> None:
|
async def attach_live_body(note, data: dict) -> None:
|
||||||
"""Decorate a PULL response with forge-checked freshness (#2690).
|
"""Decorate a PULL response with forge-checked freshness (#2690).
|
||||||
|
|
||||||
@@ -1115,6 +1155,14 @@ async def attach_live_body(note, data: dict) -> None:
|
|||||||
_refresh_provenance(note, fetched.commit_sha),
|
_refresh_provenance(note, fetched.commit_sha),
|
||||||
site="pull provenance-refresh",
|
site="pull provenance-refresh",
|
||||||
)
|
)
|
||||||
|
elif _verdict_still_vouches(note, fields, fetched.commit_sha or ""):
|
||||||
|
# Containment failed, but an unexpired `ok` verdict stamped at exactly
|
||||||
|
# this commit already judged this body a faithful rendering of it —
|
||||||
|
# the annotated-record case (#2782). Trust the judgment over the
|
||||||
|
# substring test; `data["verification"]` travels in the same payload,
|
||||||
|
# so a reader can see the basis rather than take "current" on faith.
|
||||||
|
data["body_source"] = "forge"
|
||||||
|
data["body_freshness"] = "current"
|
||||||
else:
|
else:
|
||||||
data["body_source"] = "cache"
|
data["body_source"] = "cache"
|
||||||
data["body_freshness"] = "diverged"
|
data["body_freshness"] = "diverged"
|
||||||
|
|||||||
@@ -272,3 +272,89 @@ async def test_forge_failure_inside_lookup_never_breaks_the_pull():
|
|||||||
data = _data()
|
data = _data()
|
||||||
await svc.attach_live_body(_note(), data)
|
await svc.attach_live_body(_note(), data)
|
||||||
assert "body_source" not in 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")
|
||||||
|
|||||||
Reference in New Issue
Block a user