test(review): cover findings #1 (non-bare caller id) and #4 (backfill connection scope)

#1: assert a URL-form / mixed-case caller id is normalized to the bare canonical form in the papers upsert (the fix's whole point; existing DQ-5 test only used a bare caller). #4: assert ra_grobid_backfill.main() opens two separate connections (read released before the GROBID loop, fresh one for the write) rather than holding one across the loop. Both pass; ruff clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Tarik Moussa
2026-06-27 09:09:39 +02:00
parent b2f01ce010
commit ab621fc5f5
2 changed files with 68 additions and 2 deletions

View File

@@ -6,10 +6,12 @@ Only the pure, offline logic is covered (id normalization + Citation assembly);
from __future__ import annotations
from unittest.mock import patch
from contextlib import contextmanager
from typing import Any
from unittest.mock import MagicMock, patch
from codex.models import Citation
from scripts.ra_grobid_backfill import _normalize_cited_id, build_grobid_citations
from scripts.ra_grobid_backfill import _normalize_cited_id, build_grobid_citations, main
# ---------------------------------------------------------------------------
# _normalize_cited_id
@@ -74,3 +76,37 @@ def test_build_citations_normalizes_dedups_and_drops_self() -> None:
def test_build_citations_empty_when_no_refs() -> None:
with patch("scripts.ra_grobid_backfill.extract_references", return_value=[]):
assert build_grobid_citations("10.1/x", "x.pdf") == []
# ---------------------------------------------------------------------------
# main — connection scope (review finding #4)
# ---------------------------------------------------------------------------
def test_main_uses_separate_connections_for_read_and_write(tmp_path: Any) -> None:
"""Review #4: the read connection is released BEFORE the slow GROBID loop, and a
fresh connection is opened for the write — so no idle-in-transaction connection
spans the per-paper network loop (which would abort on an SSH-tunnel drop)."""
(tmp_path / "paper.pdf").write_bytes(b"%PDF-1.4")
conns: list[MagicMock] = []
@contextmanager
def fake_get_conn() -> Any:
c = MagicMock()
c.execute.return_value.fetchall.return_value = [
{"id": "10.1/x", "source_path": "/d/paper.txt", "out_edges": 0}
]
c.execute.return_value.fetchone.return_value = {"papers": 1, "citing": 1, "edges": 1}
conns.append(c)
yield c
with (
patch("scripts.ra_grobid_backfill.get_conn", side_effect=fake_get_conn),
patch("scripts.ra_grobid_backfill.build_grobid_citations", return_value=[]),
):
rc = main(["--pdf-dir", str(tmp_path), "--write"])
assert rc == 0
# Two DISTINCT get_conn() context managers (read, then write) — not one held
# open across the GROBID loop.
assert len(conns) == 2