fix(6872): a verdict or write-path stamp on a vanished shape revives its row instead of inserting a duplicate
CI & Build / Python lint (push) Successful in 2s
CI & Build / Plugin hooks (push) Successful in 14s
CI & Build / TypeScript typecheck (push) Successful in 32s
CI & Build / Python tests (push) Successful in 1m46s
CI & Build / integration (push) Successful in 1m42s
CI & Build / Build & push image (push) Successful in 18s
CI & Build / Python lint (push) Successful in 2s
CI & Build / Plugin hooks (push) Successful in 14s
CI & Build / TypeScript typecheck (push) Successful in 32s
CI & Build / Python tests (push) Successful in 1m46s
CI & Build / integration (push) Successful in 1m42s
CI & Build / Build & push image (push) Successful in 18s
classify_shapes(repo=...) and the write-path hook looked only at live rows, so a shape whose row had vanished got a second provisional row under the same (project, repo, path, symbol, kind). The unique constraint refused it, and the whole classify batch was lost as a tool error. provisional_row() now finds the row under that identity, vanished or not. A vanished row is revived with a reappeared event and keeps its judgment. A new row is inserted inside a savepoint, so a concurrent insert of the same identity falls back to that row instead of failing the transaction. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -258,6 +258,46 @@ def _event(row: CodeShape, event: str, at: datetime, *, commit: str = "") -> Cod
|
||||
)
|
||||
|
||||
|
||||
async def provisional_row(
|
||||
session, *, project_id: int, repo_key: str, path: str, symbol: str, kind: str,
|
||||
) -> CodeShape:
|
||||
"""The row for a shape a caller met before any live row for it existed:
|
||||
a shape being written this turn, or one judged ahead of the sync.
|
||||
|
||||
The identity (project, repo, path, symbol, kind) is unique, so "no live
|
||||
row" does not mean "no row". A VANISHED row under the identity is revived
|
||||
and keeps its standing judgment, as a sync revives a shape that comes
|
||||
back. The next sync then confirms it, or stamps it vanished again until
|
||||
the code reaches the bound ref. With no row at all, a provisional one is
|
||||
inserted inside a savepoint. If a concurrent request inserts the same
|
||||
identity first, the savepoint rolls back and that request's row is used.
|
||||
The caller's transaction survives either way.
|
||||
"""
|
||||
from sqlalchemy.exc import IntegrityError
|
||||
|
||||
def _same():
|
||||
return select(CodeShape).where(
|
||||
CodeShape.project_id == project_id, CodeShape.repo_key == repo_key,
|
||||
CodeShape.path == path, CodeShape.symbol == symbol, CodeShape.kind == kind,
|
||||
)
|
||||
|
||||
row = (await session.execute(_same())).scalars().first()
|
||||
if row is None:
|
||||
try:
|
||||
async with session.begin_nested():
|
||||
row = CodeShape(project_id=project_id, repo_key=repo_key,
|
||||
path=path, symbol=symbol, kind=kind)
|
||||
session.add(row)
|
||||
await session.flush()
|
||||
return row
|
||||
except IntegrityError:
|
||||
row = (await session.execute(_same())).scalars().one()
|
||||
if row.vanished_at is not None:
|
||||
row.vanished_at = None
|
||||
session.add(_event(row, "reappeared", datetime.now(timezone.utc)))
|
||||
return row
|
||||
|
||||
|
||||
async def _judge(
|
||||
session, row: CodeShape, *, status: str, snippet_id: int | None,
|
||||
by: str | None, reason: str | None, at: datetime, reason_code: str | None = None,
|
||||
@@ -726,11 +766,10 @@ async def classify_shapes(
|
||||
if not matches and repo_key and item.get("status") != "unclassified":
|
||||
path = (item.get("path") or "").strip()
|
||||
symbol = (item.get("symbol") or "").strip()
|
||||
row = CodeShape(
|
||||
project_id=project_id, repo_key=repo_key, path=path,
|
||||
row = await provisional_row(
|
||||
session, project_id=project_id, repo_key=repo_key, path=path,
|
||||
symbol=symbol, kind=kind or snippet_kind(symbol, ""),
|
||||
)
|
||||
session.add(row)
|
||||
by_key.setdefault((path, symbol), []).append(row)
|
||||
matches = [row]
|
||||
provisional += 1
|
||||
@@ -1655,13 +1694,13 @@ async def suggest_write_path_instances(
|
||||
if row is None:
|
||||
if not repo_key:
|
||||
continue
|
||||
row = CodeShape(
|
||||
project_id=project_id, repo_key=repo_key,
|
||||
# It may be a vanished row coming back, with its judgment.
|
||||
row = await provisional_row(
|
||||
session, project_id=project_id, repo_key=repo_key,
|
||||
path=path, symbol=name, kind=kind,
|
||||
)
|
||||
session.add(row)
|
||||
by_key[(name, kind)] = row
|
||||
elif not (row.status in _MECHANICAL_TODO or row.classified_by == "hook"):
|
||||
if not (row.status in _MECHANICAL_TODO or row.classified_by == "hook"):
|
||||
continue # a judgment — or the canon itself — stands
|
||||
# A proposal, never a status: the writer decides.
|
||||
row.proposed_snippet_id = sid
|
||||
|
||||
@@ -1252,6 +1252,81 @@ async def test_a_shape_written_this_turn_is_judged_before_the_sync_sees_it(seede
|
||||
assert row.vanished_at is None and row.last_seen_commit == "c2"
|
||||
|
||||
|
||||
async def _rows_at(pid: int, path: str) -> list[CodeShape]:
|
||||
"""Every row at ``path``, vanished ones included: what the identity
|
||||
constraint sees."""
|
||||
async with async_session() as s:
|
||||
return list((await s.execute(select(CodeShape).where(
|
||||
CodeShape.project_id == pid, CodeShape.path == path,
|
||||
))).scalars().all())
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
async def test_judging_a_vanished_shape_revives_its_row(seeded):
|
||||
"""A verdict with `repo` on a shape whose row has vanished lands on that
|
||||
row and brings it back. It does not insert a second row under the same
|
||||
identity, which the database refuses, taking the whole batch with it."""
|
||||
from grimoire.services import repo_bindings as repo_bindings_svc
|
||||
|
||||
owner, pid, sid = seeded["owner"], seeded["pid"], seeded["snippet"]
|
||||
await repo_bindings_svc.set_binding(owner, REPO, pid)
|
||||
await sync_repo_shapes(
|
||||
pid, REPO, [s for s in SHAPES if s[0] != "src/util.py"], seen_marker="c1",
|
||||
)
|
||||
[gone] = await _rows_at(pid, "src/util.py")
|
||||
assert gone.vanished_at is not None
|
||||
|
||||
out = await classify_shapes(owner, pid, [
|
||||
{"path": "src/util.py", "symbol": "helper", "kind": "sym", "status": "exempt",
|
||||
"reason": "the module's own parse step", "reason_code": "pure-helper"},
|
||||
{"path": "src/app.py", "symbol": "make_app", "status": "instance", "snippet_id": sid},
|
||||
], repo=REPO)
|
||||
assert out["classified"] == 2 and out["unmatched"] == []
|
||||
|
||||
[row] = await _rows_at(pid, "src/util.py")
|
||||
assert row.id == gone.id and row.vanished_at is None
|
||||
assert (row.status, row.reason_code) == ("exempt", "pure-helper")
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
async def test_a_write_path_stamp_on_a_vanished_shape_revives_its_row(seeded):
|
||||
"""The write-path hook meets a shape being written again after a sync
|
||||
stamped it vanished. Its suggestion lands on the existing row, with no
|
||||
second insert, and a standing verdict on that row still stands."""
|
||||
from datetime import datetime, timezone
|
||||
|
||||
from grimoire.services.shape_ledger import suggest_write_path_instances
|
||||
|
||||
owner, pid, sid = seeded["owner"], seeded["pid"], seeded["snippet"]
|
||||
pulled = {sid: datetime.now(timezone.utc)}
|
||||
code = "def helper():\n return factory()\n"
|
||||
await sync_repo_shapes(
|
||||
pid, REPO, [s for s in SHAPES if s[0] != "src/util.py"], seen_marker="c1",
|
||||
)
|
||||
|
||||
suggested = await suggest_write_path_instances(
|
||||
owner, pid, path="src/util.py", shapes=[("sym", "helper")],
|
||||
code=code, pulled=pulled, repo_key=REPO,
|
||||
)
|
||||
assert [s["symbol"] for s in suggested] == ["helper"]
|
||||
[row] = await _rows_at(pid, "src/util.py")
|
||||
assert row.vanished_at is None and row.proposed_snippet_id == sid
|
||||
|
||||
# A verdict on the row, then another vanish: the next stamp leaves it be.
|
||||
await classify_shapes(owner, pid, [
|
||||
{"path": "src/util.py", "symbol": "helper", "status": "exempt", "reason": "local"},
|
||||
])
|
||||
await sync_repo_shapes(
|
||||
pid, REPO, [s for s in SHAPES if s[0] != "src/util.py"], seen_marker="c2",
|
||||
)
|
||||
assert await suggest_write_path_instances(
|
||||
owner, pid, path="src/util.py", shapes=[("sym", "helper")],
|
||||
code=code, pulled=pulled, repo_key=REPO,
|
||||
) == []
|
||||
[row] = await _rows_at(pid, "src/util.py")
|
||||
assert row.status == "exempt" and row.classified_by == "agent"
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
async def test_unjudged_is_what_nobody_read(seeded):
|
||||
"""The end-of-turn question: a shape with no row, an unclassified one, a
|
||||
|
||||
Reference in New Issue
Block a user