Skip to content

Add BioC and Europe PMC full-text providers - #99

Open
cmungall wants to merge 2 commits into
mainfrom
claude/enhance-fulltext-strategies-1nIPR
Open

cmungall wants to merge 2 commits into
mainfrom
claude/enhance-fulltext-strategies-1nIPR

Conversation

@cmungall

@cmungall cmungall commented Oct 3, 2026

Copy link
Copy Markdown
Member

Summary

Adds two new full-text providers to expand article text retrieval capabilities: BioC XML provider for structured paragraph-level text from PMC Open Access subset, and Europe PMC provider for fetching full-text XML from open access articles.

Key Changes

  • New BioC Provider (src/linkml_reference_validator/etl/fulltext/bioc.py):

    • Fetches clean paragraph-level text from NCBI BioNLP BioC API
    • Parses BioC XML format to extract passage text
    • Requires PMID and article must be in PMC Open Access subset
    • Registered with FullTextProviderRegistry as "bioc"
  • New Europe PMC Provider (src/linkml_reference_validator/etl/fulltext/epmc.py):

    • Queries Europe PMC search API to find open access articles by PMID
    • Fetches full-text XML from Europe PMC fullTextXML endpoint
    • Parses JATS XML format to extract body paragraphs
    • Captures license information from search results
    • Registered with FullTextProviderRegistry as "epmc"
  • Updated Provider Registry (src/linkml_reference_validator/etl/fulltext/__init__.py):

    • Imports new providers to ensure registration
    • Alphabetically sorted imports and exports for consistency
  • Comprehensive Tests (tests/test_fulltext_providers.py):

    • Tests for provider name, PMID requirement, successful fetches, and error handling
    • Tests for provider registration in the registry
    • Mocked HTTP responses to avoid external API calls

Implementation Details

  • Both providers implement rate limiting via config.rate_limit_delay
  • Both require minimum text length (MIN_FULLTEXT_CHARS) to return results
  • XML parsing uses BeautifulSoup with appropriate parsers (xml for both)
  • Comprehensive logging for debugging API failures and parsing issues
  • Docstrings with examples for key methods

https://claude.ai/code/session_01PYbfu1JPi9rr7v5XU6qbJj

claude added 2 commits October 3, 2026 18:32
Implement multiple strategies for pulling fulltext from publications:
- BioC XML API (NCBI BioNLP) for cleanest structured text
- Europe PMC for wider open access coverage
- Unpaywall API for finding OA versions via DOI
- Identifier conversion utilities (DOI <-> PMID <-> PMCID)

Integrate strategies into PMIDSource and DOISource with fallback:
1. BioC XML (fastest, cleanest)
2. Europe PMC (broader coverage)
3. Traditional PMC XML/HTML

Based on patterns from aurelian and artl-mcp repositories.
Add BioCFullTextProvider and EuropePMCFullTextProvider as proper
FullTextProvider implementations following the existing architecture.

- bioc.py: Fetches structured text from NCBI BioNLP BioC XML API
- epmc.py: Fetches fulltext XML for OA articles from Europe PMC
  (complements epmc_preprint.py which handles preprints only)

Remove fulltext_strategies.py which bypassed the provider chain by
fetching fulltext inside source classes. The provider architecture
uses _enrich_with_full_text() in reference_fetcher.py to call
providers after sources return content.

Revert doi.py and pmid.py to main branch versions - fulltext
enrichment happens via the provider chain, not inside sources.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PYbfu1JPi9rr7v5XU6qbJj
@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @cmungall's task in 1m 21s —— View job


Review: Add BioC and Europe PMC full-text providers

  • Read diff and surrounding provider code (pmc.py, epmc_preprint.py, base.py, reference_fetcher.py, models.py)
  • Run tests: not done. uv run pytest / mypy need approval in this runner, so I couldn't run them. Please confirm CI is green.
  • Post review

Both providers follow the existing pattern closely: they use registry registration, the shared MIN_FULLTEXT_CHARS floor, rate_limit_delay, and oa_status="green" like pmc. They're small and easy to read. I found one probable bug, one correctness risk that is specific to a validator, and a few gaps in integration and tests.


🔴 Probable bug: the Europe PMC fullTextXML URL may be wrong

epmc.py:28 and epmc.py:131-132:

EUROPEPMC_FULLTEXT_URL = ".../rest/{source}/{id}/fullTextXML"
pmcid_clean = pmcid.replace("PMC", "")
url = EUROPEPMC_FULLTEXT_URL.format(source="PMC", id=pmcid_clean)
# -> .../rest/PMC/123456/fullTextXML

As far as I know, the documented Europe PMC endpoint is GET /rest/{PMCID}/fullTextXML with the prefix kept, e.g. .../rest/PMC3257301/fullTextXML. The /{source}/{id}/... form is used for references, citations and similar endpoints, not for fullTextXML. If that's right, every real request returns a non-200, so the provider silently returns None. The mocked test can't catch this because it never checks the URL. I couldn't check the live endpoint from this runner, so please try one request by hand.

Suggested fix: url = f"https://www.ebi.ac.uk/europepmc/webservices/rest/{pmcid}/fullTextXML". Make sure pmcid has the PMC prefix (the search API returns it with the prefix). Also add a test that asserts on mock_get.call_args_list[1].

Fix this →

🟠 Correctness risk: BioC output includes the reference list (and other non-body passages)

bioc.py:107-115 joins every <passage>. BioC documents include passages whose <infon key="section_type"> is REF, which holds the bibliography (titles of other papers), plus TABLE, COMP_INT, AUTH_CONT and similar. In a supporting-text validator this can cause false positives. A snippet quoting a cited paper's title, or a claim worded like one, would "validate" against the citing paper. I'd filter out at least REF (and probably COMP_INT, AUTH_CONT, ACK_FUND), and add a test with a REF passage that is excluded.

Fix this →

A smaller issue of the same kind is in epmc.py:175. body.find_all("p") is recursive, and JATS allows <p><list><list-item><p>…</p></list-item></list></p>, so nested paragraphs get emitted twice. That's harmless for matching but bloats the cache. Collecting only <p> elements with no <p> ancestor inside body (or using body.get_text("\n\n")) avoids it. Also, <sec><title> headings are dropped, which is fine but worth knowing.

🟡 Integration and docs: the providers can be reached but nobody will find them

  • They aren't in the default chain (models.py:540: ["pmc", "epmc_preprint", "unpaywall", "openalex"]). That's a reasonable conservative choice, but then users have to know they exist:
    • The full_text_providers field description in models.py:543 lists the built-ins and should name bioc and epmc.
    • docs/how-to/fetch-full-text-and-pdfs.md (provider list, config table, full_text_provider values) doesn't mention them.
  • base.py has a long Scope comment explaining that pmc and epmc_preprint deliberately return green with no access_type, and so write to the public cache. These two new providers have the same behaviour (green, no access_type). They fall under the same "archive's own API" reasoning, so the comment should list them. Otherwise the next reader may see them as an accidental hole in the licence policy. The same applies to docs/troubleshooting.md:818.
  • Name clash: epmc vs epmc_preprint vs the pmc provider is a little confusing. epmc also overlaps heavily with pmc (same PMCID, same JATS). Its main value is probably as a fallback when NCBI is down or rate-limiting. A sentence in the docstring or docs saying when to prefer it would help.

🟡 Code quality and project conventions

  • try/except Exception around BeautifulSoup(...) (bioc.py:100-104, epmc.py:164-168). CLAUDE.md asks to avoid try/except around deterministic code. BeautifulSoup with the xml parser recovers from malformed input instead of raising, so these blocks are effectively dead and hide real bugs. I'd remove them. The requests and .json() handling is at the external boundary and is fine. Note, though, that epmc_preprint.py doesn't catch these and leaves it to the chain-level except in reference_fetcher.py:755. Catching here and returning None means a network error looks like "no full text" instead of setting had_error. That matters: full_text_attempted gets set on a clean miss, and the chain is then never retried (reference_fetcher.py comment around L800). A transient timeout in bioc/epmc would therefore be recorded as a permanent absence, but only if this provider is the last one tried. I'd let RequestException propagate, as epmc_preprint does, and for 5xx responses either raise or response.raise_for_status(). Keep returning None only for 404 and "not OA".
  • Inconsistent floor: pmc.py uses len(text) > MIN_FULLTEXT_CHARS while these use < MIN_FULLTEXT_CHARS (accepting ==). That's trivial, but worth matching.
  • format_hint="text" while pmc uses "xml" for the same JATS-derived text. Check that _apply_full_text_location treats "text" the intended way for the cache-version stamps (XML_EXTRACTION_CACHE_VERSION / HTML_FULL_TEXT_CACHE_VERSION).
  • locate() on both classes has no docstring (CLAUDE.md: "use docstrings"). _find_oa_result would make a nice doctest.

🟢 Performance

That's fine. epmc sleeps rate_limit_delay twice (search + fetch), which is correct. timeout=30 matches the other providers. Requests aren't pooled (no Session), the same as the rest of the codebase.

🔒 Security

There are no concerns. Inputs are PMIDs and PMCIDs placed in fixed HTTPS hosts. params= handles query encoding for the search. One nit: ids.pmid goes straight into the BioC URL path. PMIDs are validated upstream, but quote(ids.pmid) costs nothing.

🧪 Test coverage

The happy and sad paths are present, but these are missing:

  • An assertion on the requested URLs (it would have caught the EPMC URL issue above).
  • BioC: non-200/5xx, RequestException, an XML error body with a 200 status (the BioC API returns a plain-text error with 200 for unknown IDs, so check that it yields None), a short-text rejection, and REF passage exclusion.
  • EPMC: OA result missing pmcid, a fullTextXML 404, XML without <body>, an empty resultList, and that license reaches FullTextLocation.license.
  • A fetcher-level test that full_text_providers=["bioc"] / ["epmc"] works end to end through _enrich_with_full_text (still mocked at requests.get).
  • The function-local imports in the tests are unnecessary, since __init__ already imports both modules. Module-level imports would match the registry tests.

Summary: the structure is solid. Before merging, I'd (1) check and fix the Europe PMC fullTextXML URL, (2) exclude REF passages from BioC, and (3) stop turning transient network errors into a permanent "no full text". Docs, config description and test gaps can be done in the same PR or a follow-up.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants