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 <noreply@anthropic.com>
This commit is contained in:
@@ -84,9 +84,12 @@ def extract_references(
|
|||||||
doi_el = bib.find(f".//{_TEI_NS}idno[@type='DOI']")
|
doi_el = bib.find(f".//{_TEI_NS}idno[@type='DOI']")
|
||||||
doi = _text(doi_el)
|
doi = _text(doi_el)
|
||||||
|
|
||||||
# arXiv ID
|
# arXiv ID — GROBID emits type="arXiv" (camel-case); match case-insensitively
|
||||||
arxiv_el = bib.find(f".//{_TEI_NS}idno[@type='arxiv']")
|
arxiv_id = ""
|
||||||
arxiv_id = _text(arxiv_el)
|
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(
|
results.append(
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -2,13 +2,12 @@
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from pathlib import Path
|
||||||
from unittest.mock import MagicMock, patch
|
from unittest.mock import MagicMock, patch
|
||||||
|
|
||||||
import pytest
|
from codex.parsing.grobid import extract_references, extract_structure
|
||||||
|
|
||||||
from codex.parsing.grobid import extract_references
|
# Real GROBID TEI output uses type="arXiv" (camel-case), not "arxiv".
|
||||||
|
|
||||||
# Minimal valid TEI XML with 2 biblStruct elements
|
|
||||||
_TEI_XML = """\
|
_TEI_XML = """\
|
||||||
<?xml version="1.0" encoding="UTF-8"?>
|
<?xml version="1.0" encoding="UTF-8"?>
|
||||||
<TEI xmlns="http://www.tei-c.org/ns/1.0">
|
<TEI xmlns="http://www.tei-c.org/ns/1.0">
|
||||||
@@ -32,7 +31,7 @@ _TEI_XML = """\
|
|||||||
</imprint>
|
</imprint>
|
||||||
</monogr>
|
</monogr>
|
||||||
<idno type="DOI">10.1234/nlp.2021</idno>
|
<idno type="DOI">10.1234/nlp.2021</idno>
|
||||||
<idno type="arxiv">2101.00001</idno>
|
<idno type="arXiv">2101.00001</idno>
|
||||||
</biblStruct>
|
</biblStruct>
|
||||||
<biblStruct>
|
<biblStruct>
|
||||||
<analytic>
|
<analytic>
|
||||||
@@ -73,28 +72,36 @@ class _FakeResponse:
|
|||||||
pass
|
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:
|
class TestExtractReferences:
|
||||||
def test_returns_two_dicts(self, tmp_path: pytest.TempdirFactory) -> None:
|
def test_returns_two_dicts(self, tmp_path: Path) -> None:
|
||||||
pdf = tmp_path / "paper.pdf" # type: ignore[operator]
|
pdf = tmp_path / "paper.pdf"
|
||||||
pdf.write_bytes(b"%PDF fake")
|
pdf.write_bytes(b"%PDF fake")
|
||||||
|
|
||||||
with patch("httpx.Client") as mock_client_cls:
|
with patch("httpx.Client") as mock_client_cls:
|
||||||
mock_client = MagicMock()
|
mock_client = MagicMock()
|
||||||
mock_client_cls.return_value.__enter__.return_value = mock_client
|
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")
|
refs = extract_references(str(pdf), grobid_url="http://localhost:8070")
|
||||||
|
|
||||||
assert len(refs) == 2
|
assert len(refs) == 2
|
||||||
|
|
||||||
def test_all_keys_present(self, tmp_path: pytest.TempdirFactory) -> None:
|
def test_all_keys_present(self, tmp_path: Path) -> None:
|
||||||
pdf = tmp_path / "paper.pdf" # type: ignore[operator]
|
pdf = tmp_path / "paper.pdf"
|
||||||
pdf.write_bytes(b"%PDF fake")
|
pdf.write_bytes(b"%PDF fake")
|
||||||
|
|
||||||
with patch("httpx.Client") as mock_client_cls:
|
with patch("httpx.Client") as mock_client_cls:
|
||||||
mock_client = MagicMock()
|
mock_client = MagicMock()
|
||||||
mock_client_cls.return_value.__enter__.return_value = mock_client
|
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")
|
refs = extract_references(str(pdf), grobid_url="http://localhost:8070")
|
||||||
|
|
||||||
@@ -102,14 +109,14 @@ class TestExtractReferences:
|
|||||||
for ref in refs:
|
for ref in refs:
|
||||||
assert required_keys == set(ref.keys()), f"Missing keys in {ref}"
|
assert required_keys == set(ref.keys()), f"Missing keys in {ref}"
|
||||||
|
|
||||||
def test_first_ref_values(self, tmp_path: pytest.TempdirFactory) -> None:
|
def test_first_ref_values(self, tmp_path: Path) -> None:
|
||||||
pdf = tmp_path / "paper.pdf" # type: ignore[operator]
|
pdf = tmp_path / "paper.pdf"
|
||||||
pdf.write_bytes(b"%PDF fake")
|
pdf.write_bytes(b"%PDF fake")
|
||||||
|
|
||||||
with patch("httpx.Client") as mock_client_cls:
|
with patch("httpx.Client") as mock_client_cls:
|
||||||
mock_client = MagicMock()
|
mock_client = MagicMock()
|
||||||
mock_client_cls.return_value.__enter__.return_value = mock_client
|
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")
|
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["doi"] == "10.1234/nlp.2021"
|
||||||
assert first["arxiv_id"] == "2101.00001"
|
assert first["arxiv_id"] == "2101.00001"
|
||||||
|
|
||||||
def test_second_ref_missing_arxiv_is_empty_string(
|
def test_second_ref_missing_arxiv_is_empty_string(self, tmp_path: Path) -> None:
|
||||||
self, tmp_path: pytest.TempdirFactory
|
pdf = tmp_path / "paper.pdf"
|
||||||
) -> None:
|
|
||||||
pdf = tmp_path / "paper.pdf" # type: ignore[operator]
|
|
||||||
pdf.write_bytes(b"%PDF fake")
|
pdf.write_bytes(b"%PDF fake")
|
||||||
|
|
||||||
with patch("httpx.Client") as mock_client_cls:
|
with patch("httpx.Client") as mock_client_cls:
|
||||||
mock_client = MagicMock()
|
mock_client = MagicMock()
|
||||||
mock_client_cls.return_value.__enter__.return_value = mock_client
|
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")
|
refs = extract_references(str(pdf), grobid_url="http://localhost:8070")
|
||||||
|
|
||||||
@@ -137,3 +142,20 @@ class TestExtractReferences:
|
|||||||
assert second["arxiv_id"] == ""
|
assert second["arxiv_id"] == ""
|
||||||
assert second["title"] == "Attention Is All You Need"
|
assert second["title"] == "Attention Is All You Need"
|
||||||
assert second["year"] == "2017"
|
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("<TEI>raw xml</TEI>")
|
||||||
|
|
||||||
|
result = extract_structure(str(pdf), grobid_url="http://localhost:8070")
|
||||||
|
|
||||||
|
assert result == "<TEI>raw xml</TEI>"
|
||||||
|
mock_client.post.assert_called_once()
|
||||||
|
assert "processFulltextDocument" in mock_client.post.call_args[0][0]
|
||||||
|
|||||||
Reference in New Issue
Block a user