From d1ffea1c1ce544e38ae2c8d4d663a8dadf3fca48 Mon Sep 17 00:00:00 2001 From: Tarik Moussa Date: Thu, 4 Jun 2026 23:47:18 +0200 Subject: [PATCH] fix(parsing): case-insensitive arXiv idno match; fix tmp_path types; add extract_structure test Review-Gate finding: GROBID emits type="arXiv" (camel-case), not "arxiv". XPath literal match silently returned empty strings for all real responses. Fix: iterate idno elements and compare .lower() == "arxiv". Test fixture updated to reflect real GROBID output (type="arXiv"). Co-Authored-By: Claude Sonnet 4.6 --- codex/parsing/grobid.py | 9 ++++-- tests/parsing/test_grobid.py | 60 ++++++++++++++++++++++++------------ 2 files changed, 47 insertions(+), 22 deletions(-) diff --git a/codex/parsing/grobid.py b/codex/parsing/grobid.py index 03bb186..b2b5443 100644 --- a/codex/parsing/grobid.py +++ b/codex/parsing/grobid.py @@ -84,9 +84,12 @@ def extract_references( doi_el = bib.find(f".//{_TEI_NS}idno[@type='DOI']") doi = _text(doi_el) - # arXiv ID - arxiv_el = bib.find(f".//{_TEI_NS}idno[@type='arxiv']") - arxiv_id = _text(arxiv_el) + # arXiv ID — GROBID emits type="arXiv" (camel-case); match case-insensitively + arxiv_id = "" + for idno_el in bib.iter(f"{_TEI_NS}idno"): + if idno_el.get("type", "").lower() == "arxiv": + arxiv_id = (idno_el.text or "").strip() + break results.append( { diff --git a/tests/parsing/test_grobid.py b/tests/parsing/test_grobid.py index eaa7dfd..8011606 100644 --- a/tests/parsing/test_grobid.py +++ b/tests/parsing/test_grobid.py @@ -2,13 +2,12 @@ from __future__ import annotations +from pathlib import Path from unittest.mock import MagicMock, patch -import pytest +from codex.parsing.grobid import extract_references, extract_structure -from codex.parsing.grobid import extract_references - -# Minimal valid TEI XML with 2 biblStruct elements +# Real GROBID TEI output uses type="arXiv" (camel-case), not "arxiv". _TEI_XML = """\ @@ -32,7 +31,7 @@ _TEI_XML = """\ 10.1234/nlp.2021 - 2101.00001 + 2101.00001 @@ -73,28 +72,36 @@ class _FakeResponse: pass +def _mock_post(xml: str = _TEI_XML) -> MagicMock: + fake = MagicMock() + fake.text = xml + fake.status_code = 200 + fake.raise_for_status = MagicMock() + return fake + + class TestExtractReferences: - def test_returns_two_dicts(self, tmp_path: pytest.TempdirFactory) -> None: - pdf = tmp_path / "paper.pdf" # type: ignore[operator] + def test_returns_two_dicts(self, tmp_path: Path) -> None: + pdf = tmp_path / "paper.pdf" pdf.write_bytes(b"%PDF fake") with patch("httpx.Client") as mock_client_cls: mock_client = MagicMock() mock_client_cls.return_value.__enter__.return_value = mock_client - mock_client.post.return_value = _FakeResponse() + mock_client.post.return_value = _mock_post() refs = extract_references(str(pdf), grobid_url="http://localhost:8070") assert len(refs) == 2 - def test_all_keys_present(self, tmp_path: pytest.TempdirFactory) -> None: - pdf = tmp_path / "paper.pdf" # type: ignore[operator] + def test_all_keys_present(self, tmp_path: Path) -> None: + pdf = tmp_path / "paper.pdf" pdf.write_bytes(b"%PDF fake") with patch("httpx.Client") as mock_client_cls: mock_client = MagicMock() mock_client_cls.return_value.__enter__.return_value = mock_client - mock_client.post.return_value = _FakeResponse() + mock_client.post.return_value = _mock_post() refs = extract_references(str(pdf), grobid_url="http://localhost:8070") @@ -102,14 +109,14 @@ class TestExtractReferences: for ref in refs: assert required_keys == set(ref.keys()), f"Missing keys in {ref}" - def test_first_ref_values(self, tmp_path: pytest.TempdirFactory) -> None: - pdf = tmp_path / "paper.pdf" # type: ignore[operator] + def test_first_ref_values(self, tmp_path: Path) -> None: + pdf = tmp_path / "paper.pdf" pdf.write_bytes(b"%PDF fake") with patch("httpx.Client") as mock_client_cls: mock_client = MagicMock() mock_client_cls.return_value.__enter__.return_value = mock_client - mock_client.post.return_value = _FakeResponse() + mock_client.post.return_value = _mock_post() refs = extract_references(str(pdf), grobid_url="http://localhost:8070") @@ -120,16 +127,14 @@ class TestExtractReferences: assert first["doi"] == "10.1234/nlp.2021" assert first["arxiv_id"] == "2101.00001" - def test_second_ref_missing_arxiv_is_empty_string( - self, tmp_path: pytest.TempdirFactory - ) -> None: - pdf = tmp_path / "paper.pdf" # type: ignore[operator] + def test_second_ref_missing_arxiv_is_empty_string(self, tmp_path: Path) -> None: + pdf = tmp_path / "paper.pdf" pdf.write_bytes(b"%PDF fake") with patch("httpx.Client") as mock_client_cls: mock_client = MagicMock() mock_client_cls.return_value.__enter__.return_value = mock_client - mock_client.post.return_value = _FakeResponse() + mock_client.post.return_value = _mock_post() refs = extract_references(str(pdf), grobid_url="http://localhost:8070") @@ -137,3 +142,20 @@ class TestExtractReferences: assert second["arxiv_id"] == "" assert second["title"] == "Attention Is All You Need" assert second["year"] == "2017" + + +class TestExtractStructure: + def test_returns_xml_string(self, tmp_path: Path) -> None: + pdf = tmp_path / "paper.pdf" + pdf.write_bytes(b"%PDF fake") + + with patch("httpx.Client") as mock_client_cls: + mock_client = MagicMock() + mock_client_cls.return_value.__enter__.return_value = mock_client + mock_client.post.return_value = _mock_post("raw xml") + + result = extract_structure(str(pdf), grobid_url="http://localhost:8070") + + assert result == "raw xml" + mock_client.post.assert_called_once() + assert "processFulltextDocument" in mock_client.post.call_args[0][0]