fix(ingest): make both upsert UNIQUE gaps visible (audit C-15 follow-up)
Per review: the two latent upsert conflicts should be observable even where not auto-fixed. - bibkey collision (auto-suffixed) now logs a warning so a paper landing as 'BobenkoLutz2023a' instead of '…2023' isn't silent. - openalex_id collision (two papers claiming the same OpenAlex work — a real data conflict, not auto-renamable) now raises a clear ValueError naming the openalex_id and the offending paper, instead of the raw psycopg UniqueViolation. Regression test added for the openalex_id error path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -164,10 +164,26 @@ def ingest_paper(
|
|||||||
)
|
)
|
||||||
break
|
break
|
||||||
except psycopg.errors.UniqueViolation as exc:
|
except psycopg.errors.UniqueViolation as exc:
|
||||||
if base_bibkey is None or "bibkey" not in str(exc).lower() or attempt == 26:
|
msg = str(exc).lower()
|
||||||
raise
|
|
||||||
conn.rollback()
|
conn.rollback()
|
||||||
|
if "openalex" in msg:
|
||||||
|
# Two different papers.id claim the same OpenAlex work — a real data
|
||||||
|
# conflict, not something to auto-rename. Surface it clearly instead
|
||||||
|
# of the raw psycopg error (companion gap to C-15's bibkey case).
|
||||||
|
raise ValueError(
|
||||||
|
f"openalex_id {paper.openalex_id!r} is already attached to a "
|
||||||
|
f"different paper; ingesting '{paper.id}' would duplicate it — "
|
||||||
|
"resolve the conflicting paper first."
|
||||||
|
) from exc
|
||||||
|
if base_bibkey is None or "bibkey" not in msg or attempt == 26:
|
||||||
|
raise
|
||||||
paper.bibkey = f"{base_bibkey}{chr(ord('a') + attempt)}"
|
paper.bibkey = f"{base_bibkey}{chr(ord('a') + attempt)}"
|
||||||
|
logger.warning(
|
||||||
|
"bibkey %r already exists; ingesting '%s' as %r instead",
|
||||||
|
base_bibkey,
|
||||||
|
paper.id,
|
||||||
|
paper.bibkey,
|
||||||
|
)
|
||||||
|
|
||||||
# Register openalex_id in paper_identifiers (alias table).
|
# Register openalex_id in paper_identifiers (alias table).
|
||||||
# Every ingest records the primary openalex_id so the graph JOIN
|
# Every ingest records the primary openalex_id so the graph JOIN
|
||||||
|
|||||||
@@ -560,6 +560,35 @@ def test_ingest_disambiguates_bibkey_on_unique_violation() -> None:
|
|||||||
mock_conn.rollback.assert_called_once()
|
mock_conn.rollback.assert_called_once()
|
||||||
|
|
||||||
|
|
||||||
|
def test_ingest_openalex_collision_raises_clear_error() -> None:
|
||||||
|
"""An openalex_id UNIQUE collision raises a clear, actionable error (audit C-15).
|
||||||
|
|
||||||
|
Unlike a bibkey clash (auto-suffixed), a shared openalex_id means two papers
|
||||||
|
claim the same OpenAlex work — a real data conflict that must surface, not the
|
||||||
|
raw psycopg error.
|
||||||
|
"""
|
||||||
|
paper = _make_paper()
|
||||||
|
mock_conn = MagicMock()
|
||||||
|
|
||||||
|
def execute_side_effect(sql: str, params: Any = None) -> MagicMock:
|
||||||
|
if "INSERT INTO papers" in sql:
|
||||||
|
raise psycopg.errors.UniqueViolation(
|
||||||
|
'duplicate key value violates unique constraint "papers_openalex_id_key"'
|
||||||
|
)
|
||||||
|
return MagicMock()
|
||||||
|
|
||||||
|
mock_conn.execute.side_effect = execute_side_effect
|
||||||
|
|
||||||
|
with (
|
||||||
|
patch("codex.ingest.openalex.fetch_paper", return_value=paper),
|
||||||
|
patch("codex.ingest.openalex.fetch_citations", return_value=[]),
|
||||||
|
patch("codex.ingest.get_embedder", return_value=_fake_embedder()),
|
||||||
|
patch("codex.ingest.get_conn", side_effect=_make_conn_cm(mock_conn)),
|
||||||
|
pytest.raises(ValueError, match="openalex_id"),
|
||||||
|
):
|
||||||
|
ingest_paper(paper.id)
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user