user2595/codex-py
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: audit remediation Wave 2 — identity normalization C-7, C-10 (+ T-3, ADR D-1) #13

Merged
user2595 merged 5 commits from fix/audit-wave-2 into main 2026-06-16 04:53:41 +00:00
Conversation 0 Commits 5 Files Changed 5 +238 -7
5 changed files with 238 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
codex/ingest.py
View File

@@ -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
],
)

36
docs/adr/ADR-F15-literature-graph.md
View File

@@ -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.

139
infra/reingest_canonical_ids.sh Executable file
View File

@@ -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 <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)."

47
tests/ingest/test_ingest.py
View File

@@ -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."""

7
tests/sources/test_openalex.py
View File

@@ -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: