diff --git a/codex/ingest.py b/codex/ingest.py index b63799d..31e7e3d 100644 --- a/codex/ingest.py +++ b/codex/ingest.py @@ -98,6 +98,14 @@ def ingest_paper( if paper is None: raise ValueError(f"Paper not found: {paper_id}") + # Canonical identity is the caller-supplied id — what the CLI / ingest scripts + # use and what papers.id is contracted to hold (arXiv id or DOI). OpenAlex's + # fetch_paper fills paper.id with the URL-form doi/id (e.g. + # "https://doi.org/10.48550/arxiv.math/0603097"), which broke bare-id lookups + # and cross-citation matching (audit C-7). The OpenAlex work id is retained + # separately as openalex_id and in the paper_identifiers alias table. + paper.id = paper_id.strip() + # Auto-generate bibkey when source has none (D-04). if paper.bibkey is None: paper.bibkey = _make_bibkey(paper) @@ -286,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 ], ) @@ -300,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/docs/adr/ADR-F15-literature-graph.md b/docs/adr/ADR-F15-literature-graph.md index 8a25bff..4733885 100644 --- a/docs/adr/ADR-F15-literature-graph.md +++ b/docs/adr/ADR-F15-literature-graph.md @@ -1,8 +1,10 @@ # ADR-F15 — Literature Graph / Citation PageRank -**Status:** IMPL (GO after live-corpus spike 2026-06-15) +**Status:** IMPL — GO after live-corpus spike 2026-06-15. See **Audit Update +(2026-06-15)** at the end: the ID-format mismatch is now resolved in code +(audit C-1, C-7) and the Spike Result numbers below predate that resolution. **Owners:** F-15 Worker (Sonnet) -**Last updated:** 2026-06-15 +**Last updated:** 2026-06-15 (audit addendum) --- @@ -138,6 +140,12 @@ If the `citations` table is empty: graph has 0 nodes; `graph report` prints ## Known Limitation: ID-Format Mismatch +> **RESOLVED (audit 2026-06-15) — see Audit Update below.** This no longer holds: +> `build_citation_graph` resolves OpenAlex `cited_id`s to the canonical +> `papers.id` via the shared `RESOLVED_CITATIONS_SQL` (C-1) and ingest stores +> canonical ids (C-7), so inter-KB citations *are* now edges. Original text kept +> for the record. + `cited_id` uses OpenAlex IDs; `papers.id` uses DOIs. Cross-citations between ingested papers are therefore not represented as graph edges. This is a D-05-class issue (same root cause as the `citing_id`/`openalex_id` @@ -155,3 +163,27 @@ small while the corpus is < 100 papers. - R-44: `codex graph report [--top-n N] [--json]` ✓ - R-45: graceful degradation < 5 nodes: uniform scores, warning ✓ - R-46: **this document** — GO; Bobenko+Springborn 2007 at rank 7 validates hub detection; score flatness expected at corpus size 29; increase to > 100 papers for stronger PageRank differentiation + +--- + +## Audit Update (2026-06-15) + +A post-hoc audit of the shipped F-15 code found that the live-corpus spike above +was measured on a graph the current code no longer produces: + +- **ID-format mismatch resolved.** `build_citation_graph` resolves OpenAlex + `cited_id`s to the canonical `papers.id` through the shared + `RESOLVED_CITATIONS_SQL`, now also used by `codex.discover` so the two cannot + diverge (audit C-1). Ingest stores the caller's canonical id as `papers.id` + instead of OpenAlex's URL-form doi (audit C-7). Inter-KB citations are + therefore represented as edges — the "Known Limitation" above is obsolete. +- **Spike numbers are stale.** The 478 dangling nodes, the rank-7 + Bobenko/Springborn result, and the 5.7 % score spread were measured *before* + the resolution + canonicalisation. They must be **re-measured after the corpus + is re-ingested** onto canonical ids (the chosen C-7 migration); until then the + GO rests on a superseded topology. + +**Action (pending):** after re-ingesting via `ingest_all.sh`, re-run +`codex graph report`, refresh the Spike Result table, and confirm the +foundational hubs (Pinkall/Polthier, Yamabe, Schoen, Bobenko/Springborn) still +surface. diff --git a/infra/reingest_canonical_ids.sh b/infra/reingest_canonical_ids.sh new file mode 100755 index 0000000..600ade3 --- /dev/null +++ b/infra/reingest_canonical_ids.sh @@ -0,0 +1,139 @@ +#!/usr/bin/env bash +# One-time migration to canonical paper IDs (audit C-7). +# +# Before the C-7 fix, ingest stored OpenAlex's URL-form doi/id as papers.id +# (e.g. "https://doi.org/10.48550/arxiv.math/0603097"). After the fix, ingest +# keys papers.id on the bare caller id ("math/0603097"). The existing rows keep +# their old URL-form primary keys, so a plain re-ingest would DUPLICATE every +# paper (new bare-id row alongside the old URL-id row). This script wipes the +# corpus and rebuilds it on canonical ids. +# +# DESTRUCTIVE: `TRUNCATE papers CASCADE` removes papers + chunks + citations + +# formulas + figures + paper_identifiers (code_links.paper_id is set NULL). All +# content is rebuilt by ingest_all.sh, which re-fetches OpenAlex and re-embeds +# (minutes for ~36 papers). +# +# Prerequisites: SSH tunnel on :5433 (see ingest_all.sh) and .env.jetson-ingest. +# Usage: bash infra/reingest_canonical_ids.sh +set -euo pipefail + +CODEX_DIR="$(cd "$(dirname "$0")/.." && pwd)" +ENV_FILE="$CODEX_DIR/.env.jetson-ingest" +PY="$CODEX_DIR/.venv/bin/python" +TUNNEL_PORT=5433 + +# ── 1. Preconditions ───────────────────────────────────────────────────────── +if ! nc -z localhost "$TUNNEL_PORT" 2>/dev/null; then + echo "ERROR: SSH tunnel not active on :$TUNNEL_PORT" + echo "Run: ssh -f -N -L 5433:localhost:5432 alfred@192.168.178.103" + exit 1 +fi +[[ -f "$ENV_FILE" ]] || { echo "ERROR: $ENV_FILE missing"; exit 1; } +[[ -x "$PY" ]] || { echo "ERROR: venv python not found at $PY (run uv sync)"; exit 1; } +# Load DATABASE_URL into the environment (not echoed — keeps the password out of logs). +set -a; source "$ENV_FILE"; set +a + +# run_sql : execute one statement via psycopg, print any result rows. +# Reads DATABASE_URL from the environment; suppresses the DSN on error. +run_sql() { + "$PY" - "$1" <<'PYEOF' +import os, sys, psycopg +from psycopg.rows import dict_row +try: + with psycopg.connect(os.environ["DATABASE_URL"], row_factory=dict_row, connect_timeout=10) as c: + cur = c.execute(sys.argv[1]) + if cur.description: + for row in cur.fetchall(): + print(" " + " ".join(f"{k}={v!r}" for k, v in row.items())) + c.commit() +except Exception as e: + print(f" DB ERROR: {type(e).__name__} (details suppressed to avoid DSN leak)") + sys.exit(1) +PYEOF +} + +# ── 2. BEFORE snapshot ─────────────────────────────────────────────────────── +echo "── BEFORE migration ──────────────────────────────────────────" +run_sql "SELECT count(*) AS papers, count(*) FILTER (WHERE starts_with(id, 'https://')) AS url_form_ids FROM papers" +echo " sample ids:" +run_sql "SELECT id FROM papers ORDER BY added_at LIMIT 3" + +# ── 2b. Preflight: verify the schema is re-ingestable BEFORE wiping anything ── +# ingest writes chunks.section (F-16). On a live DB that predates it the column +# is missing, and the app user cannot ALTER tables owned by 'postgres' (audit +# M-1). Check FIRST so we never TRUNCATE and then fail mid-re-ingest. +echo "── Preflight: schema readiness ───────────────────────────────" +SECTION_OK="$("$PY" - <<'PYEOF' +import os, psycopg +try: + with psycopg.connect(os.environ["DATABASE_URL"], connect_timeout=10) as c: + cols = [r[0] for r in c.execute( + "SELECT column_name FROM information_schema.columns WHERE table_name='chunks'").fetchall()] + print("yes" if "section" in cols else "no") +except Exception: + print("error") +PYEOF +)" +if [[ "$SECTION_OK" != "yes" ]]; then + echo "ABORT — schema not ready: chunks.section is missing (audit M-1)." + echo "The app user cannot add it (chunks is owned by 'postgres'). Apply it as" + echo "the table owner, then re-run this script:" + echo "" + echo " ssh alfred@192.168.178.103 \\" + echo " \"sudo -u postgres psql -d papers -c \\\\\"ALTER TABLE chunks ADD COLUMN IF NOT EXISTS section TEXT;\\\\\"\"" + echo "" + echo "Nothing was changed — no TRUNCATE was issued." + exit 1 +fi +echo " chunks.section present — schema OK." + +# ── 3. Confirmation gate (destructive) ─────────────────────────────────────── +echo "" +echo "This TRUNCATEs papers CASCADE (papers/chunks/citations/formulas/figures/" +echo "paper_identifiers) and re-ingests via ingest_all.sh. The DB content is" +echo "rebuilt from scratch." +read -r -p "Type 'MIGRATE' to proceed: " confirm +[[ "$confirm" == "MIGRATE" ]] || { echo "Aborted — nothing changed."; exit 1; } + +# ── 4. Wipe ────────────────────────────────────────────────────────────────── +echo "── TRUNCATE papers CASCADE ───────────────────────────────────" +run_sql "TRUNCATE papers CASCADE" +echo " wiped." + +# ── 5. Re-ingest on canonical ids ──────────────────────────────────────────── +echo "── Re-ingest (ingest_all.sh) ─────────────────────────────────" +bash "$CODEX_DIR/ingest_all.sh" + +# ── 6. AFTER snapshot + live verification ──────────────────────────────────── +echo "── AFTER migration ───────────────────────────────────────────" +echo " C-7 — url_form_ids should now be 0; sample ids should be bare:" +run_sql "SELECT count(*) AS papers, count(*) FILTER (WHERE starts_with(id, 'https://')) AS url_form_ids FROM papers" +run_sql "SELECT id FROM papers ORDER BY added_at LIMIT 3" + +# C-1 must be verified through the REAL resolver-based discovery_leads(), not a +# raw cited_id check: cited_id/openalex_id stay in OpenAlex form, so the raw +# "cited_id NOT IN papers.id" count is non-zero by design — the resolver is what +# excludes ingested papers. Check that no ingested paper leaks into the leads. +echo " C-1 — real discovery_leads() must contain no already-ingested paper:" +PYTHONPATH="$CODEX_DIR" "$PY" - <<'PYEOF' +import os, psycopg +from psycopg.rows import dict_row +from codex.discover import discovery_leads +try: + leads = discovery_leads(limit=100000) + with psycopg.connect(os.environ["DATABASE_URL"], row_factory=dict_row, connect_timeout=10) as c: + ingested = {r["id"] for r in c.execute("SELECT id FROM papers").fetchall()} + ingested |= { + r["openalex_id"] + for r in c.execute("SELECT openalex_id FROM papers WHERE openalex_id IS NOT NULL").fetchall() + } + leaked = sum(1 for lead in leads if lead["cited_id"] in ingested) + print(f" leads={len(leads)} ingested-papers-leaked-into-leads={leaked}") +except Exception as e: + print(f" DB ERROR: {type(e).__name__} (details suppressed)") +PYEOF + +echo "" +echo "✓ Migration complete." +echo " Expected: url_form_ids=0 (C-7 verified) and leaked=0 (C-1 verified) above." +echo " Next: re-run 'codex graph report' and refresh the ADR-F15 spike table (audit D-1)." diff --git a/tests/ingest/test_ingest.py b/tests/ingest/test_ingest.py index 80e4d23..016854a 100644 --- a/tests/ingest/test_ingest.py +++ b/tests/ingest/test_ingest.py @@ -106,6 +106,37 @@ def test_ingest_paper_basic() -> None: ) +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 # --------------------------------------------------------------------------- @@ -320,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() @@ -349,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.""" diff --git a/tests/sources/test_openalex.py b/tests/sources/test_openalex.py index d481aa2..5a78b99 100644 --- a/tests/sources/test_openalex.py +++ b/tests/sources/test_openalex.py @@ -15,7 +15,8 @@ from codex.sources.openalex import _resolve_id _SAMPLE_WORK = { "id": "https://openalex.org/W2741809807", - "doi": "10.1145/3592430", + # Real OpenAlex returns the DOI as a full URL, not a bare DOI (audit T-3). + "doi": "https://doi.org/10.1145/3592430", "title": "Conformal Prediction: A Review", "publication_year": 2023, "authorships": [ @@ -90,6 +91,10 @@ def test_fetch_paper_success(monkeypatch: pytest.MonkeyPatch) -> None: assert paper.authors == ["Alice Smith", "Bob Jones"] assert paper.abstract == "Conformal prediction is great" assert paper.openalex_id == "https://openalex.org/W2741809807" + # fetch_paper maps id from the (URL-form) doi; ingest later canonicalises + # papers.id to the caller's bare id (audit C-7). Pin the real mapping here so + # the fixture cannot silently misrepresent OpenAlex again (audit T-3). + assert paper.id == "https://doi.org/10.1145/3592430" def test_fetch_paper_arxiv_id_prefixed(monkeypatch: pytest.MonkeyPatch) -> None: