Merge pull request 'fix: audit remediation Wave 1 — correctness fixes C-1, C-4, C-11 (+ doc fixes)' (#12) from fix/audit-wave-1 into main
This commit is contained in:
@@ -295,7 +295,8 @@ class Settings(BaseSettings):
|
|||||||
le=1.0,
|
le=1.0,
|
||||||
description=(
|
description=(
|
||||||
"Weight of the PageRank citation-boost in hybrid search. "
|
"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."
|
"Set to 0.0 to disable the boost without removing the flag."
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -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 __future__ import annotations
|
||||||
|
|
||||||
from codex.db import get_conn
|
from codex.db import get_conn
|
||||||
|
from codex.graph import RESOLVED_CITATIONS_SQL
|
||||||
|
|
||||||
|
|
||||||
def discovery_leads(limit: int = 20) -> list[dict[str, int | str]]:
|
def discovery_leads(limit: int = 20) -> list[dict[str, int | str]]:
|
||||||
"""Return papers referenced by already-ingested papers but not yet collected.
|
"""Return papers referenced by already-ingested papers but not yet collected.
|
||||||
|
|
||||||
Returns list of dicts with keys:
|
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
|
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
|
SELECT cited_id, count(*) AS pull
|
||||||
FROM citations
|
FROM resolved
|
||||||
WHERE cited_id NOT IN (SELECT id FROM papers)
|
WHERE cited_id NOT IN (SELECT id FROM papers)
|
||||||
GROUP BY cited_id
|
GROUP BY cited_id
|
||||||
ORDER BY pull DESC
|
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]:
|
def citing_papers(paper_id: str) -> list[str]:
|
||||||
"""Return IDs of all papers that cite `paper_id` (in-graph reverse citations)."""
|
"""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:
|
with get_conn() as conn:
|
||||||
rows = conn.execute(sql, {"paper_id": paper_id}).fetchall()
|
rows = conn.execute(sql, {"paper_id": paper_id}).fetchall()
|
||||||
return [row["citing_id"] for row in rows]
|
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]:
|
def cited_by(paper_id: str) -> list[str]:
|
||||||
"""Return IDs of all papers that `paper_id` cites (forward citations)."""
|
"""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:
|
with get_conn() as conn:
|
||||||
rows = conn.execute(sql, {"paper_id": paper_id}).fetchall()
|
rows = conn.execute(sql, {"paper_id": paper_id}).fetchall()
|
||||||
return [row["cited_id"] for row in rows]
|
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.
|
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.
|
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
|
SELECT c2.cited_id AS paper_id, count(*) AS co_citations
|
||||||
FROM citations c1
|
FROM resolved c1
|
||||||
JOIN citations c2 ON c1.citing_id = c2.citing_id
|
JOIN resolved c2 ON c1.citing_id = c2.citing_id
|
||||||
WHERE c1.cited_id = %(paper_id)s
|
WHERE c1.cited_id = %(paper_id)s
|
||||||
AND c2.cited_id != %(paper_id)s
|
AND c2.cited_id != %(paper_id)s
|
||||||
GROUP BY c2.cited_id
|
GROUP BY c2.cited_id
|
||||||
|
|||||||
@@ -23,6 +23,23 @@ logger = logging.getLogger(__name__)
|
|||||||
|
|
||||||
_MIN_PAGERANK_NODES = 5
|
_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:
|
def build_citation_graph(conn: Any) -> nx.DiGraph:
|
||||||
"""Load the ``citations`` table into a directed graph.
|
"""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.
|
nx.DiGraph with every (citing_id, cited_id) pair as an edge.
|
||||||
"""
|
"""
|
||||||
rows = conn.execute(
|
rows = conn.execute(RESOLVED_CITATIONS_SQL).fetchall()
|
||||||
"""
|
|
||||||
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()
|
|
||||||
g: nx.DiGraph = nx.DiGraph()
|
g: nx.DiGraph = nx.DiGraph()
|
||||||
for row in rows:
|
for row in rows:
|
||||||
g.add_edge(row["citing_id"], row["cited_id"])
|
g.add_edge(row["citing_id"], row["cited_id"])
|
||||||
|
|||||||
@@ -248,9 +248,23 @@ def _grounded_ratio(claims: list[Claim]) -> float:
|
|||||||
return n_grounded / len(claims)
|
return n_grounded / len(claims)
|
||||||
|
|
||||||
|
|
||||||
def _make_lead_id(seq: int) -> str:
|
_KIND_PREFIX: dict[LeadKind, str] = {
|
||||||
"""Format a sequential lead id like ``L-0001``."""
|
"connection": "C",
|
||||||
return f"L-{seq:04d}"
|
"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:
|
def _safe_llm_generate(llm: LLMClient, prompt: str, model: str) -> str:
|
||||||
@@ -296,7 +310,7 @@ def _finalise_grounded_lead(
|
|||||||
return None
|
return None
|
||||||
marked = _mark_ungrounded(body, claims)
|
marked = _mark_ungrounded(body, claims)
|
||||||
return Lead(
|
return Lead(
|
||||||
id=_make_lead_id(seq),
|
id=_make_lead_id(seq, kind),
|
||||||
kind=kind,
|
kind=kind,
|
||||||
title=title,
|
title=title,
|
||||||
body=marked,
|
body=marked,
|
||||||
@@ -428,7 +442,7 @@ def find_gaps(
|
|||||||
# No coverage at all → record the gap directly (LLM not strictly needed).
|
# No coverage at all → record the gap directly (LLM not strictly needed).
|
||||||
leads.append(
|
leads.append(
|
||||||
Lead(
|
Lead(
|
||||||
id=_make_lead_id(seq),
|
id=_make_lead_id(seq, "gap"),
|
||||||
kind="gap",
|
kind="gap",
|
||||||
title=f"Gap: corpus has no material on '{topic}'",
|
title=f"Gap: corpus has no material on '{topic}'",
|
||||||
body=(
|
body=(
|
||||||
@@ -481,7 +495,7 @@ def find_gaps(
|
|||||||
for bibkey in sorted(missing):
|
for bibkey in sorted(missing):
|
||||||
leads.append(
|
leads.append(
|
||||||
Lead(
|
Lead(
|
||||||
id=_make_lead_id(seq),
|
id=_make_lead_id(seq, "gap"),
|
||||||
kind="gap",
|
kind="gap",
|
||||||
title=f"Gap: code cites '{bibkey}' but corpus does not contain it",
|
title=f"Gap: code cites '{bibkey}' but corpus does not contain it",
|
||||||
body=(
|
body=(
|
||||||
@@ -658,7 +672,7 @@ def propose_conjectures(
|
|||||||
continue
|
continue
|
||||||
leads.append(
|
leads.append(
|
||||||
Lead(
|
Lead(
|
||||||
id=_make_lead_id(seq),
|
id=_make_lead_id(seq, "conjecture"),
|
||||||
kind="conjecture",
|
kind="conjecture",
|
||||||
title=title,
|
title=title,
|
||||||
body=body,
|
body=body,
|
||||||
@@ -759,10 +773,11 @@ def write_leads(leads: list[Lead], leads_dir: str) -> None:
|
|||||||
* Grounded leads → ``leads_dir/grounded/<id>.md``
|
* Grounded leads → ``leads_dir/grounded/<id>.md``
|
||||||
* Conjectures → ``leads_dir/conjectures/<id>.md`` (HARD invariant)
|
* Conjectures → ``leads_dir/conjectures/<id>.md`` (HARD invariant)
|
||||||
|
|
||||||
The function ALSO maintains an append-only ``leads_dir/INDEX.md``:
|
The function ALSO rebuilds ``leads_dir/INDEX.md`` from the on-disk files on
|
||||||
a previously-recorded id is kept on subsequent runs even if it is
|
every run (see :func:`_update_index`): it reflects whatever lead files
|
||||||
overwritten with a newer body. The two sections (grounded /
|
currently exist under ``grounded/`` and ``conjectures/``, listed as two
|
||||||
conjectures) are visibly separated.
|
visibly separated sections. It is not append-only — a deleted lead file
|
||||||
|
drops out of the index on the next run.
|
||||||
|
|
||||||
Notes
|
Notes
|
||||||
-----
|
-----
|
||||||
|
|||||||
@@ -722,17 +722,23 @@ def compile_all(
|
|||||||
# Detect conflicts between chunks from different bibkeys
|
# Detect conflicts between chunks from different bibkeys
|
||||||
report.conflicts.extend(_detect_conflicts(concept.slug, chunks))
|
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)
|
total_claims = len(page.claims)
|
||||||
grounded_claims = sum(1 for c in page.claims if c.grounded)
|
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
|
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:
|
if total_claims == 0 or grounding_rate < settings.wiki_min_grounding_rate:
|
||||||
# Quarantine: write to wiki/draft/ instead of wiki/
|
# 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 = wiki_dir / "draft"
|
||||||
draft_dir.mkdir(parents=True, exist_ok=True)
|
draft_dir.mkdir(parents=True, exist_ok=True)
|
||||||
page_path = draft_dir / f"{concept.slug}.md"
|
(draft_dir / f"{concept.slug}.md").write_text(page.markdown, encoding="utf-8")
|
||||||
page_path.write_text(page.markdown, encoding="utf-8")
|
|
||||||
report.quarantined.append(concept.slug)
|
report.quarantined.append(concept.slug)
|
||||||
else:
|
else:
|
||||||
# Write page to wiki/
|
# Write page to wiki/
|
||||||
|
|||||||
@@ -76,7 +76,7 @@ def test_finalise_grounded_lead_succeeds_when_grounded(
|
|||||||
)
|
)
|
||||||
assert lead is not None
|
assert lead is not None
|
||||||
assert lead.kind == "connection"
|
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.confidence >= 0.5
|
||||||
assert lead.status == "unverified"
|
assert lead.status == "unverified"
|
||||||
assert lead.suggested_validation == ""
|
assert lead.suggested_validation == ""
|
||||||
|
|||||||
@@ -92,9 +92,21 @@ def test_lead_empty_provenance_raises() -> None:
|
|||||||
|
|
||||||
|
|
||||||
def test_lead_id_format_helper() -> 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-<K>-XXXX string."""
|
||||||
from codex.synthesis import _make_lead_id
|
from codex.synthesis import _make_lead_id
|
||||||
|
|
||||||
assert _make_lead_id(1) == "L-0001"
|
assert _make_lead_id(1, "connection") == "L-C-0001"
|
||||||
assert _make_lead_id(42) == "L-0042"
|
assert _make_lead_id(42, "gap") == "L-G-0042"
|
||||||
assert _make_lead_id(9999) == "L-9999"
|
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}"
|
||||||
|
|||||||
@@ -504,6 +504,47 @@ def test_log_append_records_skipped(
|
|||||||
assert "circle-packing" in content
|
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
|
# Idempotency: second compile without chunk change → no rewrite
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|||||||
Reference in New Issue
Block a user