From 1e7f66e72d23efdfd770f85334e68f8e6845ce7b Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 16 Aug 2026 12:00:40 -0400 Subject: [PATCH] =?UTF-8?q?feat(snippets):=20body=20provenance=20=E2=80=94?= =?UTF-8?q?=20the=20cache-with-provenance=20half=20of=20the=20pointer=20mo?= =?UTF-8?q?del=20(#2688)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/scribe/mcp/tools/snippets.py | 20 ++++ src/scribe/services/snippets.py | 75 ++++++++++++- tests/test_snippet_provenance.py | 175 +++++++++++++++++++++++++++++++ 3 files changed, 268 insertions(+), 2 deletions(-) create mode 100644 tests/test_snippet_provenance.py diff --git a/src/scribe/mcp/tools/snippets.py b/src/scribe/mcp/tools/snippets.py index 9d00510..b48f0b2 100644 --- a/src/scribe/mcp/tools/snippets.py +++ b/src/scribe/mcp/tools/snippets.py @@ -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". diff --git a/src/scribe/services/snippets.py b/src/scribe/services/snippets.py index b4db170..9b71c45 100644 --- a/src/scribe/services/snippets.py +++ b/src/scribe/services/snippets.py @@ -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) diff --git a/tests/test_snippet_provenance.py b/tests/test_snippet_provenance.py new file mode 100644 index 0000000..ef35941 --- /dev/null +++ b/tests/test_snippet_provenance.py @@ -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