feat(snippets): body provenance — the cache-with-provenance half of the pointer model (#2688)
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 8s
CI & Build / integration (push) Failing after 21s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Successful in 53s
CI & Build / Build & push image (push) Successful in 28s
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 8s
CI & Build / integration (push) Failing after 21s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Successful in 53s
CI & Build / Build & push image (push) Successful in 28s
Decision #2686: the recorded location is the source of truth for a snippet's code; the stored body is a cache of it. data.provenance now records what the cache is a cache OF — commit_sha + fetched_at — as a carried JSONB field following the verification precedent, so no migration is needed and absence keeps today's exact semantics. The rules: provenance follows the code (fresh SHA restamps it, a code edit without one drops it, a metadata edit carries it); writes ABOUT the code carry it — record_verification rebuilds data from scratch and would otherwise erase it silently; an ok verdict at a known commit restamps it, since the checker just proved the cache matches the source there. verify_snippet verdicts also record the commit they ran at, making "the repo moved on since the check" computable once the forge integration lands. create/update/verify MCP tools take commit_sha (git rev-parse HEAD — free for any session with a checkout). Unit tests pin the compose/carry logic; real-Postgres integration tests run create→verify→update end-to-end (#2663: DB paths get no mocked-only coverage). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -105,6 +105,7 @@ async def create_snippet(
|
||||
project_id: int = 0,
|
||||
system_ids: list[int] | None = None,
|
||||
force: bool = False,
|
||||
commit_sha: str = "",
|
||||
) -> dict:
|
||||
"""Record a shape in the project's pattern library, so every later
|
||||
instance starts from it instead of re-deriving it.
|
||||
@@ -136,6 +137,12 @@ async def create_snippet(
|
||||
proactively within their project; search finds them across projects.
|
||||
system_ids: Ids of the project's Systems to associate this snippet with.
|
||||
force: Bypass the near-duplicate gate (see below).
|
||||
commit_sha: The commit the code was read at (`git rev-parse HEAD` — you
|
||||
have the repo, so it's free). The recorded location is the source
|
||||
of truth for the code and the stored body is a cache of it; this
|
||||
stamps what the cache is a cache OF, so staleness is judgeable
|
||||
later. Optional, but pass it whenever you're recording from a
|
||||
checkout.
|
||||
|
||||
Returns the created snippet (including a parsed `snippet` field), OR — when a
|
||||
duplicate already exists and force is false — {"duplicate": true,
|
||||
@@ -181,6 +188,7 @@ async def create_snippet(
|
||||
uid, name=name, code=code, language=language, signature=signature,
|
||||
when_to_use=when_to_use, repo=repo, path=path, symbol=symbol,
|
||||
locations=locations, tags=tags, project_id=project_id or None,
|
||||
commit_sha=commit_sha,
|
||||
)
|
||||
if system_ids:
|
||||
await systems_svc.set_record_systems(uid, note.id, system_ids)
|
||||
@@ -295,6 +303,7 @@ async def find_duplicate_snippets(threshold: float = 0.0) -> dict:
|
||||
|
||||
async def verify_snippet(
|
||||
snippet_id: int, status: str, detail: str = "", path: str = "",
|
||||
commit_sha: str = "",
|
||||
) -> dict:
|
||||
"""Record whether a snippet's recorded location and code still match source.
|
||||
|
||||
@@ -329,6 +338,10 @@ async def verify_snippet(
|
||||
path: The path you actually checked, if it differs from the recorded
|
||||
one (e.g. you found the symbol at its new home). Defaults to the
|
||||
recorded path.
|
||||
commit_sha: The commit the working tree was at when you checked
|
||||
(`git rev-parse HEAD`). An "ok" verdict with it also refreshes the
|
||||
body's provenance — you just proved the cached code matches the
|
||||
source at that commit.
|
||||
|
||||
Requires write access: a verdict changes how the record is presented, so
|
||||
being able to read a snippet someone shared with you doesn't let you mark
|
||||
@@ -337,6 +350,7 @@ async def verify_snippet(
|
||||
uid = current_user_id()
|
||||
note = await snippets_svc.record_verification(
|
||||
uid, snippet_id, status=status, detail=detail, path=path,
|
||||
commit_sha=commit_sha,
|
||||
)
|
||||
if note is None:
|
||||
raise ValueError(
|
||||
@@ -359,6 +373,7 @@ async def update_snippet(
|
||||
tags: list[str] | None = None,
|
||||
project_id: int = 0,
|
||||
system_ids: list[int] | None = None,
|
||||
commit_sha: str = "",
|
||||
) -> dict:
|
||||
"""Update a snippet. Only the fields you pass change.
|
||||
|
||||
@@ -374,6 +389,10 @@ async def update_snippet(
|
||||
tags: Replaces the extra-tag set (language + "snippet" are re-derived).
|
||||
project_id: 0 leaves it unchanged, -1 detaches it from its project, a
|
||||
positive id moves it.
|
||||
commit_sha: When you're updating the code from a checkout, the commit
|
||||
it was read at (`git rev-parse HEAD`). Restamps the body's
|
||||
provenance; changing the code WITHOUT it drops the old stamp,
|
||||
since the new body no longer comes from that commit.
|
||||
|
||||
Editing someone else's snippet requires an editor or admin share from them.
|
||||
A read-only share is refused with a message saying so — record your own
|
||||
@@ -394,6 +413,7 @@ async def update_snippet(
|
||||
signature=signature, when_to_use=when_to_use,
|
||||
repo=repo, path=path, symbol=symbol,
|
||||
locations=locations, tags=tags, project_id=project,
|
||||
commit_sha=commit_sha or None,
|
||||
)
|
||||
except PermissionError as exc:
|
||||
# Readable but not writable — surface the real reason, not "not found".
|
||||
|
||||
@@ -342,7 +342,7 @@ def parse_snippet_fields(
|
||||
# and copying a blob into the column we index *around* would be pure weight.
|
||||
_DATA_FIELDS = (
|
||||
"name", "when_to_use", "signature", "language", "locations", "merged_from",
|
||||
"verification",
|
||||
"verification", "provenance",
|
||||
)
|
||||
|
||||
# --- drift check (#2086) -----------------------------------------------------
|
||||
@@ -398,6 +398,7 @@ def compose_verification(
|
||||
detail: str = "",
|
||||
path: str = "",
|
||||
checked_at: str = "",
|
||||
commit_sha: str = "",
|
||||
) -> dict:
|
||||
"""Build the `data.verification` record. Unknown statuses are rejected here
|
||||
rather than stored, so the filter never has to cope with a typo'd status."""
|
||||
@@ -414,9 +415,36 @@ def compose_verification(
|
||||
out["detail"] = detail.strip()
|
||||
if (path or "").strip():
|
||||
out["path"] = path.strip()
|
||||
# The repo commit the working tree was at when the check ran (#2688). The
|
||||
# code_sha above expires a verdict when the RECORD is edited; this makes
|
||||
# "the REPO moved on since the check" computable too, once the forge
|
||||
# integration can compare it against the current head.
|
||||
if (commit_sha or "").strip():
|
||||
out["commit_sha"] = commit_sha.strip()
|
||||
return out
|
||||
|
||||
|
||||
def compose_provenance(*, commit_sha: str, fetched_at: str = "") -> dict | None:
|
||||
"""Build the `data.provenance` record: which commit the cached body was
|
||||
read at, and when.
|
||||
|
||||
This is the pointer-model half of decision #2686 — the recorded location
|
||||
is the source of truth for the code and the stored body is a CACHE of it.
|
||||
Provenance says what that cache is a cache OF, so a reader (and later the
|
||||
forge fetch, step 5 of milestone 288) can judge staleness instead of
|
||||
guessing. Absent provenance is valid and means exactly what every snippet
|
||||
meant before this existed: a body captured by hand at an unknown point.
|
||||
"""
|
||||
sha = (commit_sha or "").strip()
|
||||
if not sha:
|
||||
return None
|
||||
return {
|
||||
"commit_sha": sha,
|
||||
"fetched_at": (fetched_at or "").strip()
|
||||
or datetime.now(timezone.utc).isoformat(),
|
||||
}
|
||||
|
||||
|
||||
def verification_view(note, fields: dict) -> dict:
|
||||
"""The verification readout for one snippet, including whether it's expired.
|
||||
|
||||
@@ -434,6 +462,7 @@ def verification_view(note, fields: dict) -> dict:
|
||||
"checked_at": stored.get("checked_at"),
|
||||
"detail": stored.get("detail"),
|
||||
"path": stored.get("path"),
|
||||
"commit_sha": stored.get("commit_sha"),
|
||||
# What the operator actually wants to know: is there something to fix?
|
||||
# An expired verdict counts as "needs looking at" even if it said ok,
|
||||
# since the code it blessed is not the code that's there now.
|
||||
@@ -451,6 +480,7 @@ def compose_data(
|
||||
locations: list[dict] | None = None,
|
||||
merged_from: list[int] | None = None,
|
||||
verification: dict | None = None,
|
||||
provenance: dict | None = None,
|
||||
) -> dict:
|
||||
"""Build the `notes.data` mirror of a snippet's structured fields.
|
||||
|
||||
@@ -479,6 +509,12 @@ def compose_data(
|
||||
# either: the verdict's code_sha expires it on read if the code moved on.
|
||||
if verification:
|
||||
out["verification"] = verification
|
||||
# Also carried: what commit the cached body was read at (#2688). The caller
|
||||
# owns the live-or-die rule — update_snippet drops it when the code changes
|
||||
# without a fresh SHA, because keeping it would claim the new body came
|
||||
# from the old commit.
|
||||
if provenance:
|
||||
out["provenance"] = provenance
|
||||
# The current code's fingerprint — NOT the code, which stays in the body
|
||||
# (see _DATA_FIELDS). Its only job is to make "this verdict has expired"
|
||||
# expressible in SQL: a jsonpath can compare `@.verification.code_sha` to
|
||||
@@ -616,10 +652,15 @@ async def create_snippet(
|
||||
locations: list[dict] | None = None,
|
||||
tags: list[str] | None = None,
|
||||
project_id: int | None = None,
|
||||
commit_sha: str = "",
|
||||
):
|
||||
"""Create a snippet note (embedded on create for immediate recall). Returns
|
||||
the created Note. Pass ``locations`` for the multi-location case; the single
|
||||
``repo``/``path``/``symbol`` are the one-location shorthand."""
|
||||
``repo``/``path``/``symbol`` are the one-location shorthand.
|
||||
|
||||
``commit_sha`` stamps the body's provenance — the commit the recording
|
||||
session read the code at (#2688). Optional: absent means what it always
|
||||
meant, a body captured at an unknown point."""
|
||||
locations = resolve_locations(repo, path, symbol, locations)
|
||||
note = await notes_svc.create_note(
|
||||
user_id,
|
||||
@@ -636,6 +677,7 @@ async def create_snippet(
|
||||
data=compose_data(
|
||||
name=name, when_to_use=when_to_use, signature=signature,
|
||||
language=language, code=code, locations=locations,
|
||||
provenance=compose_provenance(commit_sha=commit_sha),
|
||||
),
|
||||
)
|
||||
return note
|
||||
@@ -714,11 +756,17 @@ async def update_snippet(
|
||||
locations: list[dict] | None = None,
|
||||
tags: list[str] | None = None,
|
||||
project_id: int | None | object = UNSET,
|
||||
commit_sha: str | None = None,
|
||||
):
|
||||
"""Partial update: only fields passed (not None) change. Re-serializes the
|
||||
merged field set back into title/body/tags. Returns the Note, or None if the
|
||||
id isn't a snippet the caller can see.
|
||||
|
||||
``commit_sha`` restamps the body's provenance (#2688). It lives or dies
|
||||
with the code: passed → restamped at that commit; code changed without it →
|
||||
dropped, because keeping it would claim the new body came from the old
|
||||
commit; code untouched → carried.
|
||||
|
||||
Share-aware (rule #47/#78): resolves the read scope, then requires WRITE —
|
||||
so an editor/admin grant lets the holder edit, and a viewer grant does not.
|
||||
Raises PermissionError when the caller can read but not write, because "not
|
||||
@@ -761,6 +809,17 @@ async def update_snippet(
|
||||
else:
|
||||
merged_locations = cur["locations"]
|
||||
|
||||
# Provenance follows the code (#2688): a fresh SHA restamps it; a code
|
||||
# change without one drops it; an edit that leaves the code alone carries
|
||||
# it. Order matters — the explicit SHA wins even when the code changed,
|
||||
# because that is precisely the caller saying where the new body came from.
|
||||
if commit_sha is not None and commit_sha.strip():
|
||||
provenance = compose_provenance(commit_sha=commit_sha)
|
||||
elif code is not None and code != (cur.get("code") or ""):
|
||||
provenance = None
|
||||
else:
|
||||
provenance = cur.get("provenance")
|
||||
|
||||
fields: dict = {
|
||||
"title": compose_title(merged["name"], merged["when_to_use"]),
|
||||
"body": compose_body(
|
||||
@@ -782,6 +841,7 @@ async def update_snippet(
|
||||
# the code, the verdict's code_sha stops matching and it reads as
|
||||
# unverified from here on — no invalidation branch to get wrong.
|
||||
verification=merged.get("verification"),
|
||||
provenance=provenance,
|
||||
),
|
||||
}
|
||||
# Recompute tags: keep any non-language, non-marker tags the note already had
|
||||
@@ -808,6 +868,7 @@ async def record_verification(
|
||||
status: str,
|
||||
detail: str = "",
|
||||
path: str = "",
|
||||
commit_sha: str = "",
|
||||
):
|
||||
"""Record the result of a drift check against the snippet's source.
|
||||
|
||||
@@ -836,6 +897,7 @@ async def record_verification(
|
||||
checked_code_sha=code_sha(fields.get("code") or ""),
|
||||
detail=detail,
|
||||
path=path or fields.get("path") or "",
|
||||
commit_sha=commit_sha,
|
||||
)
|
||||
# Rebuilt from the CURRENT stored fields plus the new verdict, so recording a
|
||||
# check can't quietly rewrite anything else about the record. Note the body
|
||||
@@ -850,6 +912,15 @@ async def record_verification(
|
||||
locations=fields.get("locations") or [],
|
||||
merged_from=fields.get("merged_from") or [],
|
||||
verification=verification,
|
||||
# An "ok" verdict at a known commit IS a provenance claim — the checker
|
||||
# just established that the cached body matches the source there — so
|
||||
# it restamps. Any other verdict carries what was known: a verdict is
|
||||
# about the code, not a change to it, and must not erase it (#2688).
|
||||
provenance=(
|
||||
compose_provenance(commit_sha=commit_sha)
|
||||
if status == VERIFY_OK and (commit_sha or "").strip()
|
||||
else fields.get("provenance")
|
||||
),
|
||||
)
|
||||
return await notes_svc.update_note(note.user_id, snippet_id, data=data)
|
||||
|
||||
|
||||
@@ -0,0 +1,175 @@
|
||||
"""Body provenance — the cache-with-provenance half of the pointer model (#2688).
|
||||
|
||||
Decision #2686: the recorded location is the source of truth for a snippet's
|
||||
code and the stored body is a CACHE of it. `data.provenance` records what that
|
||||
cache is a cache OF — the commit the body was read at, and when — so staleness
|
||||
becomes judgeable instead of guessed, and so the forge fetch (milestone 288
|
||||
step 5) has something to refresh.
|
||||
|
||||
The rules under test, because each has a way to rot silently:
|
||||
|
||||
- Provenance follows the CODE. An edit that changes the code without a fresh
|
||||
SHA must DROP the stamp — carrying it would claim the new body came from
|
||||
the old commit, which is worse than not knowing.
|
||||
- Writes that are ABOUT the code rather than changes TO it (a verification
|
||||
verdict, a metadata edit) must CARRY it — record_verification rebuilds
|
||||
`data` from scratch, so forgetting the field there erases it invisibly.
|
||||
- An "ok" verdict at a known commit RESTAMPS it: the checker just proved the
|
||||
cached body matches the source there.
|
||||
|
||||
Unit tests cover the compose/carry logic; the integration section runs the
|
||||
same rules through the real service paths on real Postgres (the #2663 lesson —
|
||||
a DB-touching path with only mocked coverage is a path with no coverage).
|
||||
"""
|
||||
import pytest
|
||||
import pytest_asyncio
|
||||
|
||||
from scribe.services.snippets import (
|
||||
VERIFY_CHANGED,
|
||||
VERIFY_OK,
|
||||
compose_data,
|
||||
compose_provenance,
|
||||
compose_verification,
|
||||
snippet_fields,
|
||||
verification_view,
|
||||
)
|
||||
|
||||
SHA_A = "a" * 40
|
||||
SHA_B = "b" * 40
|
||||
|
||||
|
||||
# --- unit: composing ---------------------------------------------------------
|
||||
|
||||
def test_compose_provenance_stamps_sha_and_time():
|
||||
prov = compose_provenance(commit_sha=f" {SHA_A} ")
|
||||
assert prov["commit_sha"] == SHA_A
|
||||
assert prov["fetched_at"] # ISO stamp, defaulted
|
||||
|
||||
|
||||
def test_compose_provenance_without_a_sha_is_none_not_an_empty_record():
|
||||
# Absent provenance must stay ABSENT (the pre-#2688 semantics), never an
|
||||
# empty dict that readers would have to distinguish from a real one.
|
||||
assert compose_provenance(commit_sha="") is None
|
||||
assert compose_provenance(commit_sha=" ") is None
|
||||
|
||||
|
||||
def test_compose_data_carries_provenance_only_when_present():
|
||||
with_it = compose_data(name="x", provenance={"commit_sha": SHA_A, "fetched_at": "t"})
|
||||
without = compose_data(name="x", provenance=None)
|
||||
assert with_it["provenance"]["commit_sha"] == SHA_A
|
||||
assert "provenance" not in without
|
||||
|
||||
|
||||
def test_verification_records_and_reads_back_the_checked_commit():
|
||||
verdict = compose_verification(
|
||||
status=VERIFY_OK, checked_code_sha="c" * 32, commit_sha=SHA_A,
|
||||
)
|
||||
assert verdict["commit_sha"] == SHA_A
|
||||
# And an empty one is omitted, not stored as "".
|
||||
bare = compose_verification(status=VERIFY_OK, checked_code_sha="c" * 32)
|
||||
assert "commit_sha" not in bare
|
||||
|
||||
class _N: # minimal note stand-in for the read-time view
|
||||
data = None
|
||||
|
||||
fields = {"code": "", "verification": verdict}
|
||||
view = verification_view(_N(), fields)
|
||||
assert view["commit_sha"] == SHA_A
|
||||
|
||||
|
||||
# --- integration: the rules through the real service paths -------------------
|
||||
|
||||
@pytest_asyncio.fixture
|
||||
async def _dispose_engine():
|
||||
from scribe.models import engine
|
||||
yield
|
||||
await engine.dispose()
|
||||
|
||||
|
||||
@pytest_asyncio.fixture
|
||||
async def user_id(_dispose_engine):
|
||||
from scribe.models import async_session
|
||||
from scribe.models.user import User
|
||||
|
||||
async with async_session() as s:
|
||||
user = User(username="snippet_prov_itest")
|
||||
s.add(user)
|
||||
await s.flush()
|
||||
uid = user.id
|
||||
await s.commit()
|
||||
return uid
|
||||
|
||||
|
||||
async def _fresh(uid, note_id):
|
||||
from scribe.services import snippets as svc
|
||||
note = await svc.get_snippet(uid, note_id)
|
||||
return snippet_fields(note), svc.snippet_to_dict(note)
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
async def test_provenance_lives_and_dies_with_the_code_end_to_end(user_id):
|
||||
from scribe.services import snippets as svc
|
||||
|
||||
note = await svc.create_snippet(
|
||||
user_id, name="prov_helper", code="def prov_helper():\n return 1\n",
|
||||
language="python", repo="Scribe", path="src/x.py", symbol="prov_helper",
|
||||
commit_sha=SHA_A,
|
||||
)
|
||||
fields, view = await _fresh(user_id, note.id)
|
||||
assert fields["provenance"]["commit_sha"] == SHA_A
|
||||
assert view["snippet"]["provenance"]["commit_sha"] == SHA_A
|
||||
|
||||
# A metadata edit leaves the code alone → carried.
|
||||
await svc.update_snippet(user_id, note.id, when_to_use="when proving")
|
||||
fields, _ = await _fresh(user_id, note.id)
|
||||
assert fields["provenance"]["commit_sha"] == SHA_A
|
||||
|
||||
# A code edit with a fresh SHA → restamped.
|
||||
await svc.update_snippet(
|
||||
user_id, note.id, code="def prov_helper():\n return 2\n",
|
||||
commit_sha=SHA_B,
|
||||
)
|
||||
fields, _ = await _fresh(user_id, note.id)
|
||||
assert fields["provenance"]["commit_sha"] == SHA_B
|
||||
|
||||
# A code edit WITHOUT one → dropped, not carried: the new body does not
|
||||
# come from SHA_B and the record must not claim it does.
|
||||
await svc.update_snippet(
|
||||
user_id, note.id, code="def prov_helper():\n return 3\n",
|
||||
)
|
||||
fields, view = await _fresh(user_id, note.id)
|
||||
assert "provenance" not in fields
|
||||
assert "provenance" not in view["snippet"]
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
async def test_verification_stamps_the_commit_and_ok_refreshes_provenance(user_id):
|
||||
from scribe.services import snippets as svc
|
||||
|
||||
note = await svc.create_snippet(
|
||||
user_id, name="prov_verify", code="def prov_verify():\n return 1\n",
|
||||
language="python", repo="Scribe", path="src/y.py", symbol="prov_verify",
|
||||
commit_sha=SHA_A,
|
||||
)
|
||||
|
||||
# A non-ok verdict at a newer commit records where the check ran but must
|
||||
# CARRY provenance — the check didn't change what the cached body is.
|
||||
await svc.record_verification(
|
||||
user_id, note.id, status=VERIFY_CHANGED, detail="diverged", commit_sha=SHA_B,
|
||||
)
|
||||
fields, view = await _fresh(user_id, note.id)
|
||||
assert view["verification"]["commit_sha"] == SHA_B
|
||||
assert fields["provenance"]["commit_sha"] == SHA_A
|
||||
|
||||
# An OK verdict at that commit proves the cache matches the source there —
|
||||
# provenance refreshes without an edit.
|
||||
await svc.record_verification(
|
||||
user_id, note.id, status=VERIFY_OK, detail="matches", commit_sha=SHA_B,
|
||||
)
|
||||
fields, view = await _fresh(user_id, note.id)
|
||||
assert view["verification"]["commit_sha"] == SHA_B
|
||||
assert fields["provenance"]["commit_sha"] == SHA_B
|
||||
|
||||
# And the verdict itself survives untouched by the restamp.
|
||||
assert view["verification"]["status"] == VERIFY_OK
|
||||
assert view["verification"]["current"] is True
|
||||
Reference in New Issue
Block a user