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 | diff --git a/codex/cli.py b/codex/cli.py index 9128de1..44cff43 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).get_secret_value() + 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"), @@ -71,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.""" @@ -157,6 +189,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.""" @@ -391,7 +441,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 @@ -413,7 +471,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 [] ) @@ -548,7 +606,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.") @@ -560,6 +620,13 @@ def graph_report( dangling = sorted(dangling_citations(graph, known_ids)) n_citing, _ = citing_coverage(graph, known_ids) coverage = n_citing / n_papers if n_papers else 0.0 + 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( @@ -572,7 +639,11 @@ def graph_report( "papers": n_papers, "fraction": round(coverage, 4), }, - "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, }, @@ -581,12 +652,8 @@ 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) # R-D: low citing-paper coverage starves PageRank/coupling even at a healthy # paper count — warn on the share of papers that actually have out-edges. if coverage < settings.graph_min_citing_coverage: @@ -602,7 +669,9 @@ def graph_report( 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) @@ -635,3 +704,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/codex/config.py b/codex/config.py index fbe59e4..fbc6eb6 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,14 +26,25 @@ 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: SecretStr | 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 # ------------------------------------------------------------------ @@ -137,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. " @@ -145,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. " @@ -246,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'. " @@ -295,7 +306,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/db.py b/codex/db.py index e86678f..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 @@ -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/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/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. diff --git a/codex/graph.py b/codex/graph.py index 5f4a5a3..9b83b75 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"]) diff --git a/codex/ingest.py b/codex/ingest.py index a5138a6..6518d74 100644 --- a/codex/ingest.py +++ b/codex/ingest.py @@ -7,6 +7,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 @@ -205,9 +206,18 @@ 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 (audit C-7); since DQ-5 + # `openalex._canonical_id` already normalizes that to the bare form, this pin + # is belt-and-suspenders and keeps re-ingest idempotent regardless. The + # OpenAlex work id is retained as openalex_id and in paper_identifiers. + paper.id = paper_id.strip() + # DQ-2: OpenAlex often has the paper but no abstract (it cannot redistribute - # some publishers' abstracts). Supplement from S2, then Crossref, so the - # paper gets a real (non-zero) abstract embedding instead of a zero vector. + # some publishers' abstracts). Supplement from S2, then Crossref (uses the + # canonical paper.id set above), so the paper gets a real (non-zero) abstract + # embedding instead of a zero vector. if not (paper.abstract or "").strip(): recovered = _recover_abstract(paper) if recovered: @@ -232,35 +242,64 @@ 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: + 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 @@ -429,7 +468,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 ], ) @@ -443,7 +485,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/codex/mcp_server.py b/codex/mcp_server.py index 1caf294..9991627 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 @@ -298,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 # --------------------------------------------------------------------------- @@ -337,7 +334,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) 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/codex/sources/arxiv.py b/codex/sources/arxiv.py index 7c1a803..80b6932 100644 --- a/codex/sources/arxiv.py +++ b/codex/sources/arxiv.py @@ -118,10 +118,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 5862071..591ca73 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) @@ -140,7 +143,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) diff --git a/codex/synthesis.py b/codex/synthesis.py index dcc1586..d04481d 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, @@ -402,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. @@ -415,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)} @@ -428,7 +443,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 +496,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 +673,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, @@ -759,10 +774,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 ----- diff --git a/codex/wiki.py b/codex/wiki.py index b47a98a..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"] @@ -273,7 +280,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 +305,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( @@ -322,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: @@ -345,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 @@ -402,6 +424,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 +440,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 +454,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 +633,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 +652,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 # --------------------------------------------------------------------------- @@ -722,17 +746,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/docs/adr/ADR-F15-literature-graph.md b/docs/adr/ADR-F15-literature-graph.md index 8a25bff..66e1b2e 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,42 @@ 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 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: + + | 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. diff --git a/infra/reingest_canonical_ids.sh b/infra/reingest_canonical_ids.sh new file mode 100755 index 0000000..213862c --- /dev/null +++ b/infra/reingest_canonical_ids.sh @@ -0,0 +1,141 @@ +#!/usr/bin/env bash +# One-time migration to canonical paper IDs (audit C-7). +# +# Before the C-7 fix, ingest stored OpenAlex's URL-form doi/id as papers.id +# (e.g. "https://doi.org/10.48550/arxiv.math/0603097"). After the fix, ingest +# keys papers.id on the bare caller id ("math/0603097"). The existing rows keep +# their old URL-form primary keys, so a plain re-ingest would DUPLICATE every +# paper (new bare-id row alongside the old URL-id row). This script wipes the +# corpus and rebuilds it on canonical ids. +# +# DESTRUCTIVE: `TRUNCATE papers CASCADE` removes papers + chunks + citations + +# formulas + figures + paper_identifiers (code_links.paper_id is set NULL). All +# content is rebuilt by ingest_all.sh, which re-fetches OpenAlex and re-embeds +# (minutes for ~36 papers). +# +# Prerequisites: SSH tunnel on :5433 (see ingest_all.sh) and .env.jetson-ingest. +# Usage: bash infra/reingest_canonical_ids.sh +set -euo pipefail + +CODEX_DIR="$(cd "$(dirname "$0")/.." && pwd)" +ENV_FILE="$CODEX_DIR/.env.jetson-ingest" +PY="$CODEX_DIR/.venv/bin/python" +TUNNEL_PORT=5433 + +# ── 1. Preconditions ───────────────────────────────────────────────────────── +if ! nc -z localhost "$TUNNEL_PORT" 2>/dev/null; then + echo "ERROR: SSH tunnel not active on :$TUNNEL_PORT" + echo "Run: ssh -f -N -L 5433:localhost:5432 alfred@192.168.178.103" + exit 1 +fi +[[ -f "$ENV_FILE" ]] || { echo "ERROR: $ENV_FILE missing"; exit 1; } +[[ -x "$PY" ]] || { echo "ERROR: venv python not found at $PY (run uv sync)"; exit 1; } +# Load DATABASE_URL into the environment (not echoed — keeps the password out of logs). +set -a; source "$ENV_FILE"; set +a + +# run_sql : execute one statement via psycopg, print any result rows. +# Reads DATABASE_URL from the environment; suppresses the DSN on error. +run_sql() { + "$PY" - "$1" <<'PYEOF' +import os, sys, psycopg +from psycopg.rows import dict_row +try: + with psycopg.connect(os.environ["DATABASE_URL"], row_factory=dict_row, connect_timeout=10) as c: + cur = c.execute(sys.argv[1]) + if cur.description: + for row in cur.fetchall(): + print(" " + " ".join(f"{k}={v!r}" for k, v in row.items())) + c.commit() +except Exception as e: + print(f" DB ERROR: {type(e).__name__} (details suppressed to avoid DSN leak)") + sys.exit(1) +PYEOF +} + +# ── 2. BEFORE snapshot ─────────────────────────────────────────────────────── +echo "── BEFORE migration ──────────────────────────────────────────" +run_sql "SELECT count(*) AS papers, count(*) FILTER (WHERE starts_with(id, 'https://')) AS url_form_ids FROM papers" +echo " sample ids:" +run_sql "SELECT id FROM papers ORDER BY added_at LIMIT 3" + +# ── 2b. Preflight: verify the schema is re-ingestable BEFORE wiping anything ── +# ingest writes chunks.section (F-16). On a live DB that predates it the column +# is missing, and the app user cannot ALTER tables owned by 'postgres' (audit +# M-1). Check FIRST so we never TRUNCATE and then fail mid-re-ingest. +echo "── Preflight: schema readiness ───────────────────────────────" +SECTION_OK="$("$PY" - <<'PYEOF' +import os, psycopg +try: + with psycopg.connect(os.environ["DATABASE_URL"], connect_timeout=10) as c: + cols = [r[0] for r in c.execute( + "SELECT column_name FROM information_schema.columns WHERE table_name='chunks'").fetchall()] + print("yes" if "section" in cols else "no") +except Exception: + print("error") +PYEOF +)" +if [[ "$SECTION_OK" != "yes" ]]; then + echo "ABORT — schema not ready: chunks.section is missing (audit M-1)." + echo "The app user cannot add it (core tables are owned by 'postgres'). Sync the" + echo "schema with a privileged role, then re-run this script. Either:" + echo "" + 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 +fi +echo " chunks.section present — schema OK." + +# ── 3. Confirmation gate (destructive) ─────────────────────────────────────── +echo "" +echo "This TRUNCATEs papers CASCADE (papers/chunks/citations/formulas/figures/" +echo "paper_identifiers) and re-ingests via ingest_all.sh. The DB content is" +echo "rebuilt from scratch." +read -r -p "Type 'MIGRATE' to proceed: " confirm +[[ "$confirm" == "MIGRATE" ]] || { echo "Aborted — nothing changed."; exit 1; } + +# ── 4. Wipe ────────────────────────────────────────────────────────────────── +echo "── TRUNCATE papers CASCADE ───────────────────────────────────" +run_sql "TRUNCATE papers CASCADE" +echo " wiped." + +# ── 5. Re-ingest on canonical ids ──────────────────────────────────────────── +echo "── Re-ingest (ingest_all.sh) ─────────────────────────────────" +bash "$CODEX_DIR/ingest_all.sh" + +# ── 6. AFTER snapshot + live verification ──────────────────────────────────── +echo "── AFTER migration ───────────────────────────────────────────" +echo " C-7 — url_form_ids should now be 0; sample ids should be bare:" +run_sql "SELECT count(*) AS papers, count(*) FILTER (WHERE starts_with(id, 'https://')) AS url_form_ids FROM papers" +run_sql "SELECT id FROM papers ORDER BY added_at LIMIT 3" + +# C-1 must be verified through the REAL resolver-based discovery_leads(), not a +# raw cited_id check: cited_id/openalex_id stay in OpenAlex form, so the raw +# "cited_id NOT IN papers.id" count is non-zero by design — the resolver is what +# excludes ingested papers. Check that no ingested paper leaks into the leads. +echo " C-1 — real discovery_leads() must contain no already-ingested paper:" +PYTHONPATH="$CODEX_DIR" "$PY" - <<'PYEOF' +import os, psycopg +from psycopg.rows import dict_row +from codex.discover import discovery_leads +try: + leads = discovery_leads(limit=100000) + with psycopg.connect(os.environ["DATABASE_URL"], row_factory=dict_row, connect_timeout=10) as c: + ingested = {r["id"] for r in c.execute("SELECT id FROM papers").fetchall()} + ingested |= { + r["openalex_id"] + for r in c.execute("SELECT openalex_id FROM papers WHERE openalex_id IS NOT NULL").fetchall() + } + leaked = sum(1 for lead in leads if lead["cited_id"] in ingested) + print(f" leads={len(leads)} ingested-papers-leaked-into-leads={leaked}") +except Exception as e: + print(f" DB ERROR: {type(e).__name__} (details suppressed)") +PYEOF + +echo "" +echo "✓ Migration complete." +echo " Expected: url_form_ids=0 (C-7 verified) and leaked=0 (C-1 verified) above." +echo " Next: re-run 'codex graph report' and refresh the ADR-F15 spike table (audit D-1)." diff --git a/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/graph/test_cli.py b/tests/graph/test_cli.py index 2dfb449..dbf5c83 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 diff --git a/tests/ingest/test_ingest.py b/tests/ingest/test_ingest.py index f5f935f..85b54de 100644 --- a/tests/ingest/test_ingest.py +++ b/tests/ingest/test_ingest.py @@ -12,6 +12,7 @@ from unittest.mock import MagicMock, patch import httpx import numpy as np +import psycopg import pytest from codex.ingest import IngestResult, _make_bibkey, ingest_paper @@ -161,6 +162,37 @@ def test_ingest_crossref_not_called_when_openalex_has_citations() -> None: mock_cr.assert_not_called() +def test_ingest_canonicalises_paper_id_to_caller_arg() -> None: + """papers.id is the caller-supplied id, not OpenAlex's URL-form id (audit C-7). + + Real OpenAlex returns the doi/id as a full URL; ingest must key the paper on + the bare id the caller passed (matching ingest scripts and CLI lookups), and + keep the OpenAlex work id only as openalex_id. + """ + oa_paper = Paper( + id="https://doi.org/10.48550/arxiv.math/0603097", # URL form, as OpenAlex emits + title="A Test Paper", + openalex_id="https://openalex.org/W4301005794", + authors=["Alice", "Bob"], + year=2008, + abstract="Test abstract.", + ) + mock_conn = MagicMock() + + with ( + patch("codex.ingest.openalex.fetch_paper", return_value=oa_paper), + patch("codex.ingest.openalex.fetch_citations", return_value=[]), + patch("codex.ingest.get_embedder", return_value=_fake_embedder()), + patch("codex.ingest.get_conn", side_effect=_make_conn_cm(mock_conn)), + ): + result = ingest_paper("math/0603097") + + papers_params = mock_conn.execute.call_args_list[0][0][1] + assert papers_params["id"] == "math/0603097", "papers.id must be the caller's bare id" + assert papers_params["openalex_id"] == "https://openalex.org/W4301005794" + assert result.paper_id == "math/0603097" + + # --------------------------------------------------------------------------- # 2. test_ingest_paper_source_tex # --------------------------------------------------------------------------- @@ -562,9 +594,11 @@ def test_ingest_paper_rich_calls_formula_and_figure_parsers(tmp_path: Any) -> No mock_conn.executemany = MagicMock() mock_conn.commit = MagicMock() - fake_formula = FormulaChunk(paper_id=paper.id, page=1, raw_latex=r"\alpha", context="test") + # Extractors derive paper_id from the PDF filename stem ("paper"), which differs + # from papers.id — ingest must insert papers.id, not this stem (audit C-10). + fake_formula = FormulaChunk(paper_id="paper", page=1, raw_latex=r"\alpha", context="test") fake_figure = FigureChunk( - paper_id=paper.id, page=1, image_path="/tmp/fig.png", caption="Figure 1." + paper_id="paper", page=1, image_path="/tmp/fig.png", caption="Figure 1." ) mock_settings = MagicMock() @@ -591,6 +625,16 @@ def test_ingest_paper_rich_calls_formula_and_figure_parsers(tmp_path: Any) -> No assert result.formulas_upserted == 1 assert result.figures_upserted == 1 + # C-10: formula/figure rows must be keyed on papers.id, not the extractor's + # filename-stem paper_id, or the FK to papers(id) is violated. + cursor = mock_conn.cursor.return_value.__enter__.return_value + inserted_paper_ids = { + row[0] for call in cursor.executemany.call_args_list for row in call[0][1] + } + assert inserted_paper_ids == {paper.id}, ( + f"formula/figure inserts must use papers.id, got {inserted_paper_ids}" + ) + def test_ingest_paper_no_rich_skips_parsers(tmp_path: Any) -> None: """When rich=False (default), formula and figure parsers are NOT called.""" @@ -781,6 +825,69 @@ def test_ingest_doi_paper_upserts_bare_canonical_id() -> None: ) +def test_ingest_disambiguates_bibkey_on_unique_violation() -> None: + """A papers.bibkey UNIQUE collision is retried with an a/b/c suffix (audit C-15).""" + paper = _make_paper() # bibkey auto-generates to "AliceBob2023" + mock_conn = MagicMock() + state = {"first": True} + + def execute_side_effect(sql: str, params: Any = None) -> MagicMock: + # The first papers INSERT collides on the bibkey; the retry must succeed. + if "INSERT INTO papers" in sql and state["first"]: + state["first"] = False + raise psycopg.errors.UniqueViolation( + 'duplicate key value violates unique constraint "papers_bibkey_key"' + ) + return MagicMock() + + mock_conn.execute.side_effect = execute_side_effect + + with ( + patch("codex.ingest.openalex.fetch_paper", return_value=paper), + patch("codex.ingest.openalex.fetch_citations", return_value=[]), + patch("codex.ingest.get_embedder", return_value=_fake_embedder()), + patch("codex.ingest.get_conn", side_effect=_make_conn_cm(mock_conn)), + ): + ingest_paper(paper.id) + + papers_inserts = [ + c for c in mock_conn.execute.call_args_list if "INSERT INTO papers" in c[0][0] + ] + assert len(papers_inserts) == 2, "expected one retry after the bibkey collision" + assert papers_inserts[0][0][1]["bibkey"] == "AliceBob2023" + assert papers_inserts[1][0][1]["bibkey"] == "AliceBob2023a" + mock_conn.rollback.assert_called_once() + + +def test_ingest_openalex_collision_raises_clear_error() -> None: + """An openalex_id UNIQUE collision raises a clear, actionable error (audit C-15). + + Unlike a bibkey clash (auto-suffixed), a shared openalex_id means two papers + claim the same OpenAlex work — a real data conflict that must surface, not the + raw psycopg error. + """ + paper = _make_paper() + mock_conn = MagicMock() + + def execute_side_effect(sql: str, params: Any = None) -> MagicMock: + if "INSERT INTO papers" in sql: + raise psycopg.errors.UniqueViolation( + 'duplicate key value violates unique constraint "papers_openalex_id_key"' + ) + return MagicMock() + + mock_conn.execute.side_effect = execute_side_effect + + with ( + patch("codex.ingest.openalex.fetch_paper", return_value=paper), + patch("codex.ingest.openalex.fetch_citations", return_value=[]), + patch("codex.ingest.get_embedder", return_value=_fake_embedder()), + patch("codex.ingest.get_conn", side_effect=_make_conn_cm(mock_conn)), + pytest.raises(ValueError, match="openalex_id"), + ): + ingest_paper(paper.id) + + # --------------------------------------------------------------------------- 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: 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 diff --git a/tests/sources/test_openalex.py b/tests/sources/test_openalex.py index 82c329b..de557ea 100644 --- a/tests/sources/test_openalex.py +++ b/tests/sources/test_openalex.py @@ -15,8 +15,8 @@ from codex.sources.openalex import _canonical_id, _normalize_doi, _resolve_id _SAMPLE_WORK = { "id": "https://openalex.org/W2741809807", - # Real OpenAlex returns the DOI as a full URL (T-3: the old bare fixture hid - # the URL-form behaviour that DQ-5 is about). + # Real OpenAlex returns the DOI as a full URL, not a bare DOI (audit T-3 / DQ-5 + # — the old bare fixture hid the URL-form behaviour both fixes address). "doi": "https://doi.org/10.1145/3592430", "title": "Conformal Prediction: A Review", "publication_year": 2023, @@ -168,8 +168,11 @@ def test_fetch_paper_success(monkeypatch: pytest.MonkeyPatch) -> None: assert paper is not None assert isinstance(paper, Paper) - # DQ-5 / C-7: paper.id is the canonical BARE DOI, not the full URL OpenAlex - # returns. This is what makes the ingest ON CONFLICT (id) upsert idempotent. + # DQ-5: openalex._canonical_id normalizes OpenAlex's URL-form doi to the BARE + # lower-cased DOI at the source layer; ingest additionally pins papers.id to the + # caller's id (audit C-7). Both converge on the bare canonical id — which is what + # makes the ingest ON CONFLICT (id) upsert idempotent. (T-3: the fixture is + # URL-form so this mapping can't silently regress.) assert paper.id == "10.1145/3592430" assert paper.title == "Conformal Prediction: A Review" assert paper.year == 2023 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 17e6fb8..59ac7ef 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 == "" @@ -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) 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}" diff --git a/tests/wiki/test_compile.py b/tests/wiki/test_compile.py index 53ff8fc..8afcd5b 100644 --- a/tests/wiki/test_compile.py +++ b/tests/wiki/test_compile.py @@ -213,6 +213,47 @@ 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 + + +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 # --------------------------------------------------------------------------- @@ -274,6 +315,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 = [ @@ -504,6 +558,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 # ---------------------------------------------------------------------------