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

fix(review): address PR #16 code-review findings

Browse Source 
#1 ingest: normalize the pinned caller id (_norm_cited_id) so a non-bare DOI caller cannot store a URL-form papers.id that defeats DQ-5 idempotency / the startswith(10.) recovery gates. #2 quality.section_label: collapse to a controlled bucket only for an EXACT canonical heading; descriptive titles ('Abstract Nonsense...') keep their real title instead of being mislabelled. #4 ra_grobid_backfill: release the read connection before the slow GROBID network loop, fresh connection for the write (no idle-in-transaction across the loop over the flaky tunnel).

#5/#10 tex: flatten_inputs strips unresolved input/include at the depth cap (no literal leak on cycles); _norm_texkey strips only a single leading ./ . #6/#7 arxiv.fetch_source: keep non-.tex members resolvable for input; pick primary on an UN-commented documentclass line. #13 is_arxiv_id: also exclude http:// and arXiv-DOI forms. Tests added/updated for each.

Left as deliberate decisions: #3 (pre-section text drop is pre-existing in extract_sections; abstract stored separately), #8 (script normalizer is intentionally self-contained, already documented), #9/#11/#12 (no current trigger / tightening the gzip heuristic would reject valid old LaTeX like documentstyle / title truncation is by design on a write-only column). ruff + mypy clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Tarik Moussa
2026-06-26 22:50:23 +02:00
parent 9e5184385a
commit b2f01ce010
11 changed files with 190 additions and 75 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
codex/ingest.py
View File

@@ -207,12 +207,14 @@ def ingest_paper(
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()
# use and what papers.id is contracted to hold (arXiv id or DOI). Pin it to the
# caller's id (audit C-7), but normalize it the same way cited-ids are (strip a
# https://doi.org/ or doi: prefix, lower-case DOIs) so a non-bare caller cannot
# store a URL-form papers.id that would defeat DQ-5's ON CONFLICT (id) idempotency
# or the `paper.id.startswith("10.")` recovery gates below. arXiv/W ids pass
# through unchanged. The OpenAlex work id is retained as openalex_id / in
# paper_identifiers.
paper.id = _norm_cited_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 (uses the

8
codex/parsing/tex.py
View File

@@ -170,7 +170,9 @@ def _norm_texkey(name: str) -> str:
``.tex`` so ``\\input{sections/intro}`` matches the archive member
``sections/intro.tex``.
"""
n = name.strip().lstrip("./")
n = name.strip()
if n.startswith("./"): # a single leading "./" only — don't eat "../" or ".hidden"
n = n[2:]
return n[:-4] if n.endswith(".tex") else n
@@ -188,7 +190,9 @@ def flatten_inputs(primary: str, files: dict[str, str]) -> str:
def _expand(text: str, depth: int) -> str:
if depth > 10:
return text
# Cap reached (likely an \input cycle): strip any still-unresolved
# directives rather than leak a literal "\input{…}" into the parsed text.
return _INPUT_RE.sub("", text)
def _repl(match: re.Match[str]) -> str:
content = norm.get(_norm_texkey(match.group(1)))

33
codex/quality.py
View File

@@ -127,14 +127,34 @@ def _clean_title(title: str) -> str:
return t[:60].rsplit(" ", 1)[0] if len(t) > 60 else t
# A heading collapses to a controlled bucket only when it IS one of these canonical
# section words — NOT when it merely starts with one. Prefix-matching via
# classify_section mislabels real sections ("Abstract Nonsense and Categories" →
# abstract, "References Architecture" → bibliography); an exact map avoids that while
# keeping cross-source consistency for the bare headings.
_SECTION_TITLE_BUCKETS = {
"abstract": "abstract",
"zusammenfassung": "abstract",
"introduction": "intro",
"einleitung": "intro",
"proof": "proof",
"beweis": "proof",
"references": "bibliography",
"bibliography": "bibliography",
"bibliographie": "bibliography",
"literatur": "bibliography",
}
def section_label(title: str | None, content: str) -> str:
"""Label a chunk's section, preferring the real ``\\section`` heading (R-F).
For section-aware ``.tex`` ingest *title* is the real heading: if it maps to a
controlled bucket via :func:`classify_section` (intro / theorem / proof /
abstract / bibliography) that bucket is kept (cross-source consistency);
otherwise the cleaned title itself is stored (e.g. ``"preliminaries"``,
``"rigidity"``) — far more signal than collapsing everything to ``body``.
For section-aware ``.tex`` ingest *title* is the real heading: a bare canonical
heading ("Introduction", "References", "Proof") maps to its controlled bucket
(cross-source consistency); any other heading is stored as its cleaned real
title (e.g. ``"preliminaries"``, ``"introduction to operator algebras"``) — far
more signal than collapsing to ``body`` AND without the prefix-match mislabels
that ``classify_section`` would produce on descriptive titles.
When *title* is None / blank (the ``.pdf`` / ``.txt`` / ``run_quality_pass``
paths) this falls back to content-based :func:`classify_section` — unchanged
@@ -146,8 +166,7 @@ def section_label(title: str | None, content: str) -> str:
cleaned = _clean_title(title)
if not cleaned:
return classify_section(content)
bucket = classify_section(cleaned)
return bucket if bucket != "body" else cleaned
return _SECTION_TITLE_BUCKETS.get(cleaned, cleaned)
# ---------------------------------------------------------------------------

33
codex/sources/arxiv.py
View File

@@ -97,6 +97,16 @@ def fetch_metadata(arxiv_id: str) -> Paper | None:
)
def _has_documentclass(content: str) -> bool:
"""True if *content* has an UN-commented ``\\documentclass`` line.
A plain ``"\\documentclass" in content`` substring test also matches a
commented-out ``%\\documentclass`` (common in arXiv preambles); requiring the
command to start its line avoids selecting the wrong primary file.
"""
return any(line.lstrip().startswith("\\documentclass") for line in content.splitlines())
def fetch_source(arxiv_id: str) -> str | None:
"""Download and extract the primary LaTeX source for an arXiv paper.
@@ -128,29 +138,38 @@ def fetch_source(arxiv_id: str) -> str | None:
from codex.parsing.tex import flatten_inputs
raw = response.content
# Image/font/binary members are never \input'd as text; skip them so the source
# map stays text-only (decode-with-replace would otherwise store garbled bytes).
skip_ext = (".png", ".jpg", ".jpeg", ".gif", ".bmp", ".pdf", ".eps", ".ps", ".ttf", ".otf")
try:
with tarfile.open(fileobj=io.BytesIO(raw), mode="r:gz") as tf:
files: dict[str, str] = {}
primary_name: str | None = None
for member in tf.getmembers():
if not member.name.endswith(".tex"):
if not member.isfile() or member.name.lower().endswith(skip_ext):
continue
f = tf.extractfile(member)
if f is None:
continue
content = f.read().decode("utf-8", errors="replace")
files[member.name] = content
# Prefer the \documentclass file as the primary document.
if primary_name is None and "\\documentclass" in content:
# Keep ALL text members resolvable so \input of a non-.tex include
# (e.g. a .def or extension-less body file) is not silently dropped (R-C).
files[member.name] = f.read().decode("utf-8", errors="replace")
# Primary doc = a .tex with an UN-commented \documentclass.
if (
primary_name is None
and member.name.endswith(".tex")
and _has_documentclass(files[member.name])
):
primary_name = member.name
if not files:
tex_files = {k: v for k, v in files.items() if k.endswith(".tex")}
if not tex_files:
logger.debug("No .tex files found in arXiv source for %s", arxiv_id)
return None
# No \documentclass anywhere → fall back to the largest .tex.
if primary_name is None:
primary_name = max(files, key=lambda k: len(files[k]))
primary_name = max(tex_files, key=lambda k: len(tex_files[k]))
# Inline \input/\include so multi-file projects yield the full body,
# not just the primary file's skeleton of include directives (R-C).

7
scripts/ra_grobid_backfill.py
View File

@@ -142,6 +142,10 @@ def main(argv: list[str] | None = None) -> int:
pdf_dir = Path(args.pdf_dir)
pdfs = {p.stem: p for p in pdf_dir.glob("*.pdf")}
# Read the target list in a short-lived connection, then release it: the GROBID
# extraction below is a slow per-paper network loop and must NOT hold an
# idle-in-transaction connection open across it (a mid-loop SSH-tunnel drop would
# otherwise abort the whole batch — see docs/infra/jetson-ssh-stability.md).
with get_conn() as conn:
rows = conn.execute(
"""
@@ -174,6 +178,7 @@ def main(argv: list[str] | None = None) -> int:
"WRITE" if args.write else "DRY-RUN",
)
# GROBID extraction — no DB connection held during this slow network loop.
all_citations: list[Citation] = []
for paper_id, pdf in targets:
try:
@@ -192,6 +197,8 @@ def main(argv: list[str] | None = None) -> int:
logger.info("DRY-RUN — no rows written. Re-run with --write to apply.")
return 0
# Fresh connection for the write — the read connection was already released.
with get_conn() as conn:
before = _coverage(conn)
with conn.cursor() as cur:
cur.executemany(

8
scripts/rc_tex_reingest.py
View File

@@ -50,7 +50,13 @@ def is_arxiv_id(paper_id: str) -> bool:
with ``10.``; OpenAlex W-ids start with ``W`` or an ``https://openalex.org/`` URL.
"""
p = (paper_id or "").strip()
return bool(p) and not p.startswith("10.") and not p.startswith(("W", "https://"))
# Exclude DOIs (incl. the arXiv DOI 10.48550/arXiv.*), OpenAlex W-ids, and any
# http(s) URL form; everything else is treated as a bare arXiv id.
return (
bool(p)
and not p.startswith(("10.", "W"))
and not p.lower().startswith(("http://", "https://"))
)
def _section_dist(conn: psycopg.Connection[psycopg.rows.DictRow], paper_id: str) -> dict[str, int]:

12
tests/ingest/test_ingest.py
View File

@@ -238,11 +238,15 @@ def test_ingest_paper_source_tex(tmp_path: Any) -> None:
chunk_sql: str = chunk_insert_call[0][0]
assert "INSERT INTO chunks" in chunk_sql
# R-F: heading → controlled bucket where it maps ("Introduction"→intro,
# "Proof of…"→proof), else the cleaned real title ("discrete conformal maps")
# instead of collapsing to "body".
# R-F: a bare canonical heading → bucket ("Introduction"→intro); any other
# heading is stored as its cleaned real title ("proof of the main theorem",
# "discrete conformal maps") instead of being collapsed/mislabelled.
chunk_rows = chunk_insert_call[0][1]
assert [row[4] for row in chunk_rows] == ["intro", "proof", "discrete conformal maps"]
assert [row[4] for row in chunk_rows] == [
"intro",
"proof of the main theorem",
"discrete conformal maps",
]
assert result.chunks_upserted == len(section_pairs)

12
tests/parsing/test_tex.py
View File

@@ -146,6 +146,18 @@ class TestFlattenInputs:
assert r"\input" not in out
assert "A" in out and "B" in out
def test_input_cycle_does_not_leak_directive(self) -> None:
# a -> b -> a: the depth cap must strip the unresolved directive, not leak it.
files = {"a.tex": r"AA \input{b}", "b.tex": r"BB \input{a}"}
out = flatten_inputs(files["a.tex"], files)
assert r"\input" not in out
assert "AA" in out and "BB" in out
def test_dot_slash_prefix_resolves_without_overstrip(self) -> None:
# "./intro" resolves intro.tex; lstrip must not eat a real leading dot/run.
files = {"main.tex": r"\input{./intro}", "intro.tex": "BODY"}
assert "BODY" in flatten_inputs(files["main.tex"], files)
class TestChunkSections:
def test_pairs_carry_section_titles(self) -> None:

17
tests/quality/test_quality.py
View File

@@ -272,16 +272,25 @@ class TestCleanTitle:
class TestSectionLabel:
def test_title_maps_to_controlled_bucket(self):
# Headings matching the controlled vocab keep the canonical bucket.
def test_bare_canonical_heading_maps_to_bucket(self):
# Only an EXACT canonical heading collapses to its controlled bucket.
assert section_label("Introduction", "body text") == "intro"
assert section_label("Proof of Theorem 3", "body text") == "proof"
assert section_label("Proof", "body text") == "proof"
assert section_label("References", "body text") == "bibliography"
assert section_label("Abstract", "body text") == "abstract"
def test_descriptive_title_stored_verbatim(self):
# R-F win: a non-vocab heading is stored as its cleaned title, not "body".
# R-F: a heading that merely STARTS with a keyword keeps its real title,
# not the bucket — fixes classify_section prefix-match over-bucketing.
assert section_label("Preliminaries", "body text") == "preliminaries"
assert section_label("3.1 Discrete Conformal Maps", "x") == "discrete conformal maps"
assert section_label("Proof of Theorem 3", "x") == "proof of theorem 3"
assert section_label("Abstract Nonsense and Categories", "x") == (
"abstract nonsense and categories"
)
assert section_label("Introduction to Operator Algebras", "x") == (
"introduction to operator algebras"
)
def test_none_or_blank_title_falls_back_to_content(self):
# .pdf/.txt path: unchanged content-based classification.

2
tests/scripts/test_rc_tex_reingest.py
View File

@@ -16,4 +16,6 @@ def test_is_arxiv_id_rejects_dois_and_wids() -> None:
assert not is_arxiv_id("10.14279/depositonce-20357")
assert not is_arxiv_id("W2971636899")
assert not is_arxiv_id("https://openalex.org/W2971636899")
assert not is_arxiv_id("http://openalex.org/W2971636899")
assert not is_arxiv_id("10.48550/arXiv.2305.10988") # arXiv DOI form, not bare id
assert not is_arxiv_id("")

31
tests/sources/test_arxiv.py
View File

@@ -110,6 +110,37 @@ def test_fetch_source_bare_gzip_single_file(monkeypatch: pytest.MonkeyPatch) ->
assert "Old single-file paper body." in result
def test_fetch_source_resolves_non_tex_include(monkeypatch: pytest.MonkeyPatch) -> None:
"""\\input of a non-.tex body member (e.g. a .def file) is inlined, not dropped (R-C)."""
main = b"\\documentclass{article}\\begin{document}\\input{body.def}\\end{document}"
tar_bytes = _make_tar_gz({"main.tex": main, "body.def": b"NON TEX BODY TEXT"})
def mock_get(url: str, *, timeout: int = 60, follow_redirects: bool = True) -> httpx.Response:
return httpx.Response(200, content=tar_bytes)
monkeypatch.setattr(httpx, "get", mock_get)
result = arxiv.fetch_source("2301.07041")
assert result is not None
assert "NON TEX BODY TEXT" in result
def test_fetch_source_ignores_commented_documentclass(monkeypatch: pytest.MonkeyPatch) -> None:
"""A commented %\\documentclass must not select a file as the primary document."""
decoy = b"% \\documentclass{article}\n\\section{Stub}\nDECOY ONLY"
real = b"\\documentclass{book}\\begin{document}REAL BODY HERE\\end{document}"
# decoy sorts/inserts first; without the fix it would win on the substring match.
tar_bytes = _make_tar_gz({"aaa_decoy.tex": decoy, "real.tex": real})
def mock_get(url: str, *, timeout: int = 60, follow_redirects: bool = True) -> httpx.Response:
return httpx.Response(200, content=tar_bytes)
monkeypatch.setattr(httpx, "get", mock_get)
result = arxiv.fetch_source("2301.07041")
assert result is not None
assert "REAL BODY HERE" in result
assert "DECOY ONLY" not in result
# ---------------------------------------------------------------------------
# fetch_source — 404 → None
# ---------------------------------------------------------------------------