merge: reconcile data-quality roadmap with audit-remediation main (#12-15)
Resolved 4 conflicts: cli.py (R-D citing-coverage warning + main's small-corpus JSON/hub-title changes both kept); ingest.py (C-7 paper.id=caller-id pin first, then DQ-2 abstract recovery); test_ingest.py + test_openalex.py (kept both branches' tests). C-7/DQ-5 overlap (both canonicalise papers.id): kept both layers — openalex._canonical_id normalizes fetch_paper's id (DQ-5); ingest pins papers.id to the caller's bare id (C-7); they converge on the bare canonical id. Fixed the auto-merged test_fetch_paper_success that had contradictory bare/URL assertions. 402 tests pass; ruff + mypy clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -12,6 +12,7 @@ from unittest.mock import MagicMock, patch
|
||||
|
||||
import httpx
|
||||
import numpy as np
|
||||
import psycopg
|
||||
import pytest
|
||||
|
||||
from codex.ingest import IngestResult, _make_bibkey, ingest_paper
|
||||
@@ -161,6 +162,37 @@ def test_ingest_crossref_not_called_when_openalex_has_citations() -> None:
|
||||
mock_cr.assert_not_called()
|
||||
|
||||
|
||||
def test_ingest_canonicalises_paper_id_to_caller_arg() -> None:
|
||||
"""papers.id is the caller-supplied id, not OpenAlex's URL-form id (audit C-7).
|
||||
|
||||
Real OpenAlex returns the doi/id as a full URL; ingest must key the paper on
|
||||
the bare id the caller passed (matching ingest scripts and CLI lookups), and
|
||||
keep the OpenAlex work id only as openalex_id.
|
||||
"""
|
||||
oa_paper = Paper(
|
||||
id="https://doi.org/10.48550/arxiv.math/0603097", # URL form, as OpenAlex emits
|
||||
title="A Test Paper",
|
||||
openalex_id="https://openalex.org/W4301005794",
|
||||
authors=["Alice", "Bob"],
|
||||
year=2008,
|
||||
abstract="Test abstract.",
|
||||
)
|
||||
mock_conn = MagicMock()
|
||||
|
||||
with (
|
||||
patch("codex.ingest.openalex.fetch_paper", return_value=oa_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)),
|
||||
):
|
||||
result = ingest_paper("math/0603097")
|
||||
|
||||
papers_params = mock_conn.execute.call_args_list[0][0][1]
|
||||
assert papers_params["id"] == "math/0603097", "papers.id must be the caller's bare id"
|
||||
assert papers_params["openalex_id"] == "https://openalex.org/W4301005794"
|
||||
assert result.paper_id == "math/0603097"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 2. test_ingest_paper_source_tex
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -562,9 +594,11 @@ def test_ingest_paper_rich_calls_formula_and_figure_parsers(tmp_path: Any) -> No
|
||||
mock_conn.executemany = MagicMock()
|
||||
mock_conn.commit = MagicMock()
|
||||
|
||||
fake_formula = FormulaChunk(paper_id=paper.id, page=1, raw_latex=r"\alpha", context="test")
|
||||
# Extractors derive paper_id from the PDF filename stem ("paper"), which differs
|
||||
# from papers.id — ingest must insert papers.id, not this stem (audit C-10).
|
||||
fake_formula = FormulaChunk(paper_id="paper", page=1, raw_latex=r"\alpha", context="test")
|
||||
fake_figure = FigureChunk(
|
||||
paper_id=paper.id, page=1, image_path="/tmp/fig.png", caption="Figure 1."
|
||||
paper_id="paper", page=1, image_path="/tmp/fig.png", caption="Figure 1."
|
||||
)
|
||||
|
||||
mock_settings = MagicMock()
|
||||
@@ -591,6 +625,16 @@ def test_ingest_paper_rich_calls_formula_and_figure_parsers(tmp_path: Any) -> No
|
||||
assert result.formulas_upserted == 1
|
||||
assert result.figures_upserted == 1
|
||||
|
||||
# C-10: formula/figure rows must be keyed on papers.id, not the extractor's
|
||||
# filename-stem paper_id, or the FK to papers(id) is violated.
|
||||
cursor = mock_conn.cursor.return_value.__enter__.return_value
|
||||
inserted_paper_ids = {
|
||||
row[0] for call in cursor.executemany.call_args_list for row in call[0][1]
|
||||
}
|
||||
assert inserted_paper_ids == {paper.id}, (
|
||||
f"formula/figure inserts must use papers.id, got {inserted_paper_ids}"
|
||||
)
|
||||
|
||||
|
||||
def test_ingest_paper_no_rich_skips_parsers(tmp_path: Any) -> None:
|
||||
"""When rich=False (default), formula and figure parsers are NOT called."""
|
||||
@@ -781,6 +825,69 @@ def test_ingest_doi_paper_upserts_bare_canonical_id() -> None:
|
||||
)
|
||||
|
||||
|
||||
def test_ingest_disambiguates_bibkey_on_unique_violation() -> None:
|
||||
"""A papers.bibkey UNIQUE collision is retried with an a/b/c suffix (audit C-15)."""
|
||||
paper = _make_paper() # bibkey auto-generates to "AliceBob2023"
|
||||
mock_conn = MagicMock()
|
||||
state = {"first": True}
|
||||
|
||||
def execute_side_effect(sql: str, params: Any = None) -> MagicMock:
|
||||
# The first papers INSERT collides on the bibkey; the retry must succeed.
|
||||
if "INSERT INTO papers" in sql and state["first"]:
|
||||
state["first"] = False
|
||||
raise psycopg.errors.UniqueViolation(
|
||||
'duplicate key value violates unique constraint "papers_bibkey_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)),
|
||||
):
|
||||
ingest_paper(paper.id)
|
||||
|
||||
papers_inserts = [
|
||||
c for c in mock_conn.execute.call_args_list if "INSERT INTO papers" in c[0][0]
|
||||
]
|
||||
assert len(papers_inserts) == 2, "expected one retry after the bibkey collision"
|
||||
assert papers_inserts[0][0][1]["bibkey"] == "AliceBob2023"
|
||||
assert papers_inserts[1][0][1]["bibkey"] == "AliceBob2023a"
|
||||
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