From 5d06b745994cc73ad587553ad324e44d4194be2d Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 21 Sep 2026 02:18:51 -0400 Subject: [PATCH] test(backup): prove the three restored code_shapes columns survive real Postgres (#4197) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The column guard is a claim about kwargs; "the value comes back" is a different claim, and only a round trip settles it. This drives the real `restore_full_backup` over a judged shape carrying all three of the columns that were going missing. `diverges_from` gets the harder assertion. It is a FK to notes.id, so the tempting fix — carry the exported id across — produces a row pointing at whatever note holds that number in the target database: not dropped, REATTACHED, with the restore reporting success and the divergence about the wrong snippet. So the test asserts WHOSE note the pointer landed on rather than which integer it holds, and it refuses to run at all if the restore happened to reuse the source id, which would let it pass without proving anything. Same shape as the assertion in the rule_usage round trip, for the same seam. Also corrects this module's own docstring, which said the round-trip module "is not written yet". Three existed; this is the fourth. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy --- ...integration_backup_code_shape_roundtrip.py | 199 ++++++++++++++++++ tests/test_services_backup.py | 12 +- 2 files changed, 207 insertions(+), 4 deletions(-) create mode 100644 tests/test_integration_backup_code_shape_roundtrip.py diff --git a/tests/test_integration_backup_code_shape_roundtrip.py b/tests/test_integration_backup_code_shape_roundtrip.py new file mode 100644 index 0000000..57b9244 --- /dev/null +++ b/tests/test_integration_backup_code_shape_roundtrip.py @@ -0,0 +1,199 @@ +"""Real-Postgres round trip for code_shapes (#4197). + +**What this guards is a column that was simply not there.** `code_shapes` +exported `reason_code`, `recheck_at` and `diverges_from` and imported none of +them — a restore reported success and came back with every judgment's verdict +and no code for why, every recheck flag cleared, and every divergence pointer +gone. The unit-level column guard in `test_services_backup.py` is what found +it and is what stops it recurring; this file is the behavioural half, because +"the builder passes the kwarg" and "the value survives a real restore" are +different claims. + +`diverges_from` gets the harder assertion, and it is the same shape as the one +`test_integration_backup_rule_usage_roundtrip.py` makes. It is a FK to +`notes.id`, so the tempting fix — carry the exported id across — produces a +row that points at whatever note happens to hold that number in the target +database. Not dropped, REATTACHED: the restore reports success and the +divergence is about the wrong snippet, with nothing downstream able to tell. +So the assertion is about WHOSE note the pointer landed on, not about which +integer it holds, and it fails if the answer ever becomes "the source's". +""" +import pytest +import pytest_asyncio +from datetime import datetime, timezone +from sqlalchemy import select + +from scribe.models import async_session +from scribe.models.code_shape import CodeShape +from scribe.models.note import Note +from scribe.models.project import Project +from scribe.models.user import User +from scribe.services import backup +from tests.helpers import ensure_user + +pytestmark = [pytest.mark.integration, pytest.mark.usefixtures("_dispose_engine")] + +OWNER_USERNAME = "code_shape_roundtrip_owner" +RESTORED_USERNAME = "code_shape_roundtrip_restored" + +REASON_CODE = "scoped-css" +RECHECK_AT = datetime(2026, 3, 4, 5, 6, tzinfo=timezone.utc) + + +async def _purge(username: str) -> None: + """project -> code_shapes is ON DELETE CASCADE, so dropping the projects + clears the shapes this file made.""" + async with async_session() as s: + for user in (await s.execute( + select(User).where(User.username == username) + )).scalars().all(): + for proj in (await s.execute( + select(Project).where(Project.user_id == user.id) + )).scalars().all(): + await s.delete(proj) + for note in (await s.execute( + select(Note).where(Note.user_id == user.id) + )).scalars().all(): + await s.delete(note) + await s.commit() + + +async def _purge_restored() -> None: + await _purge(RESTORED_USERNAME) + async with async_session() as s: + for user in (await s.execute( + select(User).where(User.username == RESTORED_USERNAME) + )).scalars().all(): + await s.delete(user) + await s.commit() + + +@pytest_asyncio.fixture(autouse=True) +async def _no_leftovers(): + """SETUP ONLY — a database call after a `yield` here orphans a pooled + connection and breaks unrelated tests (see the sibling round-trip files).""" + await _purge_restored() + await _purge(OWNER_USERNAME) + + +@pytest_asyncio.fixture +async def source(): + """One judged shape carrying all three of the columns that went missing, + plus TWO notes: the snippet it is an instance of, and the one it diverges + from. Two, because a pointer that resolves to the same note as the + snippet would pass whether it was re-mapped or coincidentally right.""" + async with async_session() as s: + owner = await ensure_user(s, OWNER_USERNAME) + uid = owner.id + await s.commit() + + async with async_session() as s: + proj = Project(user_id=uid, title="Shapes round trip") + s.add(proj) + await s.flush() + canon = Note(user_id=uid, title="the canon", body="", note_type="snippet") + other = Note(user_id=uid, title="the one it diverges from", body="", + note_type="snippet") + s.add_all([canon, other]) + await s.flush() + shape = CodeShape( + project_id=proj.id, + repo_key="git.example.com/x/y", + path="frontend/src/views/Thing.vue", + symbol="thing-row", + kind="css", + status="variant", + snippet_id=canon.id, + reason="deliberate departure, recorded", + reason_code=REASON_CODE, + classified_by="operator", + classified_at=datetime(2026, 2, 1, tzinfo=timezone.utc), + recheck_at=RECHECK_AT, + diverges_from=other.id, + ) + s.add(shape) + await s.flush() + + user_rows = backup._user_rows([owner]) + user_rows[0]["username"] = RESTORED_USERNAME + payload = { + "version": backup.BACKUP_VERSION, + "users": user_rows, + "projects": backup._project_rows([proj]), + "notes": backup._note_rows([canon, other]), + "code_shapes": backup._code_shape_rows([shape]), + } + source_ids = {"other_note_id": other.id, "canon_note_id": canon.id} + await s.commit() + + yield {"payload": payload, **source_ids} + + await _purge(OWNER_USERNAME) + + +@pytest_asyncio.fixture +async def restored(source): + await backup.restore_full_backup(source["payload"]) + async with async_session() as s: + user = (await s.execute( + select(User).where(User.username == RESTORED_USERNAME) + )).scalars().first() + assert user is not None, "the payload's user was not restored" + proj = (await s.execute( + select(Project).where(Project.user_id == user.id) + )).scalars().one() + shape = (await s.execute( + select(CodeShape).where(CodeShape.project_id == proj.id) + )).scalars().one() + notes = { + n.title: n for n in (await s.execute( + select(Note).where(Note.user_id == user.id) + )).scalars().all() + } + # Dedented on purpose: a `yield` inside the session block holds a pooled + # connection open for the whole test, which is the failure the sibling + # round-trip files warn about in their own fixtures. + yield {"user": user, "shape": shape, "notes": notes, "source": source} + + await _purge_restored() + + +async def test_the_reason_code_and_recheck_flag_survive_a_restore(restored): + """Both were exported and both were dropped. A ledger restored without + `reason_code` keeps its verdicts and loses the argument for them, which is + the column the accounting reads to tell one kind of exemption from + another; without `recheck_at` it looks settled and is not.""" + shape = restored["shape"] + assert shape.reason_code == REASON_CODE + assert shape.recheck_at is not None + assert shape.recheck_at.replace(tzinfo=timezone.utc) == RECHECK_AT + + +async def test_the_divergence_pointer_lands_on_the_restored_note(restored): + """Not "does it hold a number" — WHOSE note it points at. + + Carrying the exported id across would leave the pointer on the SOURCE + user's note, which still exists and still answers, so the row would read + as fine and be about the wrong snippet. Asserting on ownership fails in + that case even if the two ids happened to coincide. + """ + shape = restored["shape"] + expected = restored["notes"]["the one it diverges from"] + assert expected.id != restored["source"]["other_note_id"], ( + "the restore reused the source note id, so this test cannot tell a " + "remap from a copy — the fixture is not proving what it claims" + ) + + assert shape.diverges_from is not None, "the pointer was dropped" + assert shape.diverges_from == expected.id + + async with async_session() as s: + target = await s.get(Note, shape.diverges_from) + assert target is not None + assert target.user_id == restored["user"].id, ( + "the divergence points at a note belonging to the SOURCE user — the " + "exported id was carried across instead of re-mapped" + ) + assert shape.snippet_id == restored["notes"]["the canon"].id, ( + "the two note pointers were resolved through different maps" + ) diff --git a/tests/test_services_backup.py b/tests/test_services_backup.py index fa4a328..cc45bae 100644 --- a/tests/test_services_backup.py +++ b/tests/test_services_backup.py @@ -2,10 +2,14 @@ This is the no-database lane, so these cover the parts that need none: the version/coverage constants, the pure row helpers, and the export dict shape -(via a mocked session). The full FK-remapping round-trip needs real Postgres -and belongs in a `@pytest.mark.integration` module — it is not written yet, -which is why every row helper here is a plain function that can be tested -without a session. +(via a mocked session), plus the two COLUMN GUARDS — one per direction — which +need only a stand-in and a builder. + +The full FK-remapping round-trip needs real Postgres and lives in the +`test_integration_backup_*_roundtrip.py` modules, which drive the real +`restore_full_backup`. That sentence used to read "it is not written yet"; +four of those files exist now, the newest covering the three `code_shapes` +columns the import guard below found missing. """ from datetime import datetime, timezone from types import SimpleNamespace