diff --git a/packages/core/src/repowise/core/persistence/crud/analysis/coverage.py b/packages/core/src/repowise/core/persistence/crud/analysis/coverage.py index dddc5b4635..8f89cd9b6b 100644 --- a/packages/core/src/repowise/core/persistence/crud/analysis/coverage.py +++ b/packages/core/src/repowise/core/persistence/crud/analysis/coverage.py @@ -53,11 +53,7 @@ async def save_coverage_files( p = provenance or CoverageProvenance() at = ingested_at or _now_utc() - existing = await session.execute( - select(CoverageFile).where(CoverageFile.repository_id == repository_id) - ) - for row in existing.scalars().all(): - await session.delete(row) + await session.execute(delete(CoverageFile).where(CoverageFile.repository_id == repository_id)) if await _ingest_is_single_row(session): await session.execute( delete(CoverageIngest).where(CoverageIngest.repository_id == repository_id) diff --git a/packages/core/src/repowise/core/persistence/crud/analysis/dead_code.py b/packages/core/src/repowise/core/persistence/crud/analysis/dead_code.py index 36b4ce94bb..90f0f45932 100644 --- a/packages/core/src/repowise/core/persistence/crud/analysis/dead_code.py +++ b/packages/core/src/repowise/core/persistence/crud/analysis/dead_code.py @@ -6,7 +6,7 @@ from collections.abc import Sequence from typing import Any -from sqlalchemy import select, text +from sqlalchemy import delete, select, text from sqlalchemy.ext.asyncio import AsyncSession from repowise.core.analysis.dead_code.deletion_gate import effective_safe_to_delete @@ -19,7 +19,7 @@ from repowise.core.analysis.finding_registry import excluded_types from ...models import DeadCodeFinding, _new_uuid -from .._shared import _BATCH_SIZE, _finding_file_path +from .._shared import _BATCH_SIZE, _batch_delete_in, _finding_file_path from ..repository import stamp_analysis_ran from .shown import gated_counts, shown, ungated @@ -125,15 +125,12 @@ async def save_dead_code_findings( findings: list[dict], ) -> None: """Persist dead code findings, replacing any existing open findings for the repo.""" - # Delete existing open findings for this repo before saving new ones - existing = await session.execute( - select(DeadCodeFinding).where( + await session.execute( + delete(DeadCodeFinding).where( DeadCodeFinding.repository_id == repository_id, DeadCodeFinding.status == "open", ) ) - for row in existing.scalars().all(): - await session.delete(row) null_lines_as_zero = await _lines_column_rejects_null(session) for i in range(0, len(findings), _BATCH_SIZE): @@ -196,18 +193,7 @@ async def replace_dead_code_findings( delete-then-insert with the surviving keys filtered out in Python rather than an ``ON CONFLICT`` upsert. """ - existing = await session.execute( - select(DeadCodeFinding).where(DeadCodeFinding.repository_id == repository_id) - ) - acted_on: set[tuple] = set() - for row in existing.scalars().all(): - if scope is not None and row.file_path not in scope: - continue - if row.status == "open": - await session.delete(row) - else: - acted_on.add((row.file_path, row.kind, row.symbol_name)) - await session.flush() + acted_on = await _replace_open_rows(session, repository_id, scope) writable = [ f @@ -223,6 +209,31 @@ async def replace_dead_code_findings( await stamp_analysis_ran(session, repository_id, "dead_code") +async def _replace_open_rows( + session: AsyncSession, repository_id: str, scope: frozenset[str] | set[str] | None +) -> set[tuple]: + """Delete the open rows in *scope*; return the (file, kind, symbol) of its acted-on rows. + + Acted-on rows are few, so they are read repo-wide and scoped here. + """ + repo = DeadCodeFinding.repository_id == repository_id + acted = await session.execute( + select(DeadCodeFinding.file_path, DeadCodeFinding.kind, DeadCodeFinding.symbol_name).where( + repo, DeadCodeFinding.status != "open" + ) + ) + acted_on = {tuple(r) for r in acted if scope is None or r[0] in scope} + is_open = DeadCodeFinding.status == "open" + if scope is None: + await session.execute(delete(DeadCodeFinding).where(repo, is_open)) + else: + await _batch_delete_in( + session, DeadCodeFinding, DeadCodeFinding.file_path, scope, prefilter=(repo, is_open) + ) + await session.flush() + return acted_on + + async def get_dead_code_findings( session: AsyncSession, repository_id: str, diff --git a/tests/unit/persistence/test_dead_code_crud.py b/tests/unit/persistence/test_dead_code_crud.py index ae8ddde5ae..d38a48c18c 100644 --- a/tests/unit/persistence/test_dead_code_crud.py +++ b/tests/unit/persistence/test_dead_code_crud.py @@ -472,3 +472,38 @@ async def test_legacy_not_null_store_still_accepts_an_unknown_count(async_sessio await save_dead_code_findings(async_session, repo.id, findings) await replace_dead_code_findings(async_session, repo.id, findings) assert sorted(r.lines for r in await _rows(async_session, repo.id)) == [0, 1] + + +async def test_scoped_replace_past_one_delete_chunk_keeps_exactly_the_right_rows(async_session): + """A scope wider than one delete chunk deletes every open row in it, and + only those: acted-on rows, rows outside the scope, and another repo's + rows come through unchanged.""" + repo = await insert_repo(async_session) + other = await insert_repo(async_session, name="other", local_path="/tmp/other") + paths = [f"f{i}.py" for i in range(1200)] + await save_dead_code_findings(async_session, repo.id, [_finding(p, "s") for p in paths]) + await save_dead_code_findings(async_session, other.id, [_finding("f0.py", "s")]) + await async_session.commit() + for row in await _rows(async_session, repo.id): + if row.file_path in {"f3.py", "f1100.py"}: + row.status = "ignored" + await async_session.commit() + + scope = set(paths[:1150]) + incoming = [_finding(p, "new") for p in ("f3.py", "f7.py", "f1199.py")] + await replace_dead_code_findings( + async_session, repo.id, [*incoming, _finding("f3.py", "s")], scope=scope + ) + await async_session.commit() + + rows = await _rows(async_session, repo.id) + got = sorted((r.file_path, r.symbol_name, r.status) for r in rows) + expected = sorted( + [("f3.py", "s", "ignored"), ("f1100.py", "s", "ignored")] + + [(p, "s", "open") for p in paths[1150:]] + + [("f3.py", "new", "open"), ("f7.py", "new", "open")] + ) + assert got == expected + assert [(r.file_path, r.status) for r in await _rows(async_session, other.id)] == [ + ("f0.py", "open") + ]