From 8e2f9e037dac5dfa3c59d13fa42c78dee906fd55 Mon Sep 17 00:00:00 2001 From: Tarik Moussa Date: Mon, 15 Jun 2026 12:09:38 +0200 Subject: [PATCH] fix(ingest): key formulas/figures on papers.id, not filename stem (audit C-10) extract_formulas/extract_figures derive paper_id from Path(pdf_path).stem, which need not equal papers.id (and won't, given canonical ids). Inserting that stem violated the formulas/figures -> papers(id) FK on rich (.pdf) ingest. The DELETE already used paper.id; the INSERTs now do too. Strengthened the rich-ingest test: the fake formula/figure carry a filename-stem paper_id distinct from papers.id, and the test asserts the inserted rows are keyed on papers.id. Co-Authored-By: Claude Fable 5 --- codex/ingest.py | 8 ++++++-- tests/ingest/test_ingest.py | 16 ++++++++++++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/codex/ingest.py b/codex/ingest.py index b43fdd7..31e7e3d 100644 --- a/codex/ingest.py +++ b/codex/ingest.py @@ -294,7 +294,10 @@ def ingest_paper( VALUES (%s, %s, %s, %s, %s) """, [ - (f.paper_id, f.page, f.raw_latex, f.context, f.eq_label) + # paper.id, not f.paper_id: the extractor derives its + # paper_id from the PDF filename stem, which need not match + # papers.id and would violate the FK (audit C-10). + (paper.id, f.page, f.raw_latex, f.context, f.eq_label) for f in formulas ], ) @@ -308,7 +311,8 @@ def ingest_paper( VALUES (%s, %s, %s, %s) """, [ - (fig.paper_id, fig.page, fig.caption, fig.image_path) + # paper.id, not fig.paper_id (filename stem) — see C-10 above. + (paper.id, fig.page, fig.caption, fig.image_path) for fig in figures ], ) diff --git a/tests/ingest/test_ingest.py b/tests/ingest/test_ingest.py index 638a1ec..016854a 100644 --- a/tests/ingest/test_ingest.py +++ b/tests/ingest/test_ingest.py @@ -351,9 +351,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() @@ -380,6 +382,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."""