From 115bb63f2d865f64cfb26a7664b8084a8e3a904e Mon Sep 17 00:00:00 2001 From: Tarik Moussa Date: Mon, 15 Jun 2026 11:06:12 +0200 Subject: [PATCH 01/24] fix(wiki): quarantine zero-citation pages instead of publishing (audit C-4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit compile_all's quarantine gate was guarded by `total_claims > 0`, so a page with no parseable citations — including the LLM-outage case where compile_concept returns an empty page — fell through to the publish branch and was written to wiki/ + marked compiled. Such a page carries zero grounding evidence. Now: total_claims == 0 (or grounding_rate below threshold) quarantines and does NOT update the compile-state, so a transient LLM failure retries next run instead of freezing an empty/uncited page as 'compiled'. Empty bodies are recorded but not written as draft files. Regression test reproduces the exact bypass (uncited LLM output -> quarantined, not published). Co-Authored-By: Claude Fable 5 --- codex/wiki.py | 20 ++++++++++++------- tests/wiki/test_compile.py | 41 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 7 deletions(-) diff --git a/codex/wiki.py b/codex/wiki.py index b47a98a..b913258 100644 --- a/codex/wiki.py +++ b/codex/wiki.py @@ -722,17 +722,23 @@ def compile_all( # Detect conflicts between chunks from different bibkeys report.conflicts.extend(_detect_conflicts(concept.slug, chunks)) - # Quarantine check: compute grounding rate + # Quarantine check: compute grounding rate. + # A page with NO citations at all (total_claims == 0) carries zero grounding + # evidence — this also covers the LLM-outage case where compile_concept + # returns an empty page — and must NOT be published (audit C-4). Previously + # the `total_claims > 0` guard let such pages fall through to wiki/. total_claims = len(page.claims) grounded_claims = sum(1 for c in page.claims if c.grounded) grounding_rate = grounded_claims / total_claims if total_claims > 0 else 0.0 - if total_claims > 0 and grounding_rate < settings.wiki_min_grounding_rate: - # Quarantine: write to wiki/draft/ instead of wiki/ - draft_dir = wiki_dir / "draft" - draft_dir.mkdir(parents=True, exist_ok=True) - page_path = draft_dir / f"{concept.slug}.md" - page_path.write_text(page.markdown, encoding="utf-8") + if total_claims == 0 or grounding_rate < settings.wiki_min_grounding_rate: + # Quarantine and do NOT update the compile-state, so a transient failure + # (e.g. LLM outage) is retried on the next run instead of being frozen as + # "compiled". An empty body is not worth a draft file — just record it. + if page.markdown.strip(): + draft_dir = wiki_dir / "draft" + draft_dir.mkdir(parents=True, exist_ok=True) + (draft_dir / f"{concept.slug}.md").write_text(page.markdown, encoding="utf-8") report.quarantined.append(concept.slug) else: # Write page to wiki/ diff --git a/tests/wiki/test_compile.py b/tests/wiki/test_compile.py index 53ff8fc..6835ec0 100644 --- a/tests/wiki/test_compile.py +++ b/tests/wiki/test_compile.py @@ -504,6 +504,47 @@ def test_log_append_records_skipped( assert "circle-packing" in content +# --------------------------------------------------------------------------- +# Audit C-4: zero-citation pages must NOT bypass the quarantine +# --------------------------------------------------------------------------- + + +def test_zero_citation_page_is_quarantined_not_published( + mock_retrieve: None, + mock_formulas: None, + mock_settings: Path, +) -> None: + """A confident-but-uncited LLM response yields zero parseable claims + (grounding_rate 0.0). It must be quarantined — not written to wiki/ and + marked compiled. Regression for audit C-4: the old ``total_claims > 0`` + guard let zero-claim (and LLM-outage empty) pages fall through to wiki/. + """ + from codex.wiki import compile_all + + (mock_settings / "concepts.yaml").write_text( + textwrap.dedent( + """\ + concepts: + - slug: lobachevsky-function + title: Lobachevsky Function + aliases: [Milnor Lobachevsky] + """ + ), + encoding="utf-8", + ) + # No [BibKey] citations anywhere → zero claims → grounding_rate 0.0. + uncited = MockLLM( + "The Lobachevsky function is obviously central to all of geometry. " + "Every result follows from it. This paragraph cites nothing at all." + ) + + report = compile_all(changed_only=False, llm=uncited, output_dir=str(mock_settings)) + + assert "lobachevsky-function" in report.quarantined + assert "lobachevsky-function" not in report.compiled + assert not (mock_settings / "lobachevsky-function.md").exists() + + # --------------------------------------------------------------------------- # Idempotency: second compile without chunk change → no rewrite # --------------------------------------------------------------------------- From 3092f1814ec77c7034e96e6b3e2fdb381917acc8 Mon Sep 17 00:00:00 2001 From: Tarik Moussa Date: Mon, 15 Jun 2026 11:09:24 +0200 Subject: [PATCH 02/24] fix(synthesis): namespace lead ids by kind to stop collisions (audit C-11) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each stage (find_connections/gaps/improvements/propose_conjectures) numbered its leads from seq=1, so connection L-0001, gap L-0001 and improvement L-0001 all resolved to grounded/L-0001.md. The CLI concatenates them (connections + gaps + improvements) and write_leads writes in order, so later kinds silently overwrote earlier ones — survivors = max(per-kind count), not the sum. Reproduced: 4 distinct leads -> 2 files. _make_lead_id now takes the kind and emits L-C-/L-G-/L-I-/L-X- prefixes, keeping same-seq ids distinct across stages. Regression tests cover the format and the no-collision invariant. Co-Authored-By: Claude Fable 5 --- codex/synthesis.py | 28 +++++++++++++++++++++------- tests/synthesis/test_grounded.py | 2 +- tests/synthesis/test_model.py | 20 ++++++++++++++++---- 3 files changed, 38 insertions(+), 12 deletions(-) diff --git a/codex/synthesis.py b/codex/synthesis.py index dcc1586..6bfb68c 100644 --- a/codex/synthesis.py +++ b/codex/synthesis.py @@ -248,9 +248,23 @@ def _grounded_ratio(claims: list[Claim]) -> float: return n_grounded / len(claims) -def _make_lead_id(seq: int) -> str: - """Format a sequential lead id like ``L-0001``.""" - return f"L-{seq:04d}" +_KIND_PREFIX: dict[LeadKind, str] = { + "connection": "C", + "gap": "G", + "improvement": "I", + "conjecture": "X", +} + + +def _make_lead_id(seq: int, kind: LeadKind) -> str: + """Format a per-kind sequential lead id like ``L-C-0001``. + + Each synthesis stage numbers its leads from 1, so without a kind prefix a + connection, a gap, and an improvement all produce ``L-0001`` and clobber + one another in the shared ``grounded/`` directory (audit C-11). The kind + prefix namespaces the ids so they stay distinct. + """ + return f"L-{_KIND_PREFIX[kind]}-{seq:04d}" def _safe_llm_generate(llm: LLMClient, prompt: str, model: str) -> str: @@ -296,7 +310,7 @@ def _finalise_grounded_lead( return None marked = _mark_ungrounded(body, claims) return Lead( - id=_make_lead_id(seq), + id=_make_lead_id(seq, kind), kind=kind, title=title, body=marked, @@ -428,7 +442,7 @@ def find_gaps( # No coverage at all → record the gap directly (LLM not strictly needed). leads.append( Lead( - id=_make_lead_id(seq), + id=_make_lead_id(seq, "gap"), kind="gap", title=f"Gap: corpus has no material on '{topic}'", body=( @@ -481,7 +495,7 @@ def find_gaps( for bibkey in sorted(missing): leads.append( Lead( - id=_make_lead_id(seq), + id=_make_lead_id(seq, "gap"), kind="gap", title=f"Gap: code cites '{bibkey}' but corpus does not contain it", body=( @@ -658,7 +672,7 @@ def propose_conjectures( continue leads.append( Lead( - id=_make_lead_id(seq), + id=_make_lead_id(seq, "conjecture"), kind="conjecture", title=title, body=body, diff --git a/tests/synthesis/test_grounded.py b/tests/synthesis/test_grounded.py index 17e6fb8..7d27ba7 100644 --- a/tests/synthesis/test_grounded.py +++ b/tests/synthesis/test_grounded.py @@ -76,7 +76,7 @@ def test_finalise_grounded_lead_succeeds_when_grounded( ) assert lead is not None assert lead.kind == "connection" - assert lead.id == "L-0001" + assert lead.id == "L-C-0001" # kind-prefixed id (audit C-11) assert lead.confidence >= 0.5 assert lead.status == "unverified" assert lead.suggested_validation == "" diff --git a/tests/synthesis/test_model.py b/tests/synthesis/test_model.py index e45c18d..b2c991a 100644 --- a/tests/synthesis/test_model.py +++ b/tests/synthesis/test_model.py @@ -92,9 +92,21 @@ def test_lead_empty_provenance_raises() -> None: def test_lead_id_format_helper() -> None: - """The internal _make_lead_id helper produces zero-padded L-XXXX strings.""" + """_make_lead_id produces a kind-prefixed, zero-padded L--XXXX string.""" from codex.synthesis import _make_lead_id - assert _make_lead_id(1) == "L-0001" - assert _make_lead_id(42) == "L-0042" - assert _make_lead_id(9999) == "L-9999" + assert _make_lead_id(1, "connection") == "L-C-0001" + assert _make_lead_id(42, "gap") == "L-G-0042" + assert _make_lead_id(9999, "improvement") == "L-I-9999" + assert _make_lead_id(1, "conjecture") == "L-X-0001" + + +def test_lead_id_namespaced_per_kind_no_collision() -> None: + """Regression for audit C-11: each stage numbers from 1, so without a kind + prefix connection/gap/improvement all collide on L-0001 and overwrite one + another in grounded/. The prefix must keep same-seq ids distinct. + """ + from codex.synthesis import _make_lead_id + + ids = {_make_lead_id(1, k) for k in ("connection", "gap", "improvement", "conjecture")} + assert len(ids) == 4, f"same-seq ids must stay distinct across kinds, got {ids}" From eee147275a5685ace10c91c667d2f78aefb3d142 Mon Sep 17 00:00:00 2001 From: Tarik Moussa Date: Mon, 15 Jun 2026 11:12:55 +0200 Subject: [PATCH 03/24] fix(discover): resolve cited_id via shared resolver (audit C-1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit discover.py compared the raw OpenAlex cited_id against papers.id (a DOI/arXiv id), so an ingested paper cited by its OpenAlex id was mis-reported as a discovery lead. graph.py already resolved this via papers.openalex_id, but the fix was never propagated — two 'dangling' implementations disagreed. Extract the resolution into one shared constant RESOLVED_CITATIONS_SQL in graph.py; build_citation_graph and all four discover.py queries (discovery_leads / citing_papers / cited_by / cocited_papers) now go through it, so the 'what counts as ingested' rule cannot drift again. Live-DB validated: before, 13 ingested papers leaked into discovery_leads; after, the real discovery_leads() returns 0 ingested papers among its leads. Co-Authored-By: Claude Fable 5 --- codex/discover.py | 37 +++++++++++++++++++++++++++---------- codex/graph.py | 27 ++++++++++++++++++--------- 2 files changed, 45 insertions(+), 19 deletions(-) diff --git a/codex/discover.py b/codex/discover.py index 8f1e0f2..9d79987 100644 --- a/codex/discover.py +++ b/codex/discover.py @@ -1,22 +1,32 @@ -"""Discovery queries over the citation graph stored in PostgreSQL.""" +"""Discovery queries over the citation graph stored in PostgreSQL. + +Every query resolves ``citations.cited_id`` to a canonical ``papers.id`` through +the shared :data:`codex.graph.RESOLVED_CITATIONS_SQL`, so this module and the +citation-graph layer agree on what counts as "ingested". Previously this module +compared the raw OpenAlex ``cited_id`` against ``papers.id`` (a DOI/arXiv id), +which mis-reported already-ingested papers as discovery leads (audit C-1). +""" from __future__ import annotations from codex.db import get_conn +from codex.graph import RESOLVED_CITATIONS_SQL def discovery_leads(limit: int = 20) -> list[dict[str, int | str]]: """Return papers referenced by already-ingested papers but not yet collected. Returns list of dicts with keys: - cited_id (str) — arXiv ID, DOI, or OpenAlex W-ID of the target + cited_id (str) — canonical id of the referenced (not-yet-ingested) work pull (int) — how many already-ingested papers cite this target - Ordered by pull DESC, limited to `limit` rows. + Ordered by pull DESC, limited to `limit` rows. References that resolve to a + paper already in the corpus are excluded — they are not leads. """ - sql = """ + sql = f""" + WITH resolved AS ({RESOLVED_CITATIONS_SQL}) SELECT cited_id, count(*) AS pull - FROM citations + FROM resolved WHERE cited_id NOT IN (SELECT id FROM papers) GROUP BY cited_id ORDER BY pull DESC @@ -29,7 +39,10 @@ def discovery_leads(limit: int = 20) -> list[dict[str, int | str]]: def citing_papers(paper_id: str) -> list[str]: """Return IDs of all papers that cite `paper_id` (in-graph reverse citations).""" - sql = "SELECT citing_id FROM citations WHERE cited_id = %(paper_id)s" + sql = f""" + WITH resolved AS ({RESOLVED_CITATIONS_SQL}) + SELECT citing_id FROM resolved WHERE cited_id = %(paper_id)s + """ with get_conn() as conn: rows = conn.execute(sql, {"paper_id": paper_id}).fetchall() return [row["citing_id"] for row in rows] @@ -37,7 +50,10 @@ def citing_papers(paper_id: str) -> list[str]: def cited_by(paper_id: str) -> list[str]: """Return IDs of all papers that `paper_id` cites (forward citations).""" - sql = "SELECT cited_id FROM citations WHERE citing_id = %(paper_id)s" + sql = f""" + WITH resolved AS ({RESOLVED_CITATIONS_SQL}) + SELECT cited_id FROM resolved WHERE citing_id = %(paper_id)s + """ with get_conn() as conn: rows = conn.execute(sql, {"paper_id": paper_id}).fetchall() return [row["cited_id"] for row in rows] @@ -49,10 +65,11 @@ def cocited_papers(paper_id: str, limit: int = 10) -> list[dict[str, int | str]] A paper X is co-cited with `paper_id` if some paper cites both. Returns list of dicts: {paper_id: str, co_citations: int}, ordered DESC. """ - sql = """ + sql = f""" + WITH resolved AS ({RESOLVED_CITATIONS_SQL}) SELECT c2.cited_id AS paper_id, count(*) AS co_citations - FROM citations c1 - JOIN citations c2 ON c1.citing_id = c2.citing_id + FROM resolved c1 + JOIN resolved c2 ON c1.citing_id = c2.citing_id WHERE c1.cited_id = %(paper_id)s AND c2.cited_id != %(paper_id)s GROUP BY c2.cited_id diff --git a/codex/graph.py b/codex/graph.py index 981662d..540139f 100644 --- a/codex/graph.py +++ b/codex/graph.py @@ -23,6 +23,23 @@ logger = logging.getLogger(__name__) _MIN_PAGERANK_NODES = 5 +# Single source of truth for resolving ``citations.cited_id`` to a canonical +# paper id. ``cited_id`` stores OpenAlex IDs while ``papers.id`` stores DOIs / +# arXiv ids, so an in-KB reference cited by its OpenAlex id does not match +# ``papers.id`` directly. This resolves it to the canonical ``papers.id`` (via +# ``papers.openalex_id``, then the ``paper_identifiers`` alias table); dangling +# references keep their raw id. Both :func:`build_citation_graph` and +# :mod:`codex.discover` query through this so the "what counts as ingested" rule +# cannot drift between them again (audit C-1). Trusted constant — no user input. +RESOLVED_CITATIONS_SQL = """ + SELECT c.citing_id, + COALESCE(p.id, pi.paper_id, c.cited_id) AS cited_id + FROM citations c + LEFT JOIN papers p ON p.openalex_id = c.cited_id + LEFT JOIN paper_identifiers pi + ON pi.openalex_id = c.cited_id AND p.id IS NULL +""" + def build_citation_graph(conn: Any) -> nx.DiGraph: """Load the ``citations`` table into a directed graph. @@ -45,15 +62,7 @@ def build_citation_graph(conn: Any) -> nx.DiGraph: ------- nx.DiGraph with every (citing_id, cited_id) pair as an edge. """ - rows = conn.execute( - """ - SELECT c.citing_id, COALESCE(p.id, pi.paper_id, c.cited_id) AS cited_id - FROM citations c - LEFT JOIN papers p ON p.openalex_id = c.cited_id - LEFT JOIN paper_identifiers pi - ON pi.openalex_id = c.cited_id AND p.id IS NULL - """ - ).fetchall() + rows = conn.execute(RESOLVED_CITATIONS_SQL).fetchall() g: nx.DiGraph = nx.DiGraph() for row in rows: g.add_edge(row["citing_id"], row["cited_id"]) From 6193f5630d2c66b7e05c3249338a009c980c5197 Mon Sep 17 00:00:00 2001 From: Tarik Moussa Date: Mon, 15 Jun 2026 11:13:52 +0200 Subject: [PATCH 04/24] docs: fix two doc/code contradictions (audit D-2, D-4) - D-2 config.py: graph_cite_boost_alpha described the OLD inverted formula (score = dense * (1 + alpha*pr)); the code divides distance. Document the actual boosted_distance = distance / (1 + alpha*pr). - D-4 synthesis.py: write_leads claimed an 'append-only' INDEX.md that keeps previously-recorded ids; _update_index actually rebuilds from on-disk glob. Docstring now matches the implementation. Co-Authored-By: Claude Fable 5 --- codex/config.py | 3 ++- codex/synthesis.py | 9 +++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/codex/config.py b/codex/config.py index dfe27f2..a642a3d 100644 --- a/codex/config.py +++ b/codex/config.py @@ -295,7 +295,8 @@ class Settings(BaseSettings): le=1.0, description=( "Weight of the PageRank citation-boost in hybrid search. " - "Final score = dense_score * (1 + alpha * pagerank_score). " + "boosted_distance = distance / (1 + alpha * pagerank_score) — dividing " + "lowers the cosine distance of high-PageRank papers (lower = closer). " "Set to 0.0 to disable the boost without removing the flag." ), ) diff --git a/codex/synthesis.py b/codex/synthesis.py index 6bfb68c..ff86e89 100644 --- a/codex/synthesis.py +++ b/codex/synthesis.py @@ -773,10 +773,11 @@ def write_leads(leads: list[Lead], leads_dir: str) -> None: * Grounded leads → ``leads_dir/grounded/.md`` * Conjectures → ``leads_dir/conjectures/.md`` (HARD invariant) - The function ALSO maintains an append-only ``leads_dir/INDEX.md``: - a previously-recorded id is kept on subsequent runs even if it is - overwritten with a newer body. The two sections (grounded / - conjectures) are visibly separated. + The function ALSO rebuilds ``leads_dir/INDEX.md`` from the on-disk files on + every run (see :func:`_update_index`): it reflects whatever lead files + currently exist under ``grounded/`` and ``conjectures/``, listed as two + visibly separated sections. It is not append-only — a deleted lead file + drops out of the index on the next run. Notes ----- From c36e7c6ee7a66e92fcf39d2940047b5b0f0c642d Mon Sep 17 00:00:00 2001 From: Tarik Moussa Date: Mon, 15 Jun 2026 12:06:53 +0200 Subject: [PATCH 05/24] fix(ingest): canonicalise papers.id to the caller id (audit C-7, T-3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit openalex.fetch_paper fills paper.id from OpenAlex's URL-form doi/id (e.g. https://doi.org/10.48550/arxiv.math/0603097), and ingest upserted that as papers.id. Live DB confirmed 35/36 ids were URL-form — breaking bare-id lookups (graph related / discover citing / search by id) and leaving cited_id matching to rely solely on openalex_id. ingest now sets paper.id to the caller-supplied id (the arXiv id / DOI the CLI and ingest scripts use); OpenAlex's work id is retained as openalex_id and in paper_identifiers. Regression test: fetch_paper returns a URL-form id, papers.id upsert uses the bare caller id. T-3: the OpenAlex test fixture used a bare 'doi' (not the URL form the API emits) and never asserted paper.id, which masked this. Fixture now uses the URL form and the test pins fetch_paper's mapping. NOTE: existing rows keep their URL-form ids until re-ingested — run ingest_all.sh to migrate the 36-paper corpus (chosen migration path). Co-Authored-By: Claude Fable 5 --- codex/ingest.py | 8 ++++++++ tests/ingest/test_ingest.py | 31 +++++++++++++++++++++++++++++++ tests/sources/test_openalex.py | 7 ++++++- 3 files changed, 45 insertions(+), 1 deletion(-) diff --git a/codex/ingest.py b/codex/ingest.py index b63799d..b43fdd7 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) diff --git a/tests/ingest/test_ingest.py b/tests/ingest/test_ingest.py index 80e4d23..638a1ec 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 # --------------------------------------------------------------------------- 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: From 8e2f9e037dac5dfa3c59d13fa42c78dee906fd55 Mon Sep 17 00:00:00 2001 From: Tarik Moussa Date: Mon, 15 Jun 2026 12:09:38 +0200 Subject: [PATCH 06/24] 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.""" From ce75bc02e59dcc3b25ee50c21b18c700bdd100d6 Mon Sep 17 00:00:00 2001 From: Tarik Moussa Date: Mon, 15 Jun 2026 12:10:46 +0200 Subject: [PATCH 07/24] =?UTF-8?q?docs(adr):=20audit=20addendum=20to=20ADR-?= =?UTF-8?q?F15=20=E2=80=94=20ID=20mismatch=20resolved=20(audit=20D-1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ADR's GO rested on a live-corpus spike (478 dangling, Bobenko/Springborn rank 7, 5.7% spread) measured before the resolution + canonicalisation landed. Add an audit addendum: mark the 'Known Limitation: ID-Format Mismatch' as resolved (C-1 shared resolver + C-7 canonical ids), and flag the Spike Result numbers as pending re-measurement after the C-7 re-ingest. History preserved. Co-Authored-By: Claude Fable 5 --- docs/adr/ADR-F15-literature-graph.md | 36 ++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) 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. From 1ff8052b6998bfe0189c48d50ddac265100a0a9b Mon Sep 17 00:00:00 2001 From: Tarik Moussa Date: Mon, 15 Jun 2026 12:17:17 +0200 Subject: [PATCH 08/24] chore(infra): add canonical-id re-ingest migration helper (audit C-7) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One-time, guarded migration for the C-7 id-format change: a plain re-ingest would duplicate every paper (new bare-id row beside the old URL-id PK), so this wipes (TRUNCATE papers CASCADE) and rebuilds via ingest_all.sh. Safety: requires the SSH tunnel, prints a BEFORE snapshot, gates the TRUNCATE behind an explicit 'MIGRATE' confirmation, then prints AFTER verification — C-7 (url_form_ids should be 0) and C-1 via the real resolver-based discovery_leads() (ingested papers leaked should be 0). Uses .venv psycopg (psql is not installed); DATABASE_URL is sourced, never echoed. Joins PR #13 (Wave 2). Read-only verification SQL validated against the live DB. Co-Authored-By: Claude Fable 5 --- infra/reingest_canonical_ids.sh | 110 ++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+) create mode 100755 infra/reingest_canonical_ids.sh diff --git a/infra/reingest_canonical_ids.sh b/infra/reingest_canonical_ids.sh new file mode 100755 index 0000000..f106cdc --- /dev/null +++ b/infra/reingest_canonical_ids.sh @@ -0,0 +1,110 @@ +#!/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" + +# ── 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)." From 9f172f5c68fc8cf4fe5919fa5b758328835a7f3a Mon Sep 17 00:00:00 2001 From: Tarik Moussa Date: Mon, 15 Jun 2026 15:37:35 +0200 Subject: [PATCH 09/24] fix(infra): preflight schema check before TRUNCATE in migration helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The helper TRUNCATEd papers CASCADE and only then ran ingest_all.sh, which failed on the missing chunks.section column (audit M-1) — leaving the corpus wiped. Add a preflight that verifies chunks.section exists BEFORE the destructive step and aborts early (no TRUNCATE) with the owner-level ALTER to run, since the app user cannot alter postgres-owned tables. Co-Authored-By: Claude Fable 5 --- infra/reingest_canonical_ids.sh | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/infra/reingest_canonical_ids.sh b/infra/reingest_canonical_ids.sh index f106cdc..600ade3 100755 --- a/infra/reingest_canonical_ids.sh +++ b/infra/reingest_canonical_ids.sh @@ -58,6 +58,35 @@ run_sql "SELECT count(*) AS papers, count(*) FILTER (WHERE starts_with(id, 'http 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/" From cdb7d254df8ad43306b71ca6de0e8fb29cbb987f Mon Sep 17 00:00:00 2001 From: Tarik Moussa Date: Mon, 15 Jun 2026 15:53:25 +0200 Subject: [PATCH 10/24] feat(db): idempotent schema + 'codex migrate' command (audit M-1, T-2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit M-1: schema.sql could not be re-applied (leading CREATE TABLE/INDEX lacked IF NOT EXISTS), apply_schema was never called, and the live migration just failed on a missing chunks.section column. Fixes: - schema.sql: all CREATE TABLE/INDEX now use IF NOT EXISTS — the whole file is re-applyable as a no-op. - codex migrate: new CLI command that applies schema.sql. Connects via MIGRATION_DATABASE_URL (falls back to DATABASE_URL) and catches InsufficientPrivilege with guidance — because the app role is DML-only and core tables are owned by 'postgres' (the privilege dimension found during the live migration incident). - config: migration_database_url (optional privileged connection). - db: apply_schema docstring corrected (idempotency now true) + privilege note. - T-2: static test that schema.sql is fully idempotent; migrate privilege-error test. Co-Authored-By: Claude Fable 5 --- codex/cli.py | 31 +++++++++++++++++++++++++++++++ codex/config.py | 11 +++++++++++ codex/db.py | 11 ++++++++--- infra/schema.sql | 22 +++++++++++----------- tests/scaffold/test_db.py | 37 +++++++++++++++++++++++++++++++++++++ 5 files changed, 98 insertions(+), 14 deletions(-) diff --git a/codex/cli.py b/codex/cli.py index 33ab2c8..3a46872 100644 --- a/codex/cli.py +++ b/codex/cli.py @@ -29,6 +29,37 @@ app.add_typer(graph_app, name="graph") app.add_typer(search_app, name="search") +@app.command() +def migrate() -> None: + """Apply infra/schema.sql to bring the database schema up to date (idempotent). + + Must connect as a role that can run DDL (owns / can CREATE + ALTER the tables). + The application's DATABASE_URL is usually a least-privilege DML role and will + fail with InsufficientPrivilege; set MIGRATION_DATABASE_URL to a privileged + (owner/superuser) connection (audit M-1). + """ + import psycopg + import psycopg.rows + + from codex.config import get_settings + from codex.db import apply_schema + + settings = get_settings() + url = settings.migration_database_url or settings.database_url + try: + with psycopg.connect(url, row_factory=psycopg.rows.dict_row) as conn: + apply_schema(conn) + except psycopg.errors.InsufficientPrivilege as exc: + typer.echo( + "ERROR: schema migration needs a role that owns the tables " + "(CREATE/ALTER). Set MIGRATION_DATABASE_URL to a privileged connection " + f"and retry. ({exc})", + err=True, + ) + raise typer.Exit(1) from exc + typer.echo("Schema applied — database is up to date.") + + @app.command() def ingest( paper_id: str = typer.Argument(..., help="arXiv ID, DOI, or OpenAlex W-ID"), diff --git a/codex/config.py b/codex/config.py index a642a3d..faf79ad 100644 --- a/codex/config.py +++ b/codex/config.py @@ -34,6 +34,17 @@ class Settings(BaseSettings): ), ) + migration_database_url: str | None = Field( + default=None, + description=( + "Optional privileged connection string used by `codex migrate` to apply " + "schema DDL. The app's database_url is typically a least-privilege DML " + "role that cannot CREATE/ALTER owner-held tables (raises " + "InsufficientPrivilege); point this at an owner/superuser connection. " + "Falls back to database_url when unset." + ), + ) + # ------------------------------------------------------------------ # External services # ------------------------------------------------------------------ diff --git a/codex/db.py b/codex/db.py index e86678f..f8272b9 100644 --- a/codex/db.py +++ b/codex/db.py @@ -45,11 +45,16 @@ def get_conn() -> Generator[psycopg.Connection[psycopg.rows.DictRow], None, None def apply_schema(conn: psycopg.Connection[psycopg.rows.DictRow]) -> None: """Idempotently apply ``infra/schema.sql`` to the connected database. - Safe to call on every startup — all DDL statements use - ``CREATE … IF NOT EXISTS``. + Safe to call repeatedly — every statement is ``CREATE … IF NOT EXISTS`` or + ``ADD COLUMN IF NOT EXISTS``, so re-application is a no-op. + + Requires a connection whose role can run DDL (owns / can CREATE + ALTER the + tables). The least-privilege application role is typically DML-only and will + raise ``InsufficientPrivilege``; use a privileged connection — see + ``codex migrate`` and ``MIGRATION_DATABASE_URL`` (audit M-1). Args: - conn: An open psycopg connection (obtained via :func:`get_conn`). + conn: An open psycopg connection with DDL privileges. """ schema_path = Path(__file__).parent.parent / "infra" / "schema.sql" sql = schema_path.read_text(encoding="utf-8") diff --git a/infra/schema.sql b/infra/schema.sql index 514ea32..03ad279 100644 --- a/infra/schema.sql +++ b/infra/schema.sql @@ -13,7 +13,7 @@ CREATE EXTENSION IF NOT EXISTS vector; -- --------------------------------------------------------------------- -- Layer 1: Papers + full-text chunks -- --------------------------------------------------------------------- -CREATE TABLE papers ( +CREATE TABLE IF NOT EXISTS papers ( id TEXT PRIMARY KEY, -- canonical ID: arXiv ID or DOI openalex_id TEXT UNIQUE, -- e.g. W2741809807 bibkey TEXT UNIQUE, -- BibTeX key -> .bib + Doxygen @cite @@ -27,10 +27,10 @@ CREATE TABLE papers ( ); -- HNSW index for fast approximate nearest-neighbour search on abstracts. -CREATE INDEX papers_abstract_emb_idx +CREATE INDEX IF NOT EXISTS papers_abstract_emb_idx ON papers USING hnsw (abstract_emb vector_cosine_ops); -CREATE TABLE chunks ( +CREATE TABLE IF NOT EXISTS chunks ( id BIGSERIAL PRIMARY KEY, paper_id TEXT REFERENCES papers(id) ON DELETE CASCADE, ord INT NOT NULL, -- position within the paper @@ -38,13 +38,13 @@ CREATE TABLE chunks ( embedding vector(1024) ); -CREATE INDEX chunks_emb_idx +CREATE INDEX IF NOT EXISTS chunks_emb_idx ON chunks USING hnsw (embedding vector_cosine_ops); -CREATE INDEX chunks_paper_idx ON chunks (paper_id); +CREATE INDEX IF NOT EXISTS chunks_paper_idx ON chunks (paper_id); -- Sparse / keyword hits for exact mathematical terminology (hybrid search). -- Dense (above) + full-text (below) combined = robust against math terms. -CREATE INDEX chunks_fts_idx +CREATE INDEX IF NOT EXISTS chunks_fts_idx ON chunks USING gin (to_tsvector('english', content)); -- --------------------------------------------------------------------- @@ -53,19 +53,19 @@ CREATE INDEX chunks_fts_idx -- edges to not-yet-ingested papers are intentionally preserved — -- those are your discovery leads. -- --------------------------------------------------------------------- -CREATE TABLE citations ( +CREATE TABLE IF NOT EXISTS citations ( citing_id TEXT REFERENCES papers(id) ON DELETE CASCADE, cited_id TEXT NOT NULL, -- arXiv/DOI/OpenAlex ID of the target context TEXT, -- optional: citation context (S2) PRIMARY KEY (citing_id, cited_id) ); -CREATE INDEX citations_cited_idx ON citations (cited_id); +CREATE INDEX IF NOT EXISTS citations_cited_idx ON citations (cited_id); -- --------------------------------------------------------------------- -- Layer 3: Provenance (the "cleanly couple" goal) -- --------------------------------------------------------------------- -CREATE TABLE code_links ( +CREATE TABLE IF NOT EXISTS code_links ( id BIGSERIAL PRIMARY KEY, symbol TEXT NOT NULL, -- e.g. 'dec::hodge_star' or 'file.cpp:120' paper_id TEXT REFERENCES papers(id) ON DELETE SET NULL, @@ -74,8 +74,8 @@ CREATE TABLE code_links ( added_at TIMESTAMPTZ DEFAULT now() ); -CREATE INDEX code_links_symbol_idx ON code_links (symbol); -CREATE INDEX code_links_paper_idx ON code_links (paper_id); +CREATE INDEX IF NOT EXISTS code_links_symbol_idx ON code_links (symbol); +CREATE INDEX IF NOT EXISTS code_links_paper_idx ON code_links (paper_id); -- --------------------------------------------------------------------- -- Example query: discovery leads diff --git a/tests/scaffold/test_db.py b/tests/scaffold/test_db.py index a91e09d..4d7a991 100644 --- a/tests/scaffold/test_db.py +++ b/tests/scaffold/test_db.py @@ -33,3 +33,40 @@ def test_apply_schema_executes_sql() -> None: mock_conn.execute.assert_called_once() mock_conn.commit.assert_called_once() + + +def test_schema_sql_is_idempotent() -> None: + """Every CREATE TABLE/INDEX in infra/schema.sql uses IF NOT EXISTS (audit M-1, T-2). + + Re-applying schema.sql must be a safe no-op so the migration can run + repeatedly against an existing database. + """ + import re + from pathlib import Path + + schema = (Path(__file__).resolve().parents[2] / "infra" / "schema.sql").read_text() + # Drop line comments so commented-out example queries don't count. + code = "\n".join(line.split("--", 1)[0] for line in schema.splitlines()) + non_idempotent = re.findall( + r"CREATE\s+(?:UNIQUE\s+)?(?:TABLE|INDEX)\s+(?!IF\s+NOT\s+EXISTS)(\w+)", + code, + re.IGNORECASE, + ) + assert not non_idempotent, f"schema.sql has non-idempotent CREATE statements: {non_idempotent}" + + +def test_migrate_command_reports_privilege_error() -> None: + """`codex migrate` surfaces InsufficientPrivilege with guidance and exits 1 (M-1).""" + import psycopg + from typer.testing import CliRunner + + from codex.cli import app + + def boom(*args: object, **kwargs: object) -> object: + raise psycopg.errors.InsufficientPrivilege("must be owner of table chunks") + + with patch("psycopg.connect", boom): + result = CliRunner().invoke(app, ["migrate"]) + + assert result.exit_code == 1 + assert "MIGRATION_DATABASE_URL" in result.output From 10ff5ba3ab7ec9811164cb9f93414f8cbd43c771 Mon Sep 17 00:00:00 2001 From: Tarik Moussa Date: Mon, 15 Jun 2026 15:54:25 +0200 Subject: [PATCH 11/24] chore(infra): point migration helper preflight at 'codex migrate' Now that 'codex migrate' applies the idempotent schema via a privileged role, recommend it (with MIGRATION_DATABASE_URL) as the primary schema-sync fix in the preflight abort message, keeping the manual owner-ALTER as a fallback. Co-Authored-By: Claude Fable 5 --- infra/reingest_canonical_ids.sh | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/infra/reingest_canonical_ids.sh b/infra/reingest_canonical_ids.sh index 600ade3..213862c 100755 --- a/infra/reingest_canonical_ids.sh +++ b/infra/reingest_canonical_ids.sh @@ -76,11 +76,13 @@ 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 "The app user cannot add it (core tables are owned by 'postgres'). Sync the" + echo "schema with a privileged role, then re-run this script. Either:" 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 " (a) MIGRATION_DATABASE_URL=postgresql://postgres:PASS@HOST:PORT/papers \\" + echo " \"$CODEX_DIR/.venv/bin/codex\" migrate # applies schema.sql idempotently" + echo " (b) sudo -u postgres psql -d papers \\" + echo " -c \"ALTER TABLE chunks ADD COLUMN IF NOT EXISTS section TEXT;\"" echo "" echo "Nothing was changed — no TRUNCATE was issued." exit 1 From ae5e9a528bb27c7878f0669632fd8d33d50a99fb Mon Sep 17 00:00:00 2001 From: Tarik Moussa Date: Mon, 15 Jun 2026 16:20:54 +0200 Subject: [PATCH 12/24] docs: document 'codex migrate' + MIGRATION_DATABASE_URL as the schema-sync path README step 4 used an inline apply_schema over the app DATABASE_URL, which fails under the privilege model (least-privilege app role cannot run DDL). Replace it with 'codex migrate' and document MIGRATION_DATABASE_URL (owner/superuser) in the README env table and .env.example, so the privileged migrate path is the standard for future schema upgrades. Co-Authored-By: Claude Fable 5 --- .env.example | 6 ++++++ README.md | 15 +++++++-------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/.env.example b/.env.example index f7b60ed..8bd512a 100644 --- a/.env.example +++ b/.env.example @@ -5,6 +5,12 @@ # Example: postgresql://researcher:change_me@localhost:5432/papers DATABASE_URL=postgresql://researcher:change_me@localhost:5432/papers +# Optional: privileged connection used by `codex migrate` to apply schema DDL. +# The app DATABASE_URL is often a least-privilege DML role that cannot CREATE/ +# ALTER owner-held tables; point this at an owner/superuser connection. +# Falls back to DATABASE_URL when unset. +# MIGRATION_DATABASE_URL=postgresql://postgres:change_me@localhost:5432/papers + # GROBID service base URL (containerised — see infra/docker-compose.yml) GROBID_URL=http://localhost:8070 diff --git a/README.md b/README.md index a68202e..b794cd0 100644 --- a/README.md +++ b/README.md @@ -19,13 +19,11 @@ $EDITOR .env # 3. Install Python dependencies (requires uv) uv sync -# 4. Apply the database schema (first run only) -uv run python -c " -from codex.db import get_conn, apply_schema -with get_conn() as conn: - apply_schema(conn) -print('Schema applied.') -" +# 4. Apply the database schema (idempotent — safe to re-run after upgrades) +# Needs a role that can run DDL. If DATABASE_URL is a least-privilege app +# role, point MIGRATION_DATABASE_URL at an owner/superuser connection first: +# MIGRATION_DATABASE_URL=postgresql://postgres:...@host:5432/papers +uv run codex migrate ``` --- @@ -34,7 +32,8 @@ print('Schema applied.') | Variable | Default | Description | |---|---|---| -| `DATABASE_URL` | `postgresql://researcher:change_me@localhost:5432/papers` | libpq connection string | +| `DATABASE_URL` | `postgresql://researcher:change_me@localhost:5432/papers` | libpq connection string (app role; DML) | +| `MIGRATION_DATABASE_URL` | *(falls back to `DATABASE_URL`)* | Privileged connection used by `codex migrate` for schema DDL (owner/superuser) | | `GROBID_URL` | `http://localhost:8070` | GROBID HTTP API base URL | | `OLLAMA_BASE_URL` | `http://localhost:11434` | Local Ollama endpoint (optional) | | `EMBEDDING_MODEL` | `BAAI/bge-m3` | sentence-transformers model name | From cfbfa533983328c532f0b08d84e1128202daa1bb Mon Sep 17 00:00:00 2001 From: Tarik Moussa Date: Mon, 15 Jun 2026 16:24:22 +0200 Subject: [PATCH 13/24] fix(wiki): protect math in cross-refs; drop no-op DB call; fix mark order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - R-9 (MED): _inject_cross_refs now protects LaTeX math spans ($…$, $$…$$, \(…\), \[…\]) like inline code, so a concept name inside a formula is no longer rewritten to a [[slug]] link and corrupting the LaTeX. Regression test added. - C-13 (LOW): mark ungrounded claims on the raw body BEFORE injecting cross-refs (render then inject), so a concept name inside a claim cannot stop the ⚠ regex from matching. - C-12 (LOW): _try_embed_formulas is now a true no-op — it previously issued a per-concept information_schema round-trip that did nothing. Co-Authored-By: Claude Fable 5 --- codex/wiki.py | 44 ++++++++++++++++++++------------------ tests/wiki/test_compile.py | 13 +++++++++++ 2 files changed, 36 insertions(+), 21 deletions(-) diff --git a/codex/wiki.py b/codex/wiki.py index b913258..e66024f 100644 --- a/codex/wiki.py +++ b/codex/wiki.py @@ -402,6 +402,13 @@ def _detect_conflicts( _INLINE_CODE_RE = re.compile(r"`[^`]+`") +# LaTeX math spans — protected from cross-ref injection so a concept name inside +# a formula is not rewritten to a [[slug]] link, corrupting the LaTeX (audit R-9). +_MATH_SPAN_RE = re.compile( + r"\$\$.*?\$\$|\$[^$\n]*?\$|\\\(.*?\\\)|\\\[.*?\\\]", + re.DOTALL, +) + def _inject_cross_refs( markdown: str, @@ -411,11 +418,12 @@ def _inject_cross_refs( """Replace occurrences of other concept titles/aliases with ``[[slug]]`` links. Only exact case-insensitive whole-word matches outside of existing - ``[[…]]`` blocks or inline code spans are replaced. - Inline-code spans (`` `…` ``) are temporarily protected by null-byte - placeholders and restored after injection. + ``[[…]]`` blocks, inline code spans, or LaTeX math spans are replaced. + Inline-code (`` `…` ``) and math (``$…$``, ``$$…$$``, ``\\(…\\)``, + ``\\[…\\]``) spans are temporarily protected by null-byte placeholders and + restored after injection. """ - # Step 1: protect inline-code spans from replacement + # Step 1: protect inline-code and math spans from replacement placeholders: dict[str, str] = {} def _protect(m: re.Match[str]) -> str: @@ -424,6 +432,7 @@ def _inject_cross_refs( return key markdown = _INLINE_CODE_RE.sub(_protect, markdown) + markdown = _MATH_SPAN_RE.sub(_protect, markdown) # don't rewrite names inside LaTeX (R-9) # Step 2: inject cross-refs on unprotected text for concept in all_concepts: @@ -602,14 +611,14 @@ def compile_concept( claims = _parse_claims(raw_output) claims = _run_grounding_guard(claims, chunks) - _all_concepts = all_concepts or [] - raw_output = _inject_cross_refs(raw_output, _all_concepts, concept.slug) - - # Graceful: try to embed formula chunks (F-09) — skip if table missing + # Graceful: formula-embedding hook (F-09) — currently a no-op (audit C-12). _try_embed_formulas(concept, chunks) compiled_at = datetime.now(UTC) + # Mark ungrounded claims on the raw body FIRST, then inject cross-refs: a + # concept name inside a claim must not stop the ⚠ regex from matching (C-13). markdown = _render_page_markdown(concept, raw_output, claims, compiled_at) + markdown = _inject_cross_refs(markdown, all_concepts or [], concept.slug) return ConceptPage( concept=concept, @@ -621,20 +630,13 @@ def compile_concept( def _try_embed_formulas(concept: Concept, chunks: list[dict[str, Any]]) -> None: - """Attempt to look up formula chunks for the concept — graceful no-op if F-09 absent.""" - try: - from codex.db import get_conn + """Reserved hook for embedding F-09 formula chunks into a page. - with get_conn() as conn: - # Check if formulas table exists - row = conn.execute( - "SELECT 1 FROM information_schema.tables WHERE table_name = 'formulas'" - ).fetchone() - if row is None: - return # F-09 not present - # (Future: embed relevant raw_latex into the page) - except Exception: # noqa: BLE001 - return # DB not reachable or other error — degrade gracefully + Currently a no-op — formula embedding is not implemented yet. Kept as a stable + seam for compile_concept's call site. The previous body issued a per-concept + DB round-trip (information_schema lookup) that did nothing (audit C-12). + """ + return # --------------------------------------------------------------------------- diff --git a/tests/wiki/test_compile.py b/tests/wiki/test_compile.py index 6835ec0..632905c 100644 --- a/tests/wiki/test_compile.py +++ b/tests/wiki/test_compile.py @@ -274,6 +274,19 @@ def test_inject_cross_refs_skips_inline_code() -> None: assert "[[circle-packing]]" in result # bare occurrence replaced +def test_inject_cross_refs_skips_math_spans() -> None: + """Concept names inside LaTeX math are NOT rewritten to [[slug]] links (audit R-9).""" + concepts = [ + Concept(slug="energy", title="Energy", aliases=[]), + FIXTURE_CONCEPT, + ] + text = "Prose Energy here. Math $x = Energy^2$ and inline \\(Energy\\) stay literal." + result = _inject_cross_refs(text, concepts, current_slug="lobachevsky-function") + assert "$x = Energy^2$" in result # display/inline $...$ math untouched + assert "\\(Energy\\)" in result # \(...\) math untouched + assert "Prose [[energy]] here." in result # prose occurrence still linked + + def test_inject_cross_refs_full_list_even_when_single_concept() -> None: """Cross-ref injection uses the full concept list, not just the compiled concept.""" concepts = [ From c4106b0f51ceabdfc1913a5fc5ccb15c6d40f835 Mon Sep 17 00:00:00 2001 From: Tarik Moussa Date: Mon, 15 Jun 2026 16:32:01 +0200 Subject: [PATCH 14/24] fix(synthesis): make coverage-map gaps opt-in (audit R-8) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit find_gaps defaulted its probe topics to paper titles, so a title — which retrieves mostly its own single bibkey — fell below synthesis_gap_min_coverage and flagged almost every paper as a 'gap'. Coverage-map gaps now run only for explicitly-requested topics; the CLI gains a repeatable --topic option. The code-vs-corpus gap path (precise) is unchanged and still default. Tests: the two cases that relied on default-title coverage gaps now pass explicit topics; added a regression that no coverage gaps appear without --topic. Co-Authored-By: Claude Fable 5 --- codex/cli.py | 12 ++++++++++-- codex/synthesis.py | 15 ++++++++------- tests/synthesis/test_gaps.py | 27 ++++++++++++++++++++++++++- tests/synthesis/test_grounded.py | 3 ++- 4 files changed, 46 insertions(+), 11 deletions(-) diff --git a/codex/cli.py b/codex/cli.py index 3a46872..4ba4152 100644 --- a/codex/cli.py +++ b/codex/cli.py @@ -422,7 +422,15 @@ def synthesis_leads( "--lib-path", help=( "Path to a C++ source tree. Enables improvement leads (@cite-aware) and " - "code-vs-corpus gap detection. Without it only connection + topic gaps run." + "code-vs-corpus gap detection." + ), + ), + topic: list[str] | None = typer.Option( # noqa: B008 + None, + "--topic", + help=( + "Topic to check for coverage gaps (repeatable). Opt-in: without --topic " + "no coverage-map gaps run (avoids per-paper false positives, audit R-8)." ), ), output_dir: Optional[str] = typer.Option( # noqa: UP045 @@ -444,7 +452,7 @@ def synthesis_leads( llm = default_llm_client() connections = find_connections(top_k=settings.synthesis_top_k, llm=llm) - gaps = find_gaps(lib_path=lib_path, llm=llm) + gaps = find_gaps(lib_path=lib_path, llm=llm, topics=topic or None) improvements = ( find_improvements(lib_path, top_k=settings.synthesis_top_k, llm=llm) if lib_path else [] ) diff --git a/codex/synthesis.py b/codex/synthesis.py index ff86e89..d04481d 100644 --- a/codex/synthesis.py +++ b/codex/synthesis.py @@ -416,11 +416,12 @@ def find_gaps( Two complementary signals are used: - * **Topic coverage map** — for each topic in ``topics`` (or, if - ``None``, the union of paper titles), retrieve chunks and check - bibkey coverage. A topic covered by < ``synthesis_gap_min_coverage`` - bibkeys is reported as a gap candidate and the LLM is asked to - articulate it. + * **Topic coverage map** — for each **explicitly-requested** topic in + ``topics``, retrieve chunks and check bibkey coverage. A topic covered by + < ``synthesis_gap_min_coverage`` bibkeys is a gap candidate and the LLM is + asked to articulate it. Defaulting to paper titles flagged almost every + paper as a gap (a title retrieves mostly its own single bibkey), so the + coverage map is opt-in now; pass ``topics`` / CLI ``--topic`` (audit R-8). * **Code-vs-corpus gap** — when ``lib_path`` is given, any @cite bibkey scanned from code that is NOT present in the corpus is flagged. This catches the "cited in code, never ingested" case. @@ -429,9 +430,9 @@ def find_gaps( seq = 1 leads: list[Lead] = [] + # Coverage-map probes are the explicit topics only — no default-to-titles, + # which turned every paper into a spurious gap (audit R-8). probe_topics: list[str] = list(topics or []) - if not probe_topics: - probe_topics = [title for _, title in _list_paper_titles(db_conn)] known_bibkeys = {bk for bk, _ in _list_paper_titles(db_conn)} diff --git a/tests/synthesis/test_gaps.py b/tests/synthesis/test_gaps.py index 61e54bf..aa4007a 100644 --- a/tests/synthesis/test_gaps.py +++ b/tests/synthesis/test_gaps.py @@ -112,10 +112,35 @@ def test_find_gaps_coverage_map_low_coverage_triggers_llm( "For an ideal tetrahedron with dihedral angles the hyperbolic volume is " "V = L(gamma1) + L(gamma2) + L(gamma3). [Springborn2008 #chunk 16]\n" ) - leads = find_gaps(llm=StubLLM(grounded_body)) + # Coverage gaps are opt-in (audit R-8): pass the topic explicitly. + leads = find_gaps(llm=StubLLM(grounded_body), topics=["hyperbolic volume formula"]) assert any(lead.kind == "gap" for lead in leads) +def test_find_gaps_no_coverage_gaps_without_explicit_topics( + monkeypatch: pytest.MonkeyPatch, + mock_paper_titles: None, + mock_settings: Path, +) -> None: + """Without explicit topics, paper titles do NOT auto-generate coverage gaps (R-8).""" + sparse_chunks: list[dict[str, Any]] = [ + { + "id": 10, + "paper_id": "springborn-2008", + "ord": 16, + "content": "the hyperbolic volume is V = L(gamma1) + L(gamma2) + L(gamma3)", + "bibkey": "Springborn2008", + }, + ] + monkeypatch.setattr( + "codex.synthesis._retrieve_chunks", + lambda queries, top_k: [dict(c) for c in sparse_chunks], + ) + # No lib_path, no topics → the coverage map must not run despite sparse coverage. + leads = find_gaps(llm=StubLLM("a gap. [Springborn2008 #chunk 16]")) + assert leads == [] + + def test_find_gaps_explicit_topics_override_default( monkeypatch: pytest.MonkeyPatch, mock_paper_titles: None, diff --git a/tests/synthesis/test_grounded.py b/tests/synthesis/test_grounded.py index 7d27ba7..59ac7ef 100644 --- a/tests/synthesis/test_grounded.py +++ b/tests/synthesis/test_grounded.py @@ -148,7 +148,8 @@ def test_find_gaps_topic_with_no_chunks( ) -> None: """When retrieval returns no chunks, a gap lead is emitted directly.""" monkeypatch.setattr("codex.synthesis._retrieve_chunks", lambda queries, top_k: []) - leads = find_gaps(llm=StubLLM("")) + # Coverage gaps are opt-in now (audit R-8): pass an explicit topic. + leads = find_gaps(llm=StubLLM(""), topics=["nonexistent topic"]) assert len(leads) >= 1 assert all(lead.kind == "gap" for lead in leads) # Direct gap leads carry a validation hint (re-ingest) From 77f9d79da092e7029ef6e72076bfc6cf61e9012c Mon Sep 17 00:00:00 2001 From: Tarik Moussa Date: Mon, 15 Jun 2026 16:34:28 +0200 Subject: [PATCH 15/24] fix(mcp): real cited bibkeys in wiki_read + constant-time token compare - C-3 (MED): wiki_read returned 'sources' = [concept_slug] (a placeholder), not the cited sources. Now parses the page's inline [BibKey #loc] citations and returns the actual set of cited bibkeys. - S-2 (LOW): _TokenAuth compared the Bearer header with != (length/equality timing side-channel). Use secrets.compare_digest for constant-time comparison. Co-Authored-By: Claude Fable 5 --- codex/mcp_server.py | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/codex/mcp_server.py b/codex/mcp_server.py index 1caf294..5a82b08 100644 --- a/codex/mcp_server.py +++ b/codex/mcp_server.py @@ -24,6 +24,7 @@ Transport from __future__ import annotations import logging +import secrets from mcp.server.fastmcp import FastMCP from mcp.server.transport_security import TransportSecuritySettings @@ -121,7 +122,6 @@ def wiki_read(concept_slug: str) -> dict[str, object]: directory or the requested page does not exist. """ try: - import json from pathlib import Path from codex.config import get_settings @@ -135,16 +135,12 @@ def wiki_read(concept_slug: str) -> dict[str, object]: markdown = page_path.read_text(encoding="utf-8") - # Collect cited bibkeys from the compile state - state_path = wiki_dir / ".compile-state.json" - sources: list[str] = [] - if state_path.exists(): - try: - state: dict[str, str] = json.loads(state_path.read_text(encoding="utf-8")) - if concept_slug in state: - sources = [concept_slug] - except (json.JSONDecodeError, OSError): - pass + # Collect the actual cited bibkeys from the page's inline [BibKey #loc] + # citations (audit C-3: this previously returned the concept slug itself, + # not the cited sources). + from codex.wiki import _parse_claims + + sources = sorted({claim.bibkey for claim in _parse_claims(markdown)}) return {"markdown": markdown, "sources": sources} except Exception as exc: # noqa: BLE001 @@ -337,7 +333,9 @@ def main() -> None: self, request: Request, call_next: RequestResponseEndpoint ) -> Response: auth = request.headers.get("Authorization", "") - if auth != f"Bearer {token}": + # Constant-time comparison to avoid a token-length/equality timing + # side-channel (audit S-2). + if not secrets.compare_digest(auth, f"Bearer {token}"): return Response("Unauthorized", status_code=401) return await call_next(request) From e7b854db10ae36899cde4923f0c72c39979608d3 Mon Sep 17 00:00:00 2001 From: Tarik Moussa Date: Mon, 15 Jun 2026 16:37:30 +0200 Subject: [PATCH 16/24] fix(wiki): make grounding tokenizer robust to punctuation (audit R-1) The grounding guard tokenized with .lower().split() and split sentences on bare .!?, so faithful claims were wrongly marked ungrounded: 'volume,' != 'volume', and a decimal like 'V = 1.5' split into a 1-word fragment '5' that fell below the 5-content-word floor. - _content_words now strips edge sentence-punctuation (.,;:!?"') from tokens while preserving math delimiters ()=+; claim and chunk are normalised identically so matching stays consistent. - _SENTENCE_SPLIT_RE splits on .!? only when followed by whitespace/end, so decimals no longer create spurious sentence boundaries. Regression test: a 5-word claim with a trailing comma now grounds. Co-Authored-By: Claude Fable 5 --- codex/wiki.py | 20 +++++++++++++++++--- tests/wiki/test_compile.py | 17 +++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/codex/wiki.py b/codex/wiki.py index e66024f..7a0ec8b 100644 --- a/codex/wiki.py +++ b/codex/wiki.py @@ -273,7 +273,13 @@ _STOPWORDS: frozenset[str] = frozenset( } ) -_SENTENCE_SPLIT_RE = re.compile(r"[.!?]") +# Split on sentence-ending punctuation only when followed by whitespace/end, so a +# decimal like "1.5" does not create a spurious sentence boundary (audit R-1). +_SENTENCE_SPLIT_RE = re.compile(r"[.!?]+(?=\s|$)") + +# Punctuation stripped from token edges before grounding comparison — sentence/ +# clause marks and quotes, NOT math delimiters like ()=+ (audit R-1). +_EDGE_PUNCT = ".,;:!?\"'" def _last_sentence(text: str) -> str: @@ -292,8 +298,16 @@ def _last_sentence(text: str) -> str: def _content_words(text: str) -> list[str]: - """Return lowercased tokens with stopwords removed.""" - return [w for w in text.lower().split() if w not in _STOPWORDS] + """Return lowercased content tokens, edge-punctuation-stripped, stopwords removed. + + Stripping leading/trailing sentence punctuation makes grounding robust to + surface variation ("volume," vs "volume") so a faithful paraphrase is not + marked ungrounded over a comma (audit R-1). Math delimiters (``()=+``) are + preserved. Claim and chunk tokens are normalised identically, so matching + stays consistent. + """ + tokens = (w.strip(_EDGE_PUNCT) for w in text.lower().split()) + return [w for w in tokens if w and w not in _STOPWORDS] def _run_grounding_guard( diff --git a/tests/wiki/test_compile.py b/tests/wiki/test_compile.py index 632905c..c0db957 100644 --- a/tests/wiki/test_compile.py +++ b/tests/wiki/test_compile.py @@ -213,6 +213,23 @@ def test_grounding_guard_last_sentence_grounded_earlier_hallucination_ignored() ) +def test_grounding_guard_robust_to_trailing_comma() -> None: + """A faithful 5-word claim grounds despite a trailing comma (audit R-1). + + The chunk says "... the volume function V0 is strictly concave on the angle + domain." Before edge-punctuation stripping, the claim token "concave," != the + chunk token "concave", so the only content-5-gram failed to match and the + claim was wrongly marked ungrounded. + """ + claim = Claim( + text="the volume function V0 is strictly concave,", + bibkey="Springborn2008", + locator="chunk 16", + ) + result = _run_grounding_guard([claim], FIXTURE_CHUNKS) + assert result[0].grounded is True + + # --------------------------------------------------------------------------- # _inject_cross_refs # --------------------------------------------------------------------------- From b4fbd7930e1a8f14d18fa561b1936406dab8a993 Mon Sep 17 00:00:00 2001 From: Tarik Moussa Date: Mon, 15 Jun 2026 21:32:13 +0200 Subject: [PATCH 17/24] feat(config): wrap credentials in SecretStr (audit S-4) database_url, migration_database_url, mcp_auth_token and the mathpix app id/key are now pydantic SecretStr, so they render as '**********' in any repr/log/traceback instead of leaking the plaintext credential. Consumers (db.get_conn, cli.migrate, mcp._require_http_token, mathpix.extract_formulas) call .get_secret_value() at the point of use. Tests updated to the SecretStr type. Co-Authored-By: Claude Fable 5 --- codex/cli.py | 2 +- codex/config.py | 14 +++++++------- codex/db.py | 2 +- codex/mcp_server.py | 5 +++-- codex/parsing/mathpix.py | 6 ++++-- tests/mcp/test_http_auth.py | 5 +++-- tests/parsing/test_mathpix.py | 5 +++-- tests/scaffold/test_config.py | 7 +++++-- 8 files changed, 27 insertions(+), 19 deletions(-) diff --git a/codex/cli.py b/codex/cli.py index 4ba4152..3acac41 100644 --- a/codex/cli.py +++ b/codex/cli.py @@ -45,7 +45,7 @@ def migrate() -> None: from codex.db import apply_schema settings = get_settings() - url = settings.migration_database_url or settings.database_url + url = (settings.migration_database_url or settings.database_url).get_secret_value() try: with psycopg.connect(url, row_factory=psycopg.rows.dict_row) as conn: apply_schema(conn) diff --git a/codex/config.py b/codex/config.py index faf79ad..0adac76 100644 --- a/codex/config.py +++ b/codex/config.py @@ -9,7 +9,7 @@ from __future__ import annotations from functools import lru_cache -from pydantic import AliasChoices, Field +from pydantic import AliasChoices, Field, SecretStr from pydantic_settings import BaseSettings, SettingsConfigDict @@ -26,15 +26,15 @@ class Settings(BaseSettings): # ------------------------------------------------------------------ # Database # ------------------------------------------------------------------ - database_url: str = Field( - default="postgresql://researcher:change_me@localhost:5432/papers", + database_url: SecretStr = Field( + default=SecretStr("postgresql://researcher:change_me@localhost:5432/papers"), description=( "libpq-compatible connection string consumed by psycopg. " "Example: postgresql://user:pass@host:5432/dbname" ), ) - migration_database_url: str | None = Field( + migration_database_url: SecretStr | None = Field( default=None, description=( "Optional privileged connection string used by `codex migrate` to apply " @@ -148,7 +148,7 @@ class Settings(BaseSettings): # ------------------------------------------------------------------ # F-09 Rich Parsing # ------------------------------------------------------------------ - mathpix_app_id: str | None = Field( + mathpix_app_id: SecretStr | None = Field( default=None, description=( "MathPix App ID for cloud formula extraction. " @@ -156,7 +156,7 @@ class Settings(BaseSettings): ), ) - mathpix_app_key: str | None = Field( + mathpix_app_key: SecretStr | None = Field( default=None, description=( "MathPix App Key for cloud formula extraction. " @@ -257,7 +257,7 @@ class Settings(BaseSettings): description="Bind port for HTTP transport (ignored for stdio).", ) - mcp_auth_token: str | None = Field( + mcp_auth_token: SecretStr | None = Field( default=None, description=( "Bearer token required when mcp_transport='http'. " diff --git a/codex/db.py b/codex/db.py index f8272b9..384d4ae 100644 --- a/codex/db.py +++ b/codex/db.py @@ -36,7 +36,7 @@ def get_conn() -> Generator[psycopg.Connection[psycopg.rows.DictRow], None, None """ settings = get_settings() with psycopg.connect( - settings.database_url, + settings.database_url.get_secret_value(), row_factory=psycopg.rows.dict_row, ) as conn: yield conn diff --git a/codex/mcp_server.py b/codex/mcp_server.py index 5a82b08..9991627 100644 --- a/codex/mcp_server.py +++ b/codex/mcp_server.py @@ -294,12 +294,13 @@ def _require_http_token() -> str: from codex.config import get_settings settings = get_settings() - if not settings.mcp_auth_token: + token = settings.mcp_auth_token.get_secret_value() if settings.mcp_auth_token else "" + if not token: raise RuntimeError( "HTTP transport requires MCP_AUTH_TOKEN to be set. " "Set the environment variable or add it to .env." ) - return settings.mcp_auth_token + return token # --------------------------------------------------------------------------- diff --git a/codex/parsing/mathpix.py b/codex/parsing/mathpix.py index 1e4b2f8..f598997 100644 --- a/codex/parsing/mathpix.py +++ b/codex/parsing/mathpix.py @@ -302,8 +302,10 @@ def extract_formulas( """ settings = get_settings() - app_id = mathpix_app_id or settings.mathpix_app_id or "" - app_key = mathpix_app_key or settings.mathpix_app_key or "" + settings_id = settings.mathpix_app_id.get_secret_value() if settings.mathpix_app_id else "" + settings_key = settings.mathpix_app_key.get_secret_value() if settings.mathpix_app_key else "" + app_id = mathpix_app_id or settings_id + app_key = mathpix_app_key or settings_key if app_id and app_key: logger.info("Using MathPix backend for %s", pdf_path) diff --git a/tests/mcp/test_http_auth.py b/tests/mcp/test_http_auth.py index 74c8c79..ceed096 100644 --- a/tests/mcp/test_http_auth.py +++ b/tests/mcp/test_http_auth.py @@ -12,6 +12,7 @@ from __future__ import annotations from unittest.mock import MagicMock, patch import pytest +from pydantic import SecretStr # --------------------------------------------------------------------------- # _require_http_token @@ -37,7 +38,7 @@ def test_require_http_token_returns_token_when_set() -> None: from codex.mcp_server import _require_http_token mock_settings = MagicMock() - mock_settings.mcp_auth_token = "super-secret-token" + mock_settings.mcp_auth_token = SecretStr("super-secret-token") with patch("codex.config.get_settings", return_value=mock_settings): result = _require_http_token() @@ -97,7 +98,7 @@ def test_main_http_with_token_calls_uvicorn() -> None: mock_settings = MagicMock() mock_settings.mcp_transport = "http" - mock_settings.mcp_auth_token = "test-token" + mock_settings.mcp_auth_token = SecretStr("test-token") mock_settings.mcp_host = "127.0.0.1" mock_settings.mcp_port = 8765 diff --git a/tests/parsing/test_mathpix.py b/tests/parsing/test_mathpix.py index ce3597c..03d52ee 100644 --- a/tests/parsing/test_mathpix.py +++ b/tests/parsing/test_mathpix.py @@ -10,6 +10,7 @@ from typing import Any from unittest.mock import MagicMock, patch import pytest +from pydantic import SecretStr from codex.models import FormulaChunk @@ -295,8 +296,8 @@ class TestRouteSelection: def test_mathpix_selected_when_creds_present(self) -> None: """extract_formulas calls MathPix backend when both creds are set.""" mock_settings = MagicMock() - mock_settings.mathpix_app_id = "my_id" - mock_settings.mathpix_app_key = "my_key" + mock_settings.mathpix_app_id = SecretStr("my_id") + mock_settings.mathpix_app_key = SecretStr("my_key") mock_settings.pix2tex_fallback = True with ( diff --git a/tests/scaffold/test_config.py b/tests/scaffold/test_config.py index f8bde0c..c11751d 100644 --- a/tests/scaffold/test_config.py +++ b/tests/scaffold/test_config.py @@ -8,7 +8,10 @@ from codex.config import Settings, get_settings def test_settings_defaults() -> None: """Settings() is constructable without env vars; expected defaults are set.""" s = Settings() - assert s.database_url == "postgresql://researcher:change_me@localhost:5432/papers" + assert ( + s.database_url.get_secret_value() + == "postgresql://researcher:change_me@localhost:5432/papers" + ) assert s.embedding_model == "BAAI/bge-m3" assert s.embedding_dim == 1024 @@ -20,7 +23,7 @@ def test_settings_env_override(monkeypatch: object) -> None: mp: pytest.MonkeyPatch = monkeypatch # type: ignore[assignment] mp.setenv("DATABASE_URL", "postgresql://x:y@h:5432/db") s = Settings() - assert s.database_url == "postgresql://x:y@h:5432/db" + assert s.database_url.get_secret_value() == "postgresql://x:y@h:5432/db" def test_get_settings_is_singleton() -> None: From c7fae5c877246955dea892b32150d99811b847ef Mon Sep 17 00:00:00 2001 From: Tarik Moussa Date: Mon, 15 Jun 2026 21:39:48 +0200 Subject: [PATCH 18/24] fix(sources): drop no-op try/except + url-encode S2 ids (audit R-4, R-5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - R-4: arxiv.fetch_source had a try/except httpx.RequestError that only re-raised — removed (no behaviour change, less noise). - R-5: semanticscholar URLs now quote(paper_id, safe=':') so a legacy-arXiv '/' (arXiv:math/0603097) doesn't corrupt the path and silently return no references. Co-Authored-By: Claude Fable 5 --- codex/sources/arxiv.py | 5 +---- codex/sources/semanticscholar.py | 7 +++++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/codex/sources/arxiv.py b/codex/sources/arxiv.py index 1de3e38..5c81c13 100644 --- a/codex/sources/arxiv.py +++ b/codex/sources/arxiv.py @@ -38,10 +38,7 @@ def fetch_source(arxiv_id: str) -> str | None: file is present (signals Nougat fallback). """ url = f"{_BASE}/src/{arxiv_id}" - try: - response = httpx.get(url, timeout=60, follow_redirects=True) - except httpx.RequestError: - raise + response = httpx.get(url, timeout=60, follow_redirects=True) if response.status_code == 404: logger.debug("arXiv 404 for source id=%s", arxiv_id) return None diff --git a/codex/sources/semanticscholar.py b/codex/sources/semanticscholar.py index e82d75a..5c48c9a 100644 --- a/codex/sources/semanticscholar.py +++ b/codex/sources/semanticscholar.py @@ -14,6 +14,7 @@ import logging import threading import time from typing import Any +from urllib.parse import quote import httpx from tenacity import retry, retry_if_exception, stop_after_attempt, wait_exponential @@ -77,7 +78,9 @@ def fetch_references(paper_id: str) -> list[Citation]: list[Citation] One Citation per reference, with optional context snippet. """ - url = f"{_BASE_GRAPH}/paper/{paper_id}/references" + # quote the id so a legacy-arXiv "/" (e.g. arXiv:math/0603097) doesn't break + # the URL path; keep ":" for the arXiv:/DOI: scheme prefix (audit R-5). + url = f"{_BASE_GRAPH}/paper/{quote(paper_id, safe=':')}/references" params: dict[str, Any] = {"fields": "externalIds,contexts"} try: response = _get(url, params=params) @@ -118,7 +121,7 @@ def fetch_recommendations(paper_id: str, limit: int = 20) -> list[str]: list[str] List of recommended paper IDs. """ - url = f"{_BASE_RECS}/papers/forpaper/{paper_id}" + url = f"{_BASE_RECS}/papers/forpaper/{quote(paper_id, safe=':')}" params: dict[str, Any] = {"limit": limit} try: response = _get(url, params=params) From 92928bab99a273c83dd43f6cfc0c161798002b70 Mon Sep 17 00:00:00 2001 From: Tarik Moussa Date: Mon, 15 Jun 2026 21:43:19 +0200 Subject: [PATCH 19/24] feat(cli): graph-report titles + JSON warning; wire recommend/cocited (U-1,U-3,U-4,C-9) - U-1: 'graph report' shows the paper title next to each in-KB hub (dangling OpenAlex-id hubs are flagged), instead of bare ids. - U-3: '--json' now includes a 'small_corpus_warning' field (null unless below graph_min_corpus_size) instead of silently dropping the warning. - C-9: new 'codex discover recommend ' wires semanticscholar.fetch_recommendations (was implemented + tested but unreachable). - U-4: new 'codex graph cocited ' wires graph.find_co_cited (likewise). Co-Authored-By: Claude Fable 5 --- codex/cli.py | 67 +++++++++++++++++++++++++++++++++++------ tests/graph/test_cli.py | 6 ++-- 2 files changed, 62 insertions(+), 11 deletions(-) diff --git a/codex/cli.py b/codex/cli.py index 3acac41..a68fff4 100644 --- a/codex/cli.py +++ b/codex/cli.py @@ -188,6 +188,24 @@ def discover_leads( typer.echo(f"{item['pull']:>4}× {item['cited_id']}") +@discover_app.command("recommend") +def discover_recommend( + paper_id: str = typer.Argument( + ..., help="Semantic Scholar paper ID (or arXiv:/DOI: prefixed)." + ), + limit: int = typer.Option(20, "--limit", "-n", help="Number of recommendations."), +) -> None: + """Recommended papers for PAPER_ID via Semantic Scholar.""" + from codex.sources.semanticscholar import fetch_recommendations + + rec = fetch_recommendations(paper_id, limit=limit) + if not rec: + typer.echo(f"No recommendations found for {paper_id}.") + return + for pid in rec: + typer.echo(pid) + + @discover_app.command("citing") def discover_citing(paper_id: str = typer.Argument(...)) -> None: """List papers that cite PAPER_ID.""" @@ -582,7 +600,9 @@ def graph_report( settings = get_settings() with get_conn() as conn: graph = build_citation_graph(conn) - known_ids = {row["id"] for row in conn.execute("SELECT id FROM papers").fetchall()} + paper_rows = conn.execute("SELECT id, title FROM papers").fetchall() + known_ids = {row["id"] for row in paper_rows} + titles = {row["id"]: row["title"] for row in paper_rows} if graph.number_of_nodes() == 0: typer.echo("Citation graph is empty — ingest some papers first.") @@ -592,6 +612,13 @@ def graph_report( pr = citation_pagerank(graph) hubs = sorted(pr.items(), key=lambda x: -x[1])[:top_n] dangling = sorted(dangling_citations(graph, known_ids)) + small_corpus = n_papers < settings.graph_min_corpus_size + warning = ( + f"only {n_papers} ingested papers; recommended >= {settings.graph_min_corpus_size} " + "for meaningful ranking" + if small_corpus + else None + ) if output_json: typer.echo( @@ -599,7 +626,11 @@ def graph_report( { "nodes": graph.number_of_nodes(), "edges": graph.number_of_edges(), - "hubs": [{"paper_id": pid, "pagerank": score} for pid, score in hubs], + "small_corpus_warning": warning, # null unless below threshold (audit U-3) + "hubs": [ + {"paper_id": pid, "title": titles.get(pid), "pagerank": score} + for pid, score in hubs + ], "dangling_count": len(dangling), "dangling": dangling, }, @@ -608,18 +639,16 @@ def graph_report( ) return - if n_papers < settings.graph_min_corpus_size: - typer.echo( - f"Warning: only {n_papers} ingested papers " - f"(recommended ≥ {settings.graph_min_corpus_size} for meaningful ranking).", - err=True, - ) + if warning: + typer.echo(f"Warning: {warning}.", err=True) typer.echo(f"Graph: {graph.number_of_nodes()} nodes, {graph.number_of_edges()} edges") typer.echo("") typer.echo(f"TOP-{top_n} HUB PAPERS (PageRank)") typer.echo("-" * 48) for pid, score in hubs: - typer.echo(f" {score:.4f} {pid}") + # in-KB hubs show their title; dangling (OpenAlex-id) hubs are flagged (U-1) + label = titles.get(pid) or "(dangling — not ingested)" + typer.echo(f" {score:.4f} {pid} {label}") typer.echo("") typer.echo(f"DANGLING CITATIONS ({len(dangling)} cited but not ingested)") typer.echo("-" * 48) @@ -652,3 +681,23 @@ def graph_related( ) for pid in related: typer.echo(f" {pid}") + + +@graph_app.command("cocited") +def graph_cocited( + paper_id: str = typer.Argument(..., help="Paper ID to find co-cited papers for."), +) -> None: + """List papers frequently co-cited with PAPER_ID (graph co-citation).""" + from codex.db import get_conn + from codex.graph import build_citation_graph, find_co_cited + + with get_conn() as conn: + graph = build_citation_graph(conn) + + results = find_co_cited(paper_id, graph) + if not results: + typer.echo(f"No papers co-cited with {paper_id}.") + return + typer.echo(f"Papers co-cited with {paper_id} (count desc):") + for pid, count in results: + typer.echo(f" {count:>3}× {pid}") diff --git a/tests/graph/test_cli.py b/tests/graph/test_cli.py index 32ee60c..e99535b 100644 --- a/tests/graph/test_cli.py +++ b/tests/graph/test_cli.py @@ -28,9 +28,11 @@ def _make_conn_cm(conn: MagicMock): def _make_conn_with_paper_ids(*paper_ids: str) -> MagicMock: - """Mock conn that returns paper rows for SELECT id FROM papers.""" + """Mock conn that returns paper rows for SELECT id, title FROM papers.""" conn = MagicMock() - conn.execute.return_value.fetchall.return_value = [{"id": pid} for pid in paper_ids] + conn.execute.return_value.fetchall.return_value = [ + {"id": pid, "title": f"Title of {pid}"} for pid in paper_ids + ] return conn From f422b0fb84b20a4fcb6191909932dabfea2b82cc Mon Sep 17 00:00:00 2001 From: Tarik Moussa Date: Mon, 15 Jun 2026 21:46:02 +0200 Subject: [PATCH 20/24] fix(wiki): token-boundary grounding match + sharper ref-list filter (C-6, R-2) - C-6: the content-5-gram check used 'gram in src' on space-joined strings, so the first/last gram token could match a prefix/suffix of a longer chunk word ('set' inside 'subset'). Both gram and chunk are now space-padded, so a match requires whole-token alignment. Regression test added. - R-2: the reference-list filter's author pattern ('Surname, I.') also matched 'Theorem A.'/'Lemma B.', so theorem-dense chunks could be dropped as a bibliography. The pattern now excludes common math-structure words. Co-Authored-By: Claude Fable 5 --- codex/wiki.py | 18 +++++++++++++----- tests/wiki/test_compile.py | 24 ++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/codex/wiki.py b/codex/wiki.py index 7a0ec8b..59e17df 100644 --- a/codex/wiki.py +++ b/codex/wiki.py @@ -186,8 +186,15 @@ def _retrieve_chunks( ).fetchall() # Filter reference-list chunks: skip chunks whose content looks like a bibliography - # (heuristic: > 60 % of lines match "^\[\d+\]" or "^[A-Z][a-z]+,?\s+[A-Z]\."). - ref_pattern = re.compile(r"^\s*(\[\d+\]|[A-Z][a-z]+,?\s+[A-Z]\.)", re.MULTILINE) + # (heuristic: > 60 % of lines look like "[12]" refs or "Surname, I." authors). + # The author alternative excludes common math-structure words ("Theorem A.", + # "Lemma B.") so theorem-dense chunks aren't mistaken for a reference list (R-2). + ref_pattern = re.compile( + r"^\s*(\[\d+\]|(?!(?:Theorem|Lemma|Proposition|Corollary|Definition|Proof|" + r"Section|Figure|Fig|Table|Equation|Eq|Remark|Example|Chapter)\b)" + r"[A-Z][a-z]+,?\s+[A-Z]\.)", + re.MULTILINE, + ) filtered: list[dict[str, Any]] = [] for row in rows: content: str = row["content"] @@ -336,8 +343,9 @@ def _run_grounding_guard( for chunk in chunks: bk = str(chunk.get("bibkey") or "") if bk: - # Content words of the chunk (stopwords removed, lowercased) - cw = " ".join(_content_words(chunk["content"])) + # Content words of the chunk, space-padded so a 5-gram match is bounded + # by whole tokens, not a substring bleeding across words (audit C-6). + cw = " " + " ".join(_content_words(chunk["content"])) + " " bib_index.setdefault(bk, []).append(cw) for claim in claims: @@ -359,7 +367,7 @@ def _run_grounding_guard( found = False for i in range(len(content) - 4): # content-5-grams - gram = " ".join(content[i : i + 5]) + gram = " " + " ".join(content[i : i + 5]) + " " # padded: token-boundary match if any(gram in src for src in sources): found = True break diff --git a/tests/wiki/test_compile.py b/tests/wiki/test_compile.py index c0db957..8afcd5b 100644 --- a/tests/wiki/test_compile.py +++ b/tests/wiki/test_compile.py @@ -230,6 +230,30 @@ def test_grounding_guard_robust_to_trailing_comma() -> None: assert result[0].grounded is True +def test_grounding_guard_no_substring_bleed_across_tokens() -> None: + """A 5-gram must match whole tokens, not bleed into a longer chunk word (audit C-6). + + The chunk contains 'subset'; a claim whose first token is 'set' must NOT ground + by substring-matching the suffix of 'subset'. + """ + chunks = [ + { + "id": 99, + "paper_id": "p", + "ord": 0, + "bibkey": "TestBib", + "content": "the subset conformal map energy functional is minimized here", + } + ] + claim = Claim( + text="the set conformal map energy functional", + bibkey="TestBib", + locator="chunk 0", + ) + result = _run_grounding_guard([claim], chunks) + assert result[0].grounded is False + + # --------------------------------------------------------------------------- # _inject_cross_refs # --------------------------------------------------------------------------- From 45c3b16dd7b08144449a2d06ead5fa197073e473 Mon Sep 17 00:00:00 2001 From: Tarik Moussa Date: Mon, 15 Jun 2026 21:47:40 +0200 Subject: [PATCH 21/24] docs: clarify sparse-not-wired and cite-boost tie-breaker (audit C-14, U-2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - C-14: embed.py header now states that only the dense path is wired (search 'hybrid' is dense + Postgres FTS); encode_sparse/encode are reserved for a future sparse-retrieval layer, not yet consumed. - U-2: --cite-boost help clarifies it re-ranks within the top results (a tie-breaker), not a hard re-ranking — matching the small alpha effect. Accepted (documented heuristics, no change): R-3 conflict detection is a keyword-only signal labelled as such; R-12 classify_section is mostly 'body' because chunks are word-windows that rarely start at a section header. Co-Authored-By: Claude Fable 5 --- codex/cli.py | 3 ++- codex/embed.py | 7 ++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/codex/cli.py b/codex/cli.py index a68fff4..4a186e0 100644 --- a/codex/cli.py +++ b/codex/cli.py @@ -102,7 +102,8 @@ def search_paper( cite_boost: bool = typer.Option( False, "--cite-boost", - help="Weight results by citation PageRank (F-15). Graceful when corpus < 5 papers.", + help="Re-rank the top results by citation PageRank — a within-page " + "tie-breaker, not a hard re-ranking (F-15). Graceful when corpus < 5 papers.", ), ) -> None: """Semantic similarity search over paper abstracts.""" diff --git a/codex/embed.py b/codex/embed.py index fcfe67e..3c25ad6 100644 --- a/codex/embed.py +++ b/codex/embed.py @@ -1,4 +1,4 @@ -"""Hybrid dense + sparse embeddings via BGE-M3 (ADR-0002). +"""Dense (+ optional sparse) embeddings via BGE-M3 (ADR-0002). Uses :class:`FlagEmbedding.BGEM3FlagModel` for encoding because the ``return_dense`` / ``return_sparse`` kwargs are part of the FlagEmbedding @@ -7,6 +7,11 @@ vanilla ``SentenceTransformer`` load of ``BAAI/bge-m3`` (same weights, same model); sparse output is a list of ``{token_id: weight}`` dicts per text. +Status: only the **dense** path is wired into ingest/search today; the search +"hybrid" is dense + Postgres FTS. :meth:`Embedder.encode_sparse` / :meth:`encode` +are provided for a future sparse-retrieval layer but are not yet consumed by the +pipeline (audit C-14). + Notes for callers ----------------- * Empty input is handled explicitly — no model call is issued. From b52303d22b428fda1b69763cf4e22cb469099e36 Mon Sep 17 00:00:00 2001 From: Tarik Moussa Date: Mon, 15 Jun 2026 22:11:14 +0200 Subject: [PATCH 22/24] fix(ingest): retry on bibkey UNIQUE collision (audit C-15) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The id-keyed upsert (ON CONFLICT (id)) does not catch the papers.bibkey UNIQUE constraint, so a second paper whose auto-generated key matches an existing one (two same-author/year works → e.g. 'BobenkoLutz2023') failed the whole ingest with a UniqueViolation. Surfaced by the canonical-id re-ingest: 2305.10988 collided with 2310.17529. ingest now catches a bibkey UniqueViolation, rolls back, and retries the upsert with an a/b/c… suffix. Non-bibkey violations re-raise unchanged. Mocks don't raise, so the existing happy-path tests are untouched; a new test drives the collision-and-retry path. Co-Authored-By: Claude Fable 5 --- codex/ingest.py | 70 ++++++++++++++++++++++--------------- tests/ingest/test_ingest.py | 35 +++++++++++++++++++ 2 files changed, 77 insertions(+), 28 deletions(-) diff --git a/codex/ingest.py b/codex/ingest.py index 31e7e3d..ef5cb82 100644 --- a/codex/ingest.py +++ b/codex/ingest.py @@ -8,6 +8,7 @@ from dataclasses import dataclass from pathlib import Path import numpy as np +import psycopg from codex.config import get_settings from codex.db import get_conn @@ -125,35 +126,48 @@ def ingest_paper( # --------------------------------------------------------------- # 3. Upsert paper (INSERT … ON CONFLICT (id) DO UPDATE SET …) # --------------------------------------------------------------- + upsert_sql = """ + INSERT INTO papers (id, openalex_id, bibkey, title, authors, year, abstract, + source_path, abstract_emb) + VALUES (%(id)s, %(openalex_id)s, %(bibkey)s, %(title)s, %(authors)s, %(year)s, + %(abstract)s, %(source_path)s, %(abstract_emb)s) + ON CONFLICT (id) DO UPDATE SET + openalex_id = EXCLUDED.openalex_id, + bibkey = COALESCE(papers.bibkey, EXCLUDED.bibkey), + title = EXCLUDED.title, + authors = EXCLUDED.authors, + year = EXCLUDED.year, + abstract = EXCLUDED.abstract, + source_path = COALESCE(EXCLUDED.source_path, papers.source_path), + abstract_emb = EXCLUDED.abstract_emb + """ with get_conn() as conn: - conn.execute( - """ - INSERT INTO papers (id, openalex_id, bibkey, title, authors, year, abstract, - source_path, abstract_emb) - VALUES (%(id)s, %(openalex_id)s, %(bibkey)s, %(title)s, %(authors)s, %(year)s, - %(abstract)s, %(source_path)s, %(abstract_emb)s) - ON CONFLICT (id) DO UPDATE SET - openalex_id = EXCLUDED.openalex_id, - bibkey = COALESCE(papers.bibkey, EXCLUDED.bibkey), - title = EXCLUDED.title, - authors = EXCLUDED.authors, - year = EXCLUDED.year, - abstract = EXCLUDED.abstract, - source_path = COALESCE(EXCLUDED.source_path, papers.source_path), - abstract_emb = EXCLUDED.abstract_emb - """, - { - "id": paper.id, - "openalex_id": paper.openalex_id, - "bibkey": paper.bibkey, - "title": paper.title, - "authors": paper.authors, - "year": paper.year, - "abstract": paper.abstract, - "source_path": source_path, - "abstract_emb": abstract_emb, - }, - ) + # The id-keyed upsert does not catch a papers.bibkey UNIQUE collision (two + # same-author/year papers generate the same key). On such a collision retry + # with an a/b/c… suffix instead of failing the whole ingest (audit C-15). + base_bibkey = paper.bibkey + for attempt in range(27): + try: + conn.execute( + upsert_sql, + { + "id": paper.id, + "openalex_id": paper.openalex_id, + "bibkey": paper.bibkey, + "title": paper.title, + "authors": paper.authors, + "year": paper.year, + "abstract": paper.abstract, + "source_path": source_path, + "abstract_emb": abstract_emb, + }, + ) + break + except psycopg.errors.UniqueViolation as exc: + if base_bibkey is None or "bibkey" not in str(exc).lower() or attempt == 26: + raise + conn.rollback() + paper.bibkey = f"{base_bibkey}{chr(ord('a') + attempt)}" # Register openalex_id in paper_identifiers (alias table). # Every ingest records the primary openalex_id so the graph JOIN diff --git a/tests/ingest/test_ingest.py b/tests/ingest/test_ingest.py index 016854a..0f80f45 100644 --- a/tests/ingest/test_ingest.py +++ b/tests/ingest/test_ingest.py @@ -11,6 +11,7 @@ from typing import Any from unittest.mock import MagicMock, patch import numpy as np +import psycopg import pytest from codex.ingest import IngestResult, _make_bibkey, ingest_paper @@ -525,6 +526,40 @@ def test_ingest_sets_bibkey_when_none(tmp_path: Any) -> None: assert upsert_params["bibkey"] == "AliceBob2023" +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() + + # --------------------------------------------------------------------------- From 1f677211f7930de1d5ac64b8669fac94ef4da5e3 Mon Sep 17 00:00:00 2001 From: Tarik Moussa Date: Mon, 15 Jun 2026 22:13:56 +0200 Subject: [PATCH 23/24] =?UTF-8?q?docs(adr):=20finalise=20D-1=20=E2=80=94?= =?UTF-8?q?=20re-measure=20F-15=20spike=20on=20canonical=20corpus?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-ran the citation graph after the C-7 re-ingest (29/29 papers canonical/bare): 489 nodes, 590 edges, 472 dangling, 5.4% spread. The resolution fix's signature: Pinkall/Polthier 1993 resolves to its DOI and is now the rank-1 *in-KB* hub (was a dangling OpenAlex node); Bobenko/Springborn 2007 holds rank 7. Foundational hubs still surface — GO re-affirmed on the shipping topology. Updates the ADR spike table with before/after numbers. Co-Authored-By: Claude Fable 5 --- docs/adr/ADR-F15-literature-graph.md | 33 ++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/docs/adr/ADR-F15-literature-graph.md b/docs/adr/ADR-F15-literature-graph.md index 4733885..66e1b2e 100644 --- a/docs/adr/ADR-F15-literature-graph.md +++ b/docs/adr/ADR-F15-literature-graph.md @@ -177,13 +177,28 @@ was measured on a graph the current code no longer produces: 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. +- **Spike re-measured after migration (2026-06-15).** The corpus was re-ingested + onto canonical bare ids (29 papers, all bare — verified `url_form_ids = 0`), and + the graph re-run on the post-migration corpus: -**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. + | Metric | Original spike (pre-fix) | Post-migration (re-measured) | + |--------|--------------------------|------------------------------| + | Ingested papers | 29 | 29 | + | Graph nodes | 495 | 489 | + | Graph edges | 590 | 590 | + | Dangling nodes | 478 | 472 | + | Top-10 PR spread | 5.7 % | 5.4 % | + | Rank-1 hub | W1974956622 (dangling) | **Pinkall/Polthier 1993 — in-KB (resolved)** | + | Bobenko/Springborn 2007 | rank 7 | rank 7 | + + The resolution fix's signature is visible: Pinkall/Polthier 1993 now resolves to + its canonical DOI and rises to **rank 1 as an in-KB hub** (previously a dangling + OpenAlex node). Bobenko/Springborn 2007 holds rank 7, and the foundational works + (Pinkall/Polthier, Yamabe, Schoen, Bobenko/Springborn) still surface. **GO + re-affirmed** on the shipping topology. + + Migration footnotes: two papers initially survived in URL form because the + id-keyed upsert did not catch a `papers.openalex_id` collision, and `2305.10988` + failed on a `papers.bibkey` collision with `2310.17529` (both → + `BobenkoLutz2023`). Same upsert gap — resolved by the bibkey-retry (audit C-15) + plus a delete + re-ingest of the two stragglers; corpus is now 29/29 canonical. From e1c0a9826263d0b0bd01b09b4a6da445c5ea087c Mon Sep 17 00:00:00 2001 From: Tarik Moussa Date: Mon, 15 Jun 2026 22:25:30 +0200 Subject: [PATCH 24/24] fix(ingest): make both upsert UNIQUE gaps visible (audit C-15 follow-up) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- codex/ingest.py | 20 ++++++++++++++++++-- tests/ingest/test_ingest.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/codex/ingest.py b/codex/ingest.py index ef5cb82..a6d1663 100644 --- a/codex/ingest.py +++ b/codex/ingest.py @@ -164,10 +164,26 @@ def ingest_paper( ) break except psycopg.errors.UniqueViolation as exc: - if base_bibkey is None or "bibkey" not in str(exc).lower() or attempt == 26: - raise + msg = str(exc).lower() 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)}" + 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). # Every ingest records the primary openalex_id so the graph JOIN diff --git a/tests/ingest/test_ingest.py b/tests/ingest/test_ingest.py index 0f80f45..95f4423 100644 --- a/tests/ingest/test_ingest.py +++ b/tests/ingest/test_ingest.py @@ -560,6 +560,35 @@ def test_ingest_disambiguates_bibkey_on_unique_violation() -> None: 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) + + # ---------------------------------------------------------------------------