From 467a8d280d01f1cb07b971439f6821b3055a91bf Mon Sep 17 00:00:00 2001 From: Chris Mungall Date: Wed, 16 Sep 2026 23:16:57 -0700 Subject: [PATCH] Fetch only openly-licensed files, and never demote a cache entry Three changes, one policy: material a user may lawfully read is not automatically material a project may redistribute, and a checked-in reference cache is redistribution. That is the contract PR #61 drew for Zotero private libraries; OpenAlex and Unpaywall were not honouring it. Do not scrape landing pages. Both providers used `pdf_url or landing_url`, so a record with no file fell back to the article *page* and downloaded it. Hosts refuse this: PMC answers a rate-limited request with a reCAPTCHA interstitial served on an HTTP 200, so no status check downstream can tell it from article text. Measured on PMID:9177246, the same fetch returned real full text on 2 of 5 consecutive attempts; the other 3 cached the bot page's rejection as "no full text exists". Providers now return None when they have only a page. Bronze is not open. `oa_status: bronze` means free to read on the publisher's site under no open licence. New `access_type_for_oa_status` maps gold/diamond/ hybrid to "open" and everything else -- bronze, unrecognised, missing -- to "publisher_free". Nothing else was needed: `_enrich_with_full_text` already skips non-open locations. Unknown statuses fail closed, because an unknown licence is not a grant. A declined location is not an absence. full_text_attempted means "a clean run concluded no full text is available", and _maybe_retry_full_text never runs the chain again once it is set -- so folding a licence decline or a refused landing page into it would record a decision this project made as a fact about the article, which is this change's own thesis one layer up. Bronze-to-gold is a conversion publishers make, and a page-only DOI gains a repository PDF; neither would ever be looked at again. Both now record a `full_text_declined` reason instead of the absence flag. Recording rather than leaving nothing is the difference between retryable and retried: a decline never clears the way a transient error does, so an unset flag would re-walk the four-provider chain on every run for every such reference -- measured on one real corpus, 23,465 eligible entries and roughly thirteen hours of rate-limit sleep per run, aimed at the hosts whose rate limiting produces the interstitial this change defends against. The decline is re-examined when the entry is next re-fetched. The same laundering is fixed in PMC, which is the host in the original report: a 429 and a swallowed elink exception both returned None, which the chain read as a clean absence, so an article with a PMC body fetched during a rate-limit window was permanently recorded as having none. Both now raise into the existing had_error path; the undecidable 200-with-no-body case is unchanged. FullTextLocation gained a `declined` field so a provider can say it refused a candidate rather than returning a bare None, which the chain cannot tell from finding nothing; a declined location carries no url or text and is never fetched. The licence rule governs files found by asking an OA index. It deliberately does not govern the pmc and epmc_preprint providers, which ask an archive's own API for a document it serves for machine retrieval -- a stronger warrant than an index's summary of a third-party host, and routing them through it would decline most PMID full text for want of a licence field their API does not return. The same green deposit can be declined from OpenAlex and admitted from PMC; the asymmetry is recorded rather than left implicit. One page fetch also survives, in the pmc provider, and is safe for a reason the oa_url fallback never had: it returns text only from a div.article-body or div.tsec, which an interstitial does not carry. Green is decided by its licence, not by the status. The status names a repository rather than a permission -- a PMC author manuscript is free to read under a funder policy whose redistribution terms vary by publisher, which is the same shape of claim as bronze with a different host. A green location is open when it states a licence and not when it merely states its address. "States a licence" is an allowlist, not a non-empty field: OpenAlex's vocabulary includes other-oa (5.9M works) and publisher-specific-oa, and Unpaywall documents implied-oa, all of which are truthy strings naming the *absence* of a licence statement -- and a bare funder-policy repository deposit is exactly where they appear, so a truthiness test would reopen the route this rule closes. The source URL is recorded for an allowlist of access types (unset, open, publisher_free) rather than suppressed for a denylist, so an access type this version has not heard of stays suppressed -- the same fail-closed default this change applies to an unknown oa_status and an unknown licence. A non-open access type no longer discards the source URL. That rule was written for a private-library endpoint, where a localhost Zotero attachment address is session-specific and meaningless elsewhere; a publisher link is a stable public address, and what licensing keeps out of the public cache is the text rather than where it came from. A known miss is recorded in the docstring: a work's oa_status is its best across locations, so a work marked bronze may carry an openly-licensed repository copy this code never inspects. A refresh may improve an entry; it may never demote one. Without this the first change would itself destroy data: the #62 extractor stamp makes every pre-stamp entry stale, so the next run re-fetches, and with scraping removed those entries would come back abstract-only and overwrite themselves. `_stale_fallback` does not cover it -- that fires only when the source yields nothing, and here the abstract is something. `_preserve_cached_full_text` refuses a write that comes back with no full text at all, leaves the entry stale so a later run retries, and steps aside for `force_refresh`, which warns before it discards. The rule is about kind, not size. An earlier revision of this branch also refused a refresh whose text was a fraction of the cached length, to catch a PDF whose text layer is a publisher cover sheet. It caught that one case and mis-handled four others -- a scraped page replaced by a clean XML body, by a clean HTML body, a plain-text API body replaced by XML, and a re-extraction that merely trimmed a trailing section -- because a shorter extraction is usually a better one and a length comparison cannot tell those apart. Every one of those failed in the worse direction: a refused entry is never written, so it is never stamped, so every later run re-fetched and re-refused it. Judging whether text *is* an article belongs in the acceptance layer, alongside is_stub_notice, where a wrong answer costs one skipped fetch rather than a cache that can never migrate. The cover-page gap is recorded by a test rather than left implicit. Not refusing on size is not the same as not mentioning it. A refusal is permanent -- never written, never stamped, re-refused every run -- while a log line lets the write proceed and costs one line if it is wrong. So a refresh that keeps full text but returns a small fraction of the cached length is reported and written, which restores the observability the ratio provided incidentally without restoring the harm it caused. It is reported only on the branch that writes, since a report of a write is false anywhere else, and it measures the text after the abstract both records carry, since a median abstract alone clears a fifth of a short article. Every message quotes that same subtracted length, so the figures a reader sees are the figures the threshold compared. Keeping an entry and serving it are separate questions, and the method asks both. Whether it may be overwritten is asked with allow_stale_html=True, because a stale HTML entry is still somebody's data. Whether its text may be served as evidence is asked without that bypass, so _load_from_disk's refusal of stale HTML -- "it may be a repository landing page" -- still holds. A pre-fix entry scraped from a landing page is therefore kept on disk and withheld from validation, which falls back to the freshly fetched abstract. Built on #84: the guard returns a FetchOutcome, and both of its branches set served_stale. That command's message now covers both paths -- it could not be re-fetched, or the refresh was refused -- rather than naming only the first, which was untrue in the new branch. A preserved entry makes it exit 1 on every run until --force, which the docs now say. Verified end to end on a real cache entry: an 18,464-character `full_text_html` record survives a refresh that returns only an abstract, the file on disk is untouched, and the loss is a WARNING rather than a silent rewrite. Tests: 17 new across two files, written before the implementation. `test_locate_falls_back_to_oa_url` asserted the scraping this removes; it is inverted rather than deleted, so the file records why a landing page is not a located full text. Co-Authored-By: Claude Opus 5 (1M context) --- docs/troubleshooting.md | 118 ++- src/linkml_reference_validator/cli/cache.py | 35 +- .../etl/fulltext/base.py | 130 ++++ .../etl/fulltext/openalex.py | 56 +- .../etl/fulltext/pmc.py | 36 +- .../etl/fulltext/unpaywall.py | 34 +- .../etl/reference_fetcher.py | 384 +++++++++- src/linkml_reference_validator/models.py | 26 +- tests/test_cli_cache_reference.py | 72 +- tests/test_fulltext_providers.py | 23 +- tests/test_no_cache_downgrade.py | 526 ++++++++++++++ tests/test_open_access_policy.py | 686 ++++++++++++++++++ 12 files changed, 2064 insertions(+), 62 deletions(-) create mode 100644 tests/test_no_cache_downgrade.py create mode 100644 tests/test_open_access_policy.py diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 611635b..986416a 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -791,6 +791,60 @@ Run through this checklist when encountering issues: - [How to Repair Validation Errors](how-to/repair-validation-errors.md) - Fixing common issues - [GitHub Issues](https://github.com/linkml/linkml-reference-validator/issues) - Report bugs +### Which full text is fetched, and which is not + +Two rules govern what reaches the public reference cache. Both follow from the +contract the Zotero provider established: material you may lawfully *read* is +not automatically material a project may *redistribute*, and a checked-in cache +is redistribution. + +**A landing page is not full text.** A provider returns a location only for a +file — OpenAlex's `pdf_url`, Unpaywall's `url_for_pdf`. A record whose only +location is an article *page* yields nothing, and the record stays +`abstract_only`. Downloading such a page is scraping, and hosts refuse it: +PMC answers a rate-limited request with a reCAPTCHA interstitial served on an +HTTP 200, which no status check downstream can distinguish from article text. + +**Bronze open access is not redistributable.** `oa_status: bronze` means the +publisher has made an article free to read on its own site under no open +licence. Bronze locations are marked with a non-open `access_type` and are +skipped by ordinary validation, the same route private-library material takes. +`gold`, `diamond` and `hybrid` are treated as open; `green` only when the +location states a licence, since that status names a repository rather than a +permission. An unrecognised or missing status is **not** assumed open: an +unknown licence is not a grant. + +This governs files found by asking an OA *index* (OpenAlex, Unpaywall). It does +not govern the `pmc` and `epmc_preprint` providers, which ask an archive's own +API for a document it serves for machine retrieval — a stronger warrant than an +index's summary of a third-party host. The same green deposit can therefore be +declined from OpenAlex and admitted from PMC. + +**A declined reference is retried when the extractor version moves, not on every +run.** Declining is a decision, not a finding, so it must not be recorded as +`full_text_attempted` — that flag means a clean run concluded no full text +exists, and it stops the provider chain running again. But leaving nothing +recorded is its own problem: a decline never clears the way a transient failure +does, so every run would re-walk the whole chain for every bronze or +page-only reference. On one real corpus that is tens of thousands of entries and +hours of rate-limit sleep per run, aimed at the hosts whose rate limiting causes +the interstitial in the first place. So the decline is recorded as +`full_text_declined: ` in the cache entry, and re-examined when the entry +is next re-fetched. + +If you previously relied on landing-page or bronze full text, those references +now resolve as `abstract_only`. Excerpts quoted from that text will no longer +verify. Existing cache entries are not rewritten — see the refresh note below. + +**`cache enrich` is unaffected in where it writes.** That command has always +passed `private=True`, so every file it fetches already went to the private +research cache (`~/.cache/linkml-reference-validator/private` by default), +whatever its licence. What changes on that path is the frontmatter: an enriched +bronze entry now records `full_text_access_type: publisher_free`. Its +`full_text_url` is still recorded — a publisher link is a stable public address, +and it is the *text* that licensing keeps out of the public cache, not the +address. + ### JATS tables and XML cache refresh JATS/PMC XML extraction appends pipe-delimited tables after the existing body @@ -828,11 +882,65 @@ XML remains available with the existing stale-cache warning and is not rewritten it may still lack table rows. A later process retries the refresh. Existing stale HTML rejection remains unchanged. -A successful source refresh that returns only an abstract can replace the old -full text if no provider supplies a body. This is existing refresh behavior; -the stale fallback applies when the source returns no record, not when it -returns an abstract-only record. Keep a backup if retaining older full text is -necessary. +**A cache-wide refresh adds one line to every enriched entry.** Both index +providers now set `access_type` where they previously left it unset, so a +refreshed public entry gains a `full_text_access_type: open` line. It means the +same as its absence, but it does show up as a one-line diff on every such entry +— worth knowing before reviewing that commit. + +**`cache reference` exits 1 on a preserved entry, every run.** A preserved entry +is never written, so it is never stamped, so the next run reaches the same +decision and fails again. A script that caches a list of references and gates on +the exit status will keep failing on those until the source serves full text +again, or `--force` accepts the abstract-only refresh in its place. That is the guard working as +intended, but it is the kind of thing that gets diagnosed twice if it is not +written down. + +A successful source refresh that returns only an abstract **no longer replaces** +cached full text. Full-text retrieval fails transiently and silently — a +rate-limited PMC request is answered with a reCAPTCHA interstitial carried on an +HTTP 200 — so an abstract-only refresh is not evidence that the article has no +full text. When a refresh loses full text the cached entry is kept, a warning +names it, and the entry stays stale so a later run tries again. + +Losing it means coming back with *no* full text — an `abstract_only`, +`unavailable` or `summary` record where the cache holds `full_text_*`. Sizes are +not compared **to decide a refusal**. An earlier version of this guard also refused a refresh whose text +was a fraction of the cached length, to catch a PDF whose text layer is a +publisher cover sheet; it caught that, and wrongly refused four kinds of genuine +improvement — a scraped page replaced by a clean XML body, by a clean HTML body, +a plain-text API body replaced by XML, and a re-extraction that merely trimmed a +trailing section. A shorter extraction is usually a better one, and a length +comparison cannot tell those apart. + +Wrongly refusing is the worse error: a refused entry is never written, so it is +never stamped, so every later run re-fetches and re-refuses it. Judging whether +text *is* an article belongs in the acceptance layer, where a wrong answer costs +one skipped fetch instead of a cache that can never migrate. + +Size is still *reported*, which has none of those properties. When a refresh +keeps full text but returns under a fifth as much article text as the cache +held, the entry is written as usual and a warning names both figures: + +``` +Refresh of PMID:9177246 replaced the cached full_text_html entry (18,464 +characters of article text) with a much shorter one: 602 characters of +full_text_pdf. Written as usual, since a shorter extraction is often a cleaner +one — but check it if quoted excerpts stop verifying. +``` + +Usually that is a cleaner extraction and there is nothing to do. It is the +thread to pull when a quoted excerpt stops verifying: re-read the cached entry, +and if the text is a publisher cover sheet rather than the article, re-fetch when +the source will serve the real thing. The lengths quoted are of the article text, +with the abstract both records carry subtracted. + +A refresh that *finds* full text still rewrites the entry. `force_refresh` +(`--force`) overrides the refusal, but not the notice: it logs a warning naming +the cached entry it is about to replace and its length, because it is the remedy +this tool recommends and following that advice should not quietly discard an +article body. This is narrower than the stale fallback, which applies +only when the source returns no record at all. This pass targets JATS `table-wrap` content and searches the whole document; tables and notes inside embedded `sub-article` or `response` elements are diff --git a/src/linkml_reference_validator/cli/cache.py b/src/linkml_reference_validator/cli/cache.py index 9cb0a44..164f5dd 100644 --- a/src/linkml_reference_validator/cli/cache.py +++ b/src/linkml_reference_validator/cli/cache.py @@ -150,19 +150,24 @@ def reference_command( outcome = fetcher.fetch_with_provenance(reference_id, force_refresh=force) - # A reference that could not be re-fetched falls back to an out-of-date cache - # entry, which is the right answer for validation but not here. What this - # command promises is that the cache holds a current entry afterwards - not - # that it downloaded one, since an entry the current extractor already wrote - # needs no download. A stale entry leaves that promise unmet, so reporting - # success would take a script that gates on the exit status green through an - # outage. The reason is deliberately left open: the source may be unreachable, - # or no source may handle this identifier at all. + # What this command promises is that the cache holds a current entry + # afterwards - not that it downloaded one, since an entry the current + # extractor already wrote needs no download. Two paths leave that promise + # unmet, and the message has to be true of both: the reference could not be + # re-fetched and an out-of-date entry was served instead, or it was + # re-fetched and the result was refused for holding less full text than the + # entry already cached. Either way the write was skipped, so what is + # reported is that, rather than a guess at which path ran. Reporting success + # would take a script that gates on the exit status green through an outage. if outcome.served_stale: typer.echo( - f"Failed to cache {reference_id}: it could not be re-fetched, so an " - "out-of-date cache entry was served. The cache still holds no current " - "entry for it.", + f"Failed to cache {reference_id}: it could not be re-fetched, or the " + "refresh came back with no full text where the cache holds some. " + "Either way no entry was written, so the cache still holds no current " + "entry for it. Re-run when the source serves full text again. " + "(--force replaces cached text with whatever a refresh returns, so it " + "resolves the second case and not the first: with the source " + "unreachable there is nothing to put in its place.)", err=True, ) raise typer.Exit(1) @@ -303,6 +308,14 @@ def enrich_command( typer.echo(f"{reference.reference_id}\tnot_found\t-") continue + if location.declined: + # Reported as its own outcome, not as a find and not as an absence. + # A declined location carries no url and no text, so counting it + # would inflate `Found:` by exactly the references the provider + # refused -- and this command's whole output is an inventory. + typer.echo(f"{reference.reference_id}\tdeclined\t{location.declined}") + continue + found += 1 source = f"{location.provider or provider}:{location.source_item_id or '-'}" if dry_run: diff --git a/src/linkml_reference_validator/etl/fulltext/base.py b/src/linkml_reference_validator/etl/fulltext/base.py index d97e665..68c108d 100644 --- a/src/linkml_reference_validator/etl/fulltext/base.py +++ b/src/linkml_reference_validator/etl/fulltext/base.py @@ -17,6 +17,136 @@ logger = logging.getLogger(__name__) +#: ``oa_status`` values that are openly licensed on their own. +#: +#: ``bronze`` is deliberately absent. It means the publisher has made the article +#: free to read on its own site under no open licence: readable, not +#: redistributable. Treating "I may read this" as "this project may republish +#: this" is the distinction PR #61 drew for Zotero private libraries, and it +#: applies identically to a bronze PDF. +#: +#: ``green`` is absent too, and for a subtler reason: it describes *where* a copy +#: lives -- a repository -- not the terms it lives there under. A green PMC +#: author manuscript is free to read under a funder policy whose redistribution +#: terms vary by publisher, which is the same shape of claim as bronze with a +#: different host. It is admitted by :func:`access_type_for_oa_status` only when +#: the location states a licence. +#: +#: **Scope.** This governs a file found by asking an OA *index* -- OpenAlex or +#: Unpaywall -- where the index's own status is all that is known about the +#: terms. It deliberately does not govern ``pmc`` or ``epmc_preprint``, which +#: return ``oa_status="green"`` with no ``access_type`` and so continue to write +#: to the public cache. Those providers ask the archive's own API for a document +#: it serves for machine retrieval, which is a stronger warrant than an index's +#: summary of a third-party host, and routing them through this rule would +#: decline most PMID full text for want of a licence field their API does not +#: return. The asymmetry is deliberate: the same green deposit can be declined +#: from OpenAlex and admitted from PMC. +SELF_EVIDENTLY_OPEN_OA_STATUSES = frozenset({"gold", "diamond", "hybrid"}) + +#: ``oa_status`` values that are open *if* the location states a licence. See +#: the note on ``green`` above. +LICENCE_DEPENDENT_OA_STATUSES = frozenset({"green"}) + +#: Licence values outside the ``cc-`` family that grant redistribution. +OPEN_LICENCES = frozenset({"cc0", "public-domain", "mit"}) + + +def states_a_licence(licence: Optional[str]) -> bool: + """Report whether a location's ``license`` field grants redistribution. + + An **allowlist**, matching how this module treats ``oa_status``: a value it + has not heard of is an unknown licence, and an unknown licence is not a + grant. A truthiness test would not do, because neither API uses ``None`` as + its only way of saying "no licence statement". OpenAlex's vocabulary + includes ``other-oa`` (5.9M works) and ``publisher-specific-oa``, and + Unpaywall documents ``implied-oa`` for a copy it believes free with no + licence statement found. All three are truthy strings naming the *absence* + of a licence -- and a bare funder-policy repository deposit, which is the + case the ``green`` rule was written about, is exactly where they appear. + + Every Creative Commons variant qualifies. ``nc`` restricts commercial use + and ``nd`` restricts derivatives; neither restricts holding a verbatim copy, + which is all a cache does. + + Examples: + >>> states_a_licence("cc-by") + True + >>> states_a_licence("cc-by-nc-nd") + True + >>> states_a_licence("public-domain") + True + + The sentinels that mean "no licence statement" do not: + + >>> states_a_licence("other-oa") + False + >>> states_a_licence("implied-oa") + False + >>> states_a_licence(None) + False + """ + value = (licence or "").strip().lower() + return value.startswith("cc-") or value in OPEN_LICENCES + + +#: ``access_type`` for a location that is free to read but not openly licensed. +#: Any non-``open`` value is skipped by ``_enrich_with_full_text``; naming it +#: distinctly keeps the reason legible in a log line. +PUBLISHER_FREE_ACCESS = "publisher_free" + + +def access_type_for_oa_status( + oa_status: Optional[str], licence: Optional[str] = None +) -> str: + """Map an ``oa_status`` (and licence, where it decides) to an ``access_type``. + + Unrecognised and missing statuses are **not** treated as open. A status this + version has not heard of is an unknown licence, and the safe reading of an + unknown licence is that it does not grant redistribution. + + ``green`` is decided by the licence rather than the status, because the + status names a repository rather than a permission: a deposit that states + its licence is open, a bare one is not. "States its licence" means + :func:`states_a_licence`, not merely a non-empty field -- see there for why + the difference matters. + + Known miss: a work's ``oa_status`` is its *best* status across locations, so + a work marked ``bronze`` may still carry an openly-licensed repository copy + in a location this code never inspects. Conservative rather than wrong -- + some redistributable full text is skipped -- and widening it means walking + every location instead of the best one. + + Examples: + >>> access_type_for_oa_status("gold") + 'open' + >>> access_type_for_oa_status("bronze") + 'publisher_free' + >>> access_type_for_oa_status(None) + 'publisher_free' + >>> access_type_for_oa_status("something-new") + 'publisher_free' + + A repository deposit is open when it states its terms, and not when it + merely states its address: + + >>> access_type_for_oa_status("green", licence="cc-by") + 'open' + >>> access_type_for_oa_status("green") + 'publisher_free' + + A licence does not rescue a status that is not licence-dependent: + + >>> access_type_for_oa_status("bronze", licence="cc-by") + 'publisher_free' + """ + status = (oa_status or "").strip().lower() + if status in SELF_EVIDENTLY_OPEN_OA_STATUSES: + return "open" + if status in LICENCE_DEPENDENT_OA_STATUSES and states_a_licence(licence): + return "open" + return PUBLISHER_FREE_ACCESS + class FullTextProvider(ABC): """Abstract base class for full-text providers.""" diff --git a/src/linkml_reference_validator/etl/fulltext/openalex.py b/src/linkml_reference_validator/etl/fulltext/openalex.py index d6f385d..64c37f7 100644 --- a/src/linkml_reference_validator/etl/fulltext/openalex.py +++ b/src/linkml_reference_validator/etl/fulltext/openalex.py @@ -1,6 +1,9 @@ """OpenAlex full-text provider. -Looks up open-access locations for a DOI via the OpenAlex works API. +Looks up an openly-licensed full-text *file* for a DOI via the OpenAlex works +API. A record whose only location is a landing page yields nothing: see +:func:`~linkml_reference_validator.etl.fulltext.base.access_type_for_oa_status` +and ``locate`` below. """ import logging @@ -17,6 +20,7 @@ from linkml_reference_validator.etl.fulltext.base import ( FullTextProvider, FullTextProviderRegistry, + access_type_for_oa_status, ) logger = logging.getLogger(__name__) @@ -24,7 +28,17 @@ @FullTextProviderRegistry.register class OpenAlexProvider(FullTextProvider): - """Locate an open-access PDF/landing page for a DOI via OpenAlex. + """Locate an open-access PDF for a DOI via OpenAlex. + + Returns a location only for a ``pdf_url``. The ``oa_url`` fallback was + removed: it is an article *page*, and downloading one is scraping. + + That constrains what is *asked for*, not what comes back -- a ``pdf_url`` + answering with HTML is sniffed by ``_materialize`` and routed to the HTML + extractor. What makes that safe is the same structural defence + ``PMCFullTextProvider._fetch_pmc_html`` relies on: ``HTMLExtractor``'s + landing-page rejection, plus the ``html_full_text_version`` stamp that marks + which entries it certified. Examples: >>> OpenAlexProvider.name() @@ -55,16 +69,38 @@ def locate( best = data.get("best_oa_location") or {} pdf_url = best.get("pdf_url") - oa_url = open_access.get("oa_url") - target = pdf_url or oa_url - if not target: - return None + if not pdf_url: + # ``oa_url`` without ``pdf_url`` is a landing/article *page*, not a + # file. Retrieving it means scraping HTML the host did not offer for + # machine retrieval, and hosts defend against exactly that: PMC + # answers with a reCAPTCHA interstitial carried on an HTTP 200, so + # no status code downstream can tell it from article text. Decline + # instead, and let the record stay abstract-only. + logger.debug( + "OpenAlex has only a landing page for DOI:%s; not scraping it", + ids.doi, + ) + # Declined, not absent. Returning a bare ``None`` here would let the + # chain record that this article has no full text, and it has one -- + # it is the route that is unacceptable, and a repository deposit can + # give it an acceptable one tomorrow. + return FullTextLocation( + declined="landing_page_only", + oa_status=open_access.get("oa_status"), + provider="openalex", + ) + # Different objects on purpose, and the mismatch access_type_for_oa_status + # calls a known miss: the status is the work's best across all locations, + # the licence belongs to this one. + oa_status = open_access.get("oa_status") + licence = best.get("license") return FullTextLocation( - url=target, - format_hint="pdf" if pdf_url else "html", - oa_status=open_access.get("oa_status"), - license=best.get("license"), + url=pdf_url, + format_hint="pdf", + oa_status=oa_status, + access_type=access_type_for_oa_status(oa_status, licence), + license=licence, version=best.get("version"), provider="openalex", ) diff --git a/src/linkml_reference_validator/etl/fulltext/pmc.py b/src/linkml_reference_validator/etl/fulltext/pmc.py index e3fee90..507dc2e 100644 --- a/src/linkml_reference_validator/etl/fulltext/pmc.py +++ b/src/linkml_reference_validator/etl/fulltext/pmc.py @@ -28,6 +28,16 @@ logger = logging.getLogger(__name__) +class TransientFullTextError(RuntimeError): + """A provider failed for a reason that says nothing about the article. + + ``_enrich_with_full_text`` already treats a raised exception as + ``had_error``, which keeps the record retryable. Raising this rather than + returning ``None`` is how a provider says "ask me again" instead of "there + is nothing here". + """ + + @FullTextProviderRegistry.register class PMCFullTextProvider(FullTextProvider): """Provide PMC full text for a reference identified by PMID/PMCID. @@ -85,7 +95,11 @@ def _resolve_pmcid(self, pmid: Optional[str], config: ReferenceValidationConfig) handle.close() except Exception as exc: # external system boundary logger.warning("Failed to link PMID:%s to PMC: %s", pmid, exc) - return None + # An elink outage is not "this PMID has no PMC copy"; see the note + # in _fetch_pmc_html for why that distinction has to survive. + raise TransientFullTextError( + f"Could not link PMID:{pmid} to PMC: {exc}" + ) from exc if isinstance(result, list) and result and isinstance(result[0], dict): link_set_db = result[0].get("LinkSetDb", []) @@ -114,12 +128,28 @@ def _fetch_pmc_xml_source( return xml_content def _fetch_pmc_html(self, pmcid: str, config: ReferenceValidationConfig) -> Optional[str]: - """Fetch full text from the PMC HTML page as a fallback.""" + """Fetch full text from the PMC HTML page as a fallback. + + This is the one page fetch the OA providers' "a landing page is not full + text" rule does not cover, and what makes it safe is the requirement + below: text is returned only from a ``div.article-body`` or ``div.tsec``. + A bot-check interstitial -- which PMC serves on an HTTP 200, so no status + check sees it -- carries neither, so this yields ``None`` rather than + caching the page. The ``oa_url`` fallback that was removed had no such + structural test; it accepted whatever came back. + """ time.sleep(config.rate_limit_delay) url = f"https://www.ncbi.nlm.nih.gov/pmc/articles/PMC{pmcid}/" response = requests.get(url, timeout=30) if response.status_code != 200: - return None + # Not an absence. PMC answers a rate-limited client with 429, and + # returning ``None`` here would let the chain record that this + # article has no full text -- the defect this release is about, in + # the host whose interstitial is its evidence. Raise so the chain's + # existing ``had_error`` path keeps the record retryable. + raise TransientFullTextError( + f"PMC returned {response.status_code} for PMC{pmcid}" + ) soup = BeautifulSoup(response.content, "html.parser") article_body = soup.find("div", class_="article-body") or soup.find("div", class_="tsec") diff --git a/src/linkml_reference_validator/etl/fulltext/unpaywall.py b/src/linkml_reference_validator/etl/fulltext/unpaywall.py index f23820b..13a06b5 100644 --- a/src/linkml_reference_validator/etl/fulltext/unpaywall.py +++ b/src/linkml_reference_validator/etl/fulltext/unpaywall.py @@ -17,6 +17,7 @@ from linkml_reference_validator.etl.fulltext.base import ( FullTextProvider, FullTextProviderRegistry, + access_type_for_oa_status, ) logger = logging.getLogger(__name__) @@ -24,7 +25,10 @@ @FullTextProviderRegistry.register class UnpaywallProvider(FullTextProvider): - """Locate an open-access PDF/landing page for a DOI via Unpaywall. + """Locate an open-access PDF for a DOI via Unpaywall. + + Returns a location only for a ``url_for_pdf``; a landing ``url`` is a page, + not a file, and downloading one is scraping. Examples: >>> UnpaywallProvider.name() @@ -54,16 +58,28 @@ def locate( return None pdf_url = best.get("url_for_pdf") - landing = best.get("url") - target = pdf_url or landing - if not target: - return None + if not pdf_url: + # See OpenAlexProvider: a landing URL is a page, not a file, and + # fetching it is scraping. + logger.debug( + "Unpaywall has only a landing page for DOI:%s; not scraping it", + ids.doi, + ) + # See OpenAlexProvider: declined, not absent. + return FullTextLocation( + declined="landing_page_only", + oa_status=data.get("oa_status"), + provider="unpaywall", + ) + oa_status = data.get("oa_status") + licence = best.get("license") return FullTextLocation( - url=target, - format_hint="pdf" if pdf_url else "html", - oa_status=data.get("oa_status"), - license=best.get("license"), + url=pdf_url, + format_hint="pdf", + oa_status=oa_status, + access_type=access_type_for_oa_status(oa_status, licence), + license=licence, version=best.get("version"), provider="unpaywall", ) diff --git a/src/linkml_reference_validator/etl/reference_fetcher.py b/src/linkml_reference_validator/etl/reference_fetcher.py index 940b7c6..298ef95 100644 --- a/src/linkml_reference_validator/etl/reference_fetcher.py +++ b/src/linkml_reference_validator/etl/reference_fetcher.py @@ -23,6 +23,7 @@ from linkml_reference_validator.etl.sources.clinicaltrials import NCT_ID_PATTERN from linkml_reference_validator.etl.acquire import ContentAcquirer, resolve_format, sniff_format from linkml_reference_validator.etl.identifiers import build_identifiers +from linkml_reference_validator.etl.fulltext.base import PUBLISHER_FREE_ACCESS from linkml_reference_validator.etl.extract import Extractor, ExtractorRegistry # noqa: F401 (registers extractors) from linkml_reference_validator.etl.extract.pdf import PDFExtractor from linkml_reference_validator.etl.extract.html import HTMLExtractor @@ -33,6 +34,29 @@ logger = logging.getLogger(__name__) +#: A refresh keeping full text but returning less than this share of the cached +#: length is reported, never refused. The distinction is the whole lesson of this +#: guard's history: refusing on size blocked four kinds of genuine improvement +#: permanently, because an entry that is never written is never stamped. A log +#: line has none of those properties -- the write proceeds, the migration +#: completes, ``cache reference`` still exits 0 -- and a wrong guess costs one +#: line rather than a cache that can never migrate. +REPORT_SHRINK_RATIO = 0.2 + +#: ``access_type`` values whose source URL may be recorded, as an allowlist so +#: an unrecognised value stays suppressed. A private endpoint -- one reached +#: through someone's own credentials or installation -- is excluded because its +#: address is meaningless elsewhere and may carry a local token; everything +#: unknown is excluded for the reason this module excludes an unknown +#: ``oa_status`` and an unknown licence, which is that it cannot vouch for it. +#: +#: Not the same question as whether the *text* may be redistributed: +#: ``PUBLISHER_FREE_ACCESS`` is listed here precisely because a publisher's link +#: is a stable public address even when its content is not openly licensed. See +#: the note at the ``full_text_url`` assignment. +PUBLIC_URL_ACCESS_TYPES = frozenset({None, "open", PUBLISHER_FREE_ACCESS}) + + NEEDS_FULL_TEXT_TYPES = { "abstract_only", "unavailable", @@ -84,6 +108,59 @@ } +def _text_after_abstract(content: Optional[str]) -> str: + """Return a record's extracted text, without the abstract prepended to it. + + ``_apply_full_text_location`` stores ``abstract + "\\n\\n" + text``, so both + the cached record and a refreshed one carry the abstract. Comparing whole + records credits a failed extraction with text it was always going to have: + a ~2,000-character abstract, the median in a real cache, by itself clears a + fifth of a 10,000-character entry. + + Split on the first blank line, which is the join that produced it *when + there was an abstract to prepend*. ``_apply_full_text_location`` writes bare + text when there is not -- a source that returned no abstract, or a record + that came straight from PMC -- and the partition then lands on the first + paragraph break in the body and drops the opening paragraph instead. + + Neither mistake is symmetric, and neither is serious. Under-subtracting on + the *fresh* side leaves it larger and so less likely to report; + under-subtracting on the *cached* side raises the bar and makes a report + more likely. Both decide a log line, never a refusal. + """ + text = content or "" + _, separator, body = text.partition("\n\n") + return body if separator else text + + +class _RefreshLoss: + """Describe what a refused refresh returned, formatted only if logged. + + ``logger.warning`` decides whether a record is emitted before rendering its + arguments, so an f-string built at the call site is computed whichever way + that goes. The rest of this module passes lazy ``%s`` arguments; this keeps + the branch on ``content_type`` without breaking that. + """ + + __slots__ = ("_fresh",) + + def __init__(self, fresh: ReferenceContent) -> None: + self._fresh = fresh + + def __str__(self) -> str: + if self._fresh.content_type in NEEDS_FULL_TEXT_TYPES: + return f"no full text ({self._fresh.content_type})" + # The extracted text, not the whole record. Both records carry the same + # abstract, so including it quotes the cached entry at one size here and + # another in the caller's own ``%d``, and shows a pair that can read as + # half while the sentence says "much shorter". The subtracted lengths + # are the ones every caller decides on. + return ( + f"{len(_text_after_abstract(self._fresh.content))} characters of " + f"{self._fresh.content_type}" + ) + + @dataclass(frozen=True) class FetchOutcome: """A fetch result together with how it was obtained. @@ -263,6 +340,12 @@ def fetch_with_provenance( if not content: return self._stale_fallback(normalized_reference_id, force_refresh) + preserved = self._preserve_cached_full_text( + normalized_reference_id, content, force_refresh + ) + if preserved is not None: + return self._remember(normalized_reference_id, preserved) + self._save_by_access(content) return self._remember(normalized_reference_id, FetchOutcome(content=content)) @@ -311,6 +394,207 @@ def _stale_fallback( normalized_reference_id, FetchOutcome(content=stale, served_stale=True) ) + def _preserve_cached_full_text( + self, + normalized_reference_id: str, + fresh: ReferenceContent, + force_refresh: bool, + ) -> Optional[FetchOutcome]: + """Refuse a refresh that would replace cached full text with none. + + A refresh may improve an entry; it may never demote one. That invariant + is about the **public** validation cache, which is the only one + :meth:`_load_from_disk` reads; a private research-cache entry has no + equivalent protection, and none is attempted here. Full-text + retrieval fails transiently and silently -- a rate-limited PMC request + answers with a reCAPTCHA interstitial carried on an HTTP 200, so nothing + downstream can distinguish it from article text. The bot page is + rejected, the abstract fetch succeeds on its own, and the record would + be written as ``abstract_only``: indistinguishable from "this article + has no full text", and destroying whatever the entry held. + + :meth:`_stale_fallback` does not cover this. It fires only when the + source yields *nothing*, and here the abstract is something. + + **Deleting and serving are separate decisions, and this method makes + both.** Whether an entry may be *overwritten* is asked with + ``allow_stale_html=True``, because a stale ``full_text_html`` entry is + still somebody's data and destroying it is not this method's business. + Whether its text may be *served as evidence* is asked without that + bypass, so :meth:`_load_from_disk`'s refusal of stale HTML -- "it may be + a repository landing page" -- still holds. An entry can therefore be + kept on disk and withheld from validation at the same time, which is the + right answer for a pre-fix entry scraped from a landing page: the + curator's file is not deleted, and its suspect text is not quoted back + as though it came from the article. + + Narrow on purpose: + + * only a **loss** of full text is refused -- the refresh coming back + with none at all (see :meth:`_refresh_loses_full_text`, which is a rule + about kind and does not compare sizes) -- so an entry that gains full + text, keeps it, or had none to begin with, migrates normally. A + refresh that keeps full text but returns far less of it is reported by + :meth:`_report_shrinking_refresh` and written; + * the preserved entry is **not re-saved**, so it stays stale and the + next run that can reach the source still refreshes it -- the same + contract :meth:`_stale_fallback` documents; + * ``force_refresh`` opts out of the *refusal*, because an explicit + refresh that found less is a result the caller asked for -- but not of + the notice: :meth:`_warn_forced_discard` still says what it replaced. + + Both outcomes set ``served_stale``. The flag's consumer is ``cache + reference``, which reports "the cache still holds no current entry for + it", and that is exactly true here in both branches: the write was + skipped, so the entry on disk is the out-of-date one either way. + + Returns ``None`` when the refresh may proceed normally. + """ + if force_refresh: + self._warn_forced_discard(normalized_reference_id, fresh) + return None + + # May it be overwritten? Stale HTML is readable for this question only. + cached = self._load_from_disk( + normalized_reference_id, allow_stale=True, allow_stale_html=True + ) + if cached is None or self.needs_full_text(cached): + return None + if not self._refresh_loses_full_text(cached, fresh): + # The write proceeds. Say so if it replaced much more than it brought, + # which is the only place that report can be true. + self._report_shrinking_refresh(normalized_reference_id, cached, fresh) + return None + + logger.warning( + "Refresh of %s returned %s; keeping the cached %s entry rather than " + "overwriting it. It stays stale, so a later run will try again.", + normalized_reference_id, + _RefreshLoss(fresh), + cached.content_type, + ) + + # May its text be served? Asked without the bypass, so stale HTML is + # withheld here exactly as _stale_fallback withholds it, and validation + # falls back to the freshly fetched abstract. + servable = self._load_from_disk(normalized_reference_id, allow_stale=True) + return FetchOutcome(content=servable or fresh, served_stale=True) + + def _report_shrinking_refresh( + self, + normalized_reference_id: str, + cached: ReferenceContent, + fresh: ReferenceContent, + ) -> None: + """Note a refresh that keeps full text but returns far less of it. + + Reported rather than refused. A PDF whose text layer is a publisher + cover sheet is still typed ``full_text_pdf``, so the kind rule lets it + through and the article body is overwritten -- and until this existed + that happened with no output at all, leaving a curator whose quoted + evidence stopped verifying with nothing to pull on. + + Refusing it is what this guard used to do, and what cost four rounds of + permanently blocked migrations. A warning shares none of that: the write + proceeds, the entry is stamped, the migration completes, and a wrong + guess costs one log line. + """ + cached_length = len(_text_after_abstract(cached.content)) + if not cached_length: + return + if len(_text_after_abstract(fresh.content)) >= cached_length * REPORT_SHRINK_RATIO: + return + logger.warning( + "Refresh of %s replaced the cached %s entry (%d characters of " + "article text) with a much shorter one: %s. Written as usual, since " + "a shorter extraction is often a cleaner one -- but check it if " + "quoted excerpts stop verifying.", + normalized_reference_id, + cached.content_type, + cached_length, + _RefreshLoss(fresh), + ) + + def _warn_forced_discard( + self, normalized_reference_id: str, fresh: ReferenceContent + ) -> None: + """Say what ``force_refresh`` is about to destroy, before it does. + + The opt-out itself is right: an explicit refresh that finds less is a + result the caller asked for. Doing it silently is not. ``--force`` is + what both the ``cache reference`` failure message and the troubleshooting + docs name as the remedy, and a preserved entry fails on every run, so the + pressure to reach for it is continuous and it is typically run across a + batch rather than one reference at a time. Following that advice should + not quietly perform the loss this guard exists to prevent. + """ + cached = self._load_from_disk( + normalized_reference_id, allow_stale=True, allow_stale_html=True + ) + if cached is None or self.needs_full_text(cached): + return + if not self._refresh_loses_full_text(cached, fresh): + return + logger.warning( + "--force is replacing the cached %s entry for %s (%d characters of " + "article text) with %s. The cached text is not recoverable from here.", + cached.content_type, + normalized_reference_id, + len(_text_after_abstract(cached.content)), + _RefreshLoss(fresh), + ) + + @staticmethod + def _refresh_loses_full_text( + cached: ReferenceContent, fresh: ReferenceContent + ) -> bool: + """Report whether a refresh replaces full text with no full text. + + Deliberately a rule about *kind*, not a comparison of size. An earlier + version also refused a refresh whose text was a fraction of the cached + length, to catch a cover-page PDF extraction that clears the acceptance + floor. That guarded one real case and mis-handled four others -- a page + scrape replaced by a clean XML body, by a clean HTML body, a plain-text + API body replaced by XML, and any re-extraction that merely trimmed a + trailing section -- because a shorter extraction is usually a *better* + one, and the shrink cannot tell the two apart. + + Each of those mis-handled cases fails in the worse direction. A refusal + is not a one-off: the entry is never written, so it is never stamped, so + every later run re-fetches and re-refuses it while ``cache reference`` + exits 1. A guard meant to protect the cache instead pinned it + permanently at its pre-migration content. + + The size question belongs in the acceptance layer, where + :func:`linkml_reference_validator.etl.extract.xml.is_stub_notice` + already rejects an XML placeholder before it is ever cached, and where a + wrong answer costs one skipped fetch rather than a cache that can never + migrate. Extending it to PDF text layers -- a cover sheet reads much like + the placeholder it already catches -- would close the one case this rule + knowingly lets through; ``test_a_cover_page_pdf_is_not_refused_but_is_reported`` + pins that gap and says to invert it when that lands. + + Examples: + >>> from linkml_reference_validator.models import ReferenceContent + >>> def ref(content_type): + ... return ReferenceContent( + ... reference_id="PMID:1", content="text", content_type=content_type + ... ) + >>> ReferenceFetcher._refresh_loses_full_text( + ... ref("full_text_html"), ref("abstract_only")) + True + + A shorter or differently-shaped full text is still full text: + + >>> ReferenceFetcher._refresh_loses_full_text( + ... ref("full_text_html"), ref("full_text_xml")) + False + >>> ReferenceFetcher._refresh_loses_full_text( + ... ref("full_text_xml"), ref("full_text")) + False + """ + return fresh.content_type in NEEDS_FULL_TEXT_TYPES + def needs_full_text(self, content: ReferenceContent) -> bool: """Return True if the content lacks full text and the chain should run. @@ -340,6 +624,7 @@ def _maybe_retry_full_text(self, content: ReferenceContent) -> ReferenceContent: """ if ( not self.config.fetch_full_text + or content.full_text_declined or not self.needs_full_text(content) or content.full_text_attempted ): @@ -367,16 +652,20 @@ def _save_by_access(self, content: ReferenceContent) -> None: def _enrich_with_full_text(self, content: ReferenceContent) -> ReferenceContent: """Merge the first usable public full text from the provider chain. - If no provider yields usable full text but the chain was consulted without a - transient error, mark ``full_text_attempted`` so the record is not re-queried - on every later run. A provider/download error leaves the flag unset so a - subsequent run retries (PR #48 review #1). Locations with an explicit - non-open access type are ignored: private-library material is available to - the separate cache-enrichment workflow, never to ordinary validation. + If no provider yields usable full text but the chain was consulted without + a transient error *and without declining anything*, mark + ``full_text_attempted`` so the record is not re-queried on every later + run. A provider/download error leaves the flag unset so a subsequent run + retries (PR #48 review #1), and so does a policy decline -- see the note + at the assignment for why those are the same kind of thing. Locations + with an explicit non-open access type are ignored: private-library + material is available to the separate cache-enrichment workflow, never to + ordinary validation. """ ids = build_identifiers(content) abstract = content.content had_error = False + declined_on_policy: Optional[str] = None for provider_name in self.config.full_text_providers: provider = FullTextProviderRegistry.get(provider_name) @@ -394,12 +683,23 @@ def _enrich_with_full_text(self, content: ReferenceContent) -> ReferenceContent: if location is None: continue + if location.declined: + logger.debug( + "Provider '%s' declined a candidate for %s (%s)", + provider_name, + content.reference_id, + location.declined, + ) + declined_on_policy = location.declined + continue + if location.access_type not in (None, "open"): logger.info( "Ignoring non-public full text from provider '%s' for %s", provider_name, content.reference_id, ) + declined_on_policy = f"access_type:{location.access_type}" continue applied, error = self._apply_full_text_location( @@ -410,9 +710,33 @@ def _enrich_with_full_text(self, content: ReferenceContent) -> ReferenceContent: if applied: return content - # No usable full text: only record a definitive attempt if nothing went wrong, - # so a transient failure stays retryable on the next run. - if not had_error: + # No usable full text. ``full_text_attempted`` means "a clean run + # concluded none is available", and ``_maybe_retry_full_text`` never runs + # the chain again once it is set, so only a genuine absence may set it. + # + # Two outcomes are not absences. A transient failure is not one, which + # ``had_error`` has always covered. Neither is a location we *found* and + # declined -- a bronze PDF refused on licence, or a landing page refused + # because fetching it would be scraping. Those are decisions about + # material that exists, and recording a decision as a fact about the + # article is the defect this whole change is about, one layer up: a + # bronze record that later converts to gold, or a page-only DOI that + # later gains a repository PDF, would never be looked at again. + # + # The cost of leaving the flag unset is one chain re-run per process for + # those references, which is the trade ``had_error`` already makes. + if declined_on_policy: + # Remembered, not asserted. Leaving nothing at all would be correct + # and ruinous: a decline never clears the way a transient error + # does, so every run would re-walk the whole chain for every bronze + # or page-only reference -- four providers, each opening with a + # rate-limit sleep, against the hosts whose rate limiting produces + # the interstitial this guard exists for. On one real corpus that is + # 23,465 eligible entries and about thirteen hours of sleep per run. + # The entry is re-fetched when the extractor version moves, which is + # when a re-walk is actually worth paying for. + content.full_text_declined = declined_on_policy + elif not had_error: content.full_text_attempted = True return content @@ -493,10 +817,17 @@ def _apply_full_text_location( content.metadata or {}, xml_extraction_version=XML_EXTRACTION_CACHE_VERSION ) content.full_text_provider = location.provider or provider_name - # Non-public endpoints are not durable provenance and may contain - # session-specific access information. + # A private-library endpoint is not durable provenance and may carry + # session-specific access information -- a localhost Zotero attachment + # URL means nothing to anyone else and may encode a local token. A + # publisher's own link is neither: it is a stable public URL, and the + # reason a PUBLISHER_FREE_ACCESS location's *text* stays out of the + # public cache is licensing rather than secrecy, so recording where it + # came from costs nothing and is worth keeping. content.full_text_url = ( - None if location.access_type not in (None, "open") else location.url + location.url + if location.access_type in PUBLIC_URL_ACCESS_TYPES + else None ) content.oa_status = location.oa_status content.license = location.license @@ -833,6 +1164,11 @@ def _save_to_disk( ) if reference.full_text_attempted: lines.append("full_text_attempted: true") + if reference.full_text_declined: + lines.append( + f"full_text_declined: " + f"{self._quote_yaml_value(reference.full_text_declined)}" + ) if reference.full_text_provider: lines.append(f"full_text_provider: {reference.full_text_provider}") if reference.full_text_url: @@ -911,7 +1247,10 @@ def _save_to_disk( logger.info(f"Cached {reference.reference_id} to {cache_path}") def _load_from_disk( - self, reference_id: str, allow_stale: bool = False + self, + reference_id: str, + allow_stale: bool = False, + allow_stale_html: bool = False, ) -> Optional[ReferenceContent]: """Load reference content from the public validation cache. @@ -922,8 +1261,18 @@ def _load_from_disk( Args: reference_id: Reference identifier allow_stale: Return entries written by an older extractor instead of - treating them as absent. Used only by :meth:`_stale_fallback`, - once a fetch has already failed to produce a replacement. + treating them as absent. Used once a fetch has already failed to + produce a usable replacement -- by :meth:`_stale_fallback` when + the source yielded nothing at all, and by + :meth:`_preserve_cached_full_text` and + :meth:`_warn_forced_discard` when it yielded no full text. + allow_stale_html: Also return a stale ``full_text_html`` entry, which + ``allow_stale`` alone withholds because it may be a repository + landing page rather than the article. Set this only to decide + whether an entry may be *overwritten*; leave it off to decide + whether its text may be *served* as evidence. Meaningless + without ``allow_stale``, which rejects a stale entry of any type + before this is consulted. Returns: ReferenceContent if cached, None otherwise @@ -960,13 +1309,15 @@ def _load_from_disk( reference = self._load_markdown_format(content_text, reference_id) if ( allow_stale + and not allow_stale_html and reference is not None and reference.content_type == "full_text_html" and self._is_stale_cache_entry(content_text) ): logger.warning( "Refusing stale HTML full text for %s: it may be a repository " - "landing page. Retry when the source is reachable to repair it.", + "landing page. Retry when the source serves full text again to " + "repair it.", reference_id, ) return None @@ -1184,6 +1535,7 @@ def _load_markdown_format( is_preprint=frontmatter.get("is_preprint"), peer_review_status=frontmatter.get("peer_review_status"), full_text_attempted=bool(frontmatter.get("full_text_attempted", False)), + full_text_declined=frontmatter.get("full_text_declined"), ) def _extract_content_from_markdown(self, body: str) -> str: diff --git a/src/linkml_reference_validator/models.py b/src/linkml_reference_validator/models.py index 544622a..0d1284d 100644 --- a/src/linkml_reference_validator/models.py +++ b/src/linkml_reference_validator/models.py @@ -719,8 +719,10 @@ class ReferenceIdentifiers: class FullTextLocation: """A located full-text resource for a reference. - A provider returns either a downloadable ``url`` (PDF/HTML/XML) or inline - ``text`` it has already extracted. + A provider returns one of three things: a downloadable ``url`` + (PDF/HTML/XML), inline ``text`` it has already extracted, or -- carrying + neither -- a ``declined`` marker saying it found a candidate and refused it. + Check ``declined`` before reading ``url``. Examples: >>> loc = FullTextLocation(url="https://x/y.pdf", format_hint="pdf") @@ -737,8 +739,20 @@ class FullTextLocation: license: Optional[str] = None provider: str = "" version: Optional[str] = None # "publishedVersion" | "acceptedVersion" | ... - access_type: Optional[str] = None # "open" | "user_library" | "institutional" + # "open" | "publisher_free" | "user_library" | "institutional". Anything + # other than "open" (or None) is withheld from ordinary validation and + # routed to the private cache: see _enrich_with_full_text and + # _save_by_access. "publisher_free" is free to read on the publisher's site + # under no open licence -- readable, not redistributable. + access_type: Optional[str] = None source_item_id: Optional[str] = None + #: Set when a provider *found* a candidate and declined it on policy, rather + #: than finding nothing. A declined location carries no ``url`` or ``text`` + #: and is never fetched; it exists so the chain can tell a decision we made + #: apart from a fact about the article. Recording the first as the second is + #: what leaves a bronze record that later converts to gold unexamined + #: forever. + declined: Optional[str] = None @dataclass @@ -795,6 +809,12 @@ class ReferenceContent: oa_status: Optional[str] = None license: Optional[str] = None local_pdf_path: Optional[str] = None + #: Why the provider chain refused a candidate it found, if it did. Distinct + #: from ``full_text_attempted``, which asserts that a clean run concluded no + #: full text is available: a decline is a decision about material that + #: exists. Recorded so the chain is re-walked when the extractor version + #: moves rather than on every run -- retryable, not retried indefinitely. + full_text_declined: Optional[str] = None full_text_access_type: Optional[str] = None full_text_source_item_id: Optional[str] = None # Preprint / peer-review status, surfaced so downstream KBs can apply policies diff --git a/tests/test_cli_cache_reference.py b/tests/test_cli_cache_reference.py index 89221e9..eced9ad 100644 --- a/tests/test_cli_cache_reference.py +++ b/tests/test_cli_cache_reference.py @@ -112,7 +112,11 @@ def test_unreachable_source_with_a_stale_entry_reports_failure(cache_dir, mocker assert result.exit_code == 1 assert "Successfully cached" not in result.output assert "Failed to cache PMID:1" in result.output - assert "out-of-date" in result.output + # Asserted on the invariant both failure paths share -- no entry was + # written -- rather than on which path ran. The message covers the refused + # refresh as well as the unreachable source, and phrasing that names only + # one of them was what made this assertion wrong when the second arrived. + assert "no current entry" in result.output def test_unreachable_source_with_a_stale_entry_writes_nothing(cache_dir, mocker): @@ -199,3 +203,69 @@ def test_a_current_entry_is_reported_as_cached(cache_dir, mocker): assert result.exit_code == 0 assert "Successfully cached PMID:1" in result.output + + +# -------------------------------------------------------------------------- +# A preserved entry is not a served entry +# -------------------------------------------------------------------------- + + +def test_a_preserved_entry_is_reported_without_claiming_it_was_served( + cache_dir, mocker +): + """The exit code was right; the sentence was not. + + When ``_preserve_cached_full_text`` withholds a stale ``full_text_html`` + entry, the cached text is deliberately *not* served -- the freshly fetched + abstract is. Saying "an out-of-date cache entry was served" describes the + other branch. What is true of both is that the write was skipped, so the + cache still holds no current entry. + """ + _write_stale_entry( + cache_dir, + content="Scraped page text. " * 200, + content_type="full_text_html", + ) + _source_returning( + mocker, + ReferenceContent( + reference_id="PMID:1", content="An abstract.", content_type="abstract_only" + ), + ) + + result = _run(cache_dir, "PMID:1") + + assert result.exit_code == 1 + assert "out-of-date cache entry was served" not in result.output + assert "no current entry" in result.output + + +def test_a_preserved_entry_keeps_failing_until_forced(cache_dir, mocker): + """Worth pinning, because it is the surprising consequence of the guard. + + A script that caches a list of references and gates on the exit status will + fail on a preserved entry every run, not once: the entry is never written, + so it is never stamped, so the next run reaches the same decision. + """ + _write_stale_entry( + cache_dir, + content="Scraped page text. " * 200, + content_type="full_text_html", + ) + _source_returning( + mocker, + ReferenceContent( + reference_id="PMID:1", content="An abstract.", content_type="abstract_only" + ), + ) + + path = _fetcher(cache_dir).get_cache_path("PMID:1") + + assert _run(cache_dir, "PMID:1").exit_code == 1 + assert _run(cache_dir, "PMID:1").exit_code == 1, "still failing on a later run" + assert "Scraped page text" in path.read_text(encoding="utf-8") + + assert _run(cache_dir, "PMID:1", "--force").exit_code == 0, "--force is the exit" + # What --force costs, asserted rather than left to the reader. It is the + # remedy the failure message names, so the cost belongs in the contract. + assert "Scraped page text" not in path.read_text(encoding="utf-8") diff --git a/tests/test_fulltext_providers.py b/tests/test_fulltext_providers.py index f42dcac..bf628d9 100644 --- a/tests/test_fulltext_providers.py +++ b/tests/test_fulltext_providers.py @@ -128,7 +128,21 @@ def test_locate_returns_pdf_location(self, mock_get, config): assert loc.provider == "openalex" @patch("linkml_reference_validator.etl.fulltext.openalex.requests.get") - def test_locate_falls_back_to_oa_url(self, mock_get, config): + def test_locate_does_not_fall_back_to_oa_url(self, mock_get, config): + """A landing page is not a located full text. + + This test previously asserted the opposite: an ``oa_url`` with no + ``pdf_url`` was returned as an ``html`` location and then downloaded. + That is scraping an article *page*, which the hosts actively refuse -- + PMC answers a rate-limited request with a reCAPTCHA interstitial on an + HTTP 200, indistinguishable downstream from article text. See + tests/test_open_access_policy.py for the policy this belongs to. + + The provider reports the decline rather than returning nothing, so the + enrichment chain can tell "we refused this" from "there is none" and + keeps the record retryable. What matters here is unchanged: no ``url`` + and no ``text``, so nothing is ever fetched. + """ from linkml_reference_validator.etl.fulltext.openalex import OpenAlexProvider mock_response = MagicMock() @@ -139,9 +153,10 @@ def test_locate_falls_back_to_oa_url(self, mock_get, config): } mock_get.return_value = mock_response - loc = OpenAlexProvider().locate(ReferenceIdentifiers(doi="10.1/x"), config) - assert loc.url == "https://oa/landing" - assert loc.format_hint == "html" + location = OpenAlexProvider().locate(ReferenceIdentifiers(doi="10.1/x"), config) + + assert location.declined == "landing_page_only" + assert location.url is None and location.text is None @patch("linkml_reference_validator.etl.fulltext.openalex.requests.get") def test_locate_not_oa_returns_none(self, mock_get, config): diff --git a/tests/test_no_cache_downgrade.py b/tests/test_no_cache_downgrade.py new file mode 100644 index 0000000..4bc5c9a --- /dev/null +++ b/tests/test_no_cache_downgrade.py @@ -0,0 +1,526 @@ +"""A refresh may improve a cache entry. It may never demote one. + +The extractor-version stamp introduced in #62 makes every pre-stamp entry stale, +so the next run re-fetches and rewrites it. That is the intended migration, and +it is safe only while the replacement is at least as good as what it replaces. + +It often is not. Full-text retrieval fails transiently and *silently*: PMC +answers a rate-limited request with a reCAPTCHA interstitial carried on an +HTTP 200, so no status code marks it. The bot page is correctly rejected +downstream, the abstract fetch succeeds on its own, and the record is written as +``abstract_only`` — byte-identical to "this article has no full text". Measured +against one real reference, the same fetch returned full text on 2 of 5 +consecutive attempts. + +``_stale_fallback`` does not cover this. It fires only when the source yields +*nothing*; here the abstract is something, so the fresh record is saved over a +cached one holding thousands of characters of article text, and every excerpt +quoted from that text stops validating. + +The guard is deliberately narrow, and narrower than it first was. It refuses one +thing: a refresh that comes back with *no* full text where the cache has some. +It does not compare sizes. An earlier version also refused a refresh whose text +was a fraction of the cached length; that caught one real case -- a cover-page +PDF extraction -- and mis-handled four others, because a shorter extraction is +usually a better one. Those four are pinned below as cases that must migrate. + +The preserved entry is not re-saved, so it stays stale and the next run tries +again. A refresh that finds full text still rewrites the entry, which is the +whole point of the migration. + +Keeping an entry and serving it are separate decisions. ``_load_from_disk`` +refuses to serve stale ``full_text_html`` because it may be a repository landing +page; that refusal still holds, so such an entry is kept on disk *and* withheld +from validation, which falls back to the freshly fetched abstract. +""" + +import logging +from unittest.mock import patch + +import pytest + +from linkml_reference_validator.etl.reference_fetcher import ReferenceFetcher +from linkml_reference_validator.models import ReferenceContent, ReferenceValidationConfig + +FULL_TEXT_BODY = "Severe cleft palate was found in all homozygous mutants. " * 40 +#: ~2,000 characters, the median of the abstract-only entries in a real cache. +REAL_ABSTRACT = "A representative abstract sentence. " * 56 +ABSTRACT_BODY = "A short abstract." + + +def _write_cache(cache_dir, reference_id, content_type, body, stamped=False): + """Write a cache entry, pre-stamp (stale) by default. + + ``stamped=True`` writes the format-specific stamp as well as + ``extractor_version``: ``_is_stale_cache_entry`` requires + ``xml_extraction_version`` on a ``full_text_xml`` entry and + ``html_full_text_version`` on a ``full_text_html`` one, so the extractor + stamp alone still reads as stale for those two types. + """ + cache_dir.mkdir(parents=True, exist_ok=True) + stamp = "" + if stamped: + stamp = "extractor_version: 1\n" + if content_type == "full_text_xml": + stamp += "xml_extraction_version: 1\n" + elif content_type == "full_text_html": + stamp += "html_full_text_version: 1\n" + path = cache_dir / f"{reference_id.replace(':', '_')}.md" + path.write_text( + f"---\nreference_id: {reference_id}\n{stamp}" + f"title: A paper\ncontent_type: {content_type}\n---\n\n## Content\n\n{body}\n", + encoding="utf-8", + ) + return path + + +@pytest.fixture +def fetcher(tmp_path): + return ReferenceFetcher(ReferenceValidationConfig(cache_dir=tmp_path, email="me@example.org")) + + +def _fetch_returning(fetcher, content_type, body): + """Run ``fetch`` with the source stubbed to return the given content.""" + fresh = ReferenceContent( + reference_id="PMID:9177246", + content=body, + content_type=content_type, + title="A paper", + ) + + class _Source: + def fetch(self, identifier, config): + return fresh + + with patch( + "linkml_reference_validator.etl.reference_fetcher.ReferenceSourceRegistry.get_source", + return_value=_Source, + ): + return fetcher.fetch("PMID:9177246") + + +@pytest.mark.parametrize( + "cached_type", ["full_text_html", "full_text_xml", "full_text_pdf"] +) +def test_a_failed_full_text_refresh_keeps_the_cached_file(fetcher, tmp_path, cached_type): + """The case that loses committed evidence. + + This is the *deletion* claim, and it holds for every full-text type. What + may then be **served** differs by type and is covered separately below: + stale HTML is kept but withheld, since it may be a landing page. + """ + path = _write_cache(tmp_path, "PMID:9177246", cached_type, FULL_TEXT_BODY) + + _fetch_returning(fetcher, "abstract_only", ABSTRACT_BODY) + + assert "Severe cleft palate" in path.read_text(encoding="utf-8"), ( + "a transient full-text failure must not replace cached article text" + ) + + +def test_the_preserved_entry_stays_stale_so_the_next_run_retries(fetcher, tmp_path): + """Preserving is not migrating: leave it un-stamped.""" + path = _write_cache(tmp_path, "PMID:9177246", "full_text_html", FULL_TEXT_BODY) + + _fetch_returning(fetcher, "abstract_only", ABSTRACT_BODY) + + assert "extractor_version" not in path.read_text(encoding="utf-8"), ( + "stamping it would end the migration on the worse content" + ) + + +def test_a_successful_refresh_still_rewrites_the_entry(fetcher, tmp_path): + """The migration must still work when the replacement is good.""" + _write_cache(tmp_path, "PMID:9177246", "full_text_html", "old text " * 50) + + result = _fetch_returning(fetcher, "full_text_xml", "NEW BODY " * 50) + + assert "NEW BODY" in (result.content or "") + assert result.content_type == "full_text_xml" + + +def test_an_upgrade_from_abstract_to_full_text_is_allowed(fetcher, tmp_path): + _write_cache(tmp_path, "PMID:9177246", "abstract_only", ABSTRACT_BODY) + + result = _fetch_returning(fetcher, "full_text_xml", FULL_TEXT_BODY) + + assert result.content_type == "full_text_xml" + + +def test_an_abstract_refresh_over_an_abstract_entry_is_not_a_downgrade(fetcher, tmp_path): + """No full text is lost, so nothing is preserved and the entry migrates.""" + _write_cache(tmp_path, "PMID:9177246", "abstract_only", "stale abstract") + + result = _fetch_returning(fetcher, "abstract_only", "fresher abstract") + + assert "fresher abstract" in (result.content or "") + + +def test_no_cached_entry_means_nothing_to_preserve(fetcher): + result = _fetch_returning(fetcher, "abstract_only", ABSTRACT_BODY) + + assert result.content_type == "abstract_only" + + +# -------------------------------------------------------------------------- +# Keeping an entry is not the same as serving it +# -------------------------------------------------------------------------- + + +def test_stale_html_is_kept_on_disk_but_not_served_as_evidence(fetcher, tmp_path): + """The case the guard must not get wrong. + + A pre-fix ``full_text_html`` entry is disproportionately likely to *be* a + scraped landing page -- that is what this release stops fetching. Keeping + the curator's file is right; quoting its text back as though it came from + the article is not. ``_load_from_disk`` already refuses to serve stale HTML, + and that refusal survives the guard. + """ + path = _write_cache(tmp_path, "PMID:9177246", "full_text_html", FULL_TEXT_BODY) + + result = _fetch_returning(fetcher, "abstract_only", ABSTRACT_BODY) + + assert "Severe cleft palate" in path.read_text(encoding="utf-8"), ( + "the file must be kept" + ) + assert "Severe cleft palate" not in (result.content or ""), ( + "possible landing-page text must not be served to validation" + ) + assert result.content == ABSTRACT_BODY + + +def test_stale_non_html_full_text_is_both_kept_and_served(fetcher, tmp_path): + """XML and PDF carry no landing-page risk, so the old text is still usable.""" + path = _write_cache(tmp_path, "PMID:9177246", "full_text_xml", FULL_TEXT_BODY) + + result = _fetch_returning(fetcher, "abstract_only", ABSTRACT_BODY) + + assert "Severe cleft palate" in path.read_text(encoding="utf-8") + assert "Severe cleft palate" in (result.content or "") + + +@pytest.mark.parametrize( + "cached_type", ["full_text_html", "full_text_pdf", "full_text_xml"] +) +def test_a_preserved_entry_reports_served_stale(fetcher, tmp_path, cached_type): + """``cache reference`` must not claim success when it wrote nothing. + + Its message is "the cache still holds no current entry for it", which is + true in both branches: the write was skipped either way. + """ + _write_cache(tmp_path, "PMID:9177246", cached_type, FULL_TEXT_BODY) + + fresh = ReferenceContent( + reference_id="PMID:9177246", content=ABSTRACT_BODY, content_type="abstract_only" + ) + + class _Source: + def fetch(self, identifier, config): + return fresh + + with patch( + "linkml_reference_validator.etl.reference_fetcher.ReferenceSourceRegistry.get_source", + return_value=_Source, + ): + outcome = fetcher.fetch_with_provenance("PMID:9177246") + + assert outcome.served_stale is True + + +def test_force_refresh_warns_before_discarding_cached_full_text(fetcher, tmp_path, caplog): + """The opt-out is right; doing it silently is not. + + ``--force`` is what both the ``cache reference`` failure message and the + troubleshooting docs tell a user to run, and a preserved entry fails every + run, so the pressure to reach for it is continuous and it will usually be + run across a batch. Following that advice on the motivating entry replaces + 18,464 characters with a 1,199-character abstract; the caller asked for the + refresh, so it proceeds, but they should be told what it cost. + """ + _write_cache(tmp_path, "PMID:9177246", "full_text_html", FULL_TEXT_BODY) + + fresh = ReferenceContent( + reference_id="PMID:9177246", content=ABSTRACT_BODY, content_type="abstract_only" + ) + + class _Source: + def fetch(self, identifier, config): + return fresh + + with caplog.at_level(logging.WARNING): + with patch( + "linkml_reference_validator.etl.reference_fetcher.ReferenceSourceRegistry.get_source", + return_value=_Source, + ): + fetcher.fetch("PMID:9177246", force_refresh=True) + + assert any( + "--force is replacing the cached" in record.getMessage() + for record in caplog.records + ), "the discard must not be silent" + + +def test_force_refresh_overrides_the_guard(fetcher, tmp_path): + """An explicit refresh that finds less is a result the caller asked for.""" + path = _write_cache(tmp_path, "PMID:9177246", "full_text_xml", FULL_TEXT_BODY) + + fresh = ReferenceContent( + reference_id="PMID:9177246", content=ABSTRACT_BODY, content_type="abstract_only" + ) + + class _Source: + def fetch(self, identifier, config): + return fresh + + with patch( + "linkml_reference_validator.etl.reference_fetcher.ReferenceSourceRegistry.get_source", + return_value=_Source, + ): + fetcher.fetch("PMID:9177246", force_refresh=True) + + assert "Severe cleft palate" not in path.read_text(encoding="utf-8") + + + + + + + + +# -------------------------------------------------------------------------- +# A change of type is not a shrink +# -------------------------------------------------------------------------- + + +def test_a_page_scrape_upgrading_to_a_clean_extraction_still_migrates(fetcher, tmp_path): + """The exact population this release acts on. + + A pre-fix ``full_text_html`` entry holds a whole scraped page -- body, + reference list, related-article furniture. The refresh goes to PMC XML and + extracts the article body alone, which is legitimately a fraction of that. + Refusing it would block the migration permanently: never written, so never + stamped, so re-fetched and re-refused on every subsequent run, with + ``cache reference`` exiting 1 each time and no exit but ``--force``. + + That is the bug this guard exists to prevent, with the sign flipped. + """ + path = _write_cache( + tmp_path, "PMID:9177246", "full_text_html", "page text with furniture " * 800 + ) + + result = _fetch_returning(fetcher, "full_text_xml", "clean article body " * 330) + + assert "clean article body" in (result.content or "") + assert "clean article body" in path.read_text(encoding="utf-8"), ( + "the better extraction must be written, not refused" + ) + + + + + + + + + + +def test_a_plain_text_body_upgrading_to_xml_still_migrates(fetcher, tmp_path): + """``full_text`` is one of this project's own four content types. + + ``_FORMAT_TO_CONTENT_TYPE`` maps ``format_hint="text"`` to it, and + ``_materialize`` defaults to that hint, so a configured ``json_api`` text + provider writes it to the *public* cache. An earlier revision ranked content + types to decide how strict a size comparison should be, and leaving this one + out of the ranking gave it the strict bar and so the permanent-block loop: + not written, not stamped, re-refused every run. The ranking went with the + comparison; this pins the outcome that mattered, which is that a plain-text + body still migrates to XML. + """ + path = _write_cache( + tmp_path, "PMID:9177246", "full_text", "plain text api body " * 1000 + ) + + result = _fetch_returning(fetcher, "full_text_xml", "clean xml body text " * 320) + + assert "clean xml body text" in (result.content or "") + assert "clean xml body text" in path.read_text(encoding="utf-8") + + +def test_a_stamped_entry_is_served_from_cache_without_re_fetching(fetcher, tmp_path): + """The other side of the migration, and the reason ``stamped`` exists. + + Every other test here writes a pre-stamp entry, because the guard only runs + on a refresh and only a stale entry is refreshed. This pins the complement: + once an entry carries the current stamp it is served from disk and the + source is never consulted, so the preserve path cannot fire on it and the + fetch storm the stale entries cause does not apply to a migrated cache. + """ + _write_cache( + tmp_path, "PMID:9177246", "full_text_xml", FULL_TEXT_BODY, stamped=True + ) + + class _Source: + def fetch(self, identifier, config): # pragma: no cover - must not run + raise AssertionError("a stamped entry must not be re-fetched") + + with patch( + "linkml_reference_validator.etl.reference_fetcher.ReferenceSourceRegistry.get_source", + return_value=_Source, + ): + result = fetcher.fetch("PMID:9177246") + + assert "Severe cleft palate" in (result.content or "") + + + + + + + + + + + + + +# -------------------------------------------------------------------------- +# The page-scrape migration within one rung +# -------------------------------------------------------------------------- + + +def test_a_page_scrape_refreshed_to_a_clean_html_body_still_migrates(fetcher, tmp_path): + """The same improvement as the cross-type case, and just as reachable. + + ``PMCFullTextProvider`` returns ``format_hint="html"`` whenever its XML path + falls short, so a pre-fix landing-page scrape can legitimately be replaced by + a ``div.article-body`` extraction at the *same* content type, and much + shorter. An earlier revision refused exactly this, permanently, by comparing + sizes; the guard now asks only whether full text survived at all. + """ + path = _write_cache( + tmp_path, "PMID:9177246", "full_text_html", "whole page with furniture " * 800 + ) + + result = _fetch_returning(fetcher, "full_text_html", "clean article body " * 300) + + assert "clean article body" in (result.content or "") + assert "clean article body" in path.read_text(encoding="utf-8") + + + + +# -------------------------------------------------------------------------- +# What the rule deliberately does not catch +# -------------------------------------------------------------------------- + + +def test_a_cover_page_pdf_is_not_refused_but_is_reported(fetcher, tmp_path, caplog): + """A recorded gap, not an oversight. + + A PDF whose text layer is a publisher cover sheet is still typed + ``full_text_pdf``, so this guard lets it through. Catching it needs a + judgement about whether text *is* an article, which is the acceptance + layer's question -- ``is_stub_notice`` already asks it for XML, and there a + wrong answer costs one skipped fetch. Asking it here instead cost four + rounds of wrongly-refused migrations, each permanent, because a refusal is + never written and so never stamped. + + Pinned so the gap is visible and deliberate. If PDF stub detection lands in + the acceptance layer, this test should start failing and be inverted. + + Not refusing is not the same as not mentioning: the write is still recorded, + so a curator whose quoted evidence stops verifying has a thread to pull. + """ + path = _write_cache( + tmp_path, "PMID:9177246", "full_text_html", "clean article body " * 740 + ) + + with caplog.at_level(logging.WARNING): + _fetch_returning(fetcher, "full_text_pdf", "Purchase this article. " * 26) + + assert "clean article body" not in path.read_text(encoding="utf-8"), ( + "the write proceeds -- refusing it is what caused four rounds of " + "permanently blocked migrations" + ) + assert any( + "much shorter" in record.getMessage() for record in caplog.records + ), "but it is not silent" + + +def test_a_refused_refresh_does_not_also_claim_it_was_written(fetcher, tmp_path, caplog): + """The two messages must not contradict each other. + + The shrink report describes a write. On the refused path there is no write, + so firing it there produced two warnings back to back -- "replaced the + cached entry ... Written as usual" immediately followed by "keeping the + cached entry rather than overwriting it". A reader cannot tell which + happened, which is worse than the silence the report was added to fix. + """ + _write_cache(tmp_path, "PMID:9177246", "full_text_html", FULL_TEXT_BODY) + + with caplog.at_level(logging.WARNING): + _fetch_returning(fetcher, "abstract_only", ABSTRACT_BODY) + + messages = [record.getMessage() for record in caplog.records] + assert any("keeping the cached" in m for m in messages), "the refusal is reported" + assert not any("Written as usual" in m for m in messages), ( + "and nothing claims the opposite" + ) + + +def test_the_report_ignores_the_abstract_both_records_carry(fetcher, tmp_path, caplog): + """Otherwise the report misses the case it exists for, at the size that matters. + + ``_apply_full_text_location`` stores ``abstract + "\n\n" + text``, so a + cover-page extraction arrives carrying the abstract as well. Comparing whole + records credits it with that text: a ~2,000-character abstract alone clears a + fifth of a 10,000-character entry, so the write goes through in silence -- + which is exactly the cover-page case, at exactly the article size where it is + most likely. + """ + _write_cache(tmp_path, "PMID:9177246", "full_text_html", "article body text " * 560) + cover_page = f"{REAL_ABSTRACT}\n\n" + "Purchase this article. " * 26 + + with caplog.at_level(logging.WARNING): + _fetch_returning(fetcher, "full_text_pdf", cover_page) + + assert any( + "much shorter" in record.getMessage() for record in caplog.records + ), "the abstract must not pay for the floor" + + +def test_the_shrink_report_quotes_the_numbers_it_decided_on(fetcher, tmp_path, caplog): + """A message whose figures do not explain its own verdict is the defect. + + The decision uses abstract-subtracted lengths; rendering whole records + instead quoted the cached entry at two different sizes in one sentence and + showed a fresh/cached pair that can read as half. A curator cannot + reconstruct why "much shorter" fired, and the natural inference -- that the + threshold is near 50% -- is wrong. + """ + body = "article body text " * 167 # ~3,000 characters + _write_cache(tmp_path, "PMID:9177246", "full_text_html", f"{REAL_ABSTRACT}\n\n{body}") + cover = f"{REAL_ABSTRACT}\n\n" + "Purchase. " * 50 # ~500 characters of text + + with caplog.at_level(logging.WARNING): + _fetch_returning(fetcher, "full_text_pdf", cover) + + report = next(m for m in (r.getMessage() for r in caplog.records) if "much shorter" in m) + assert "3005 characters of article text" in report, f"cached body, not record: {report}" + assert "500 characters of full_text_pdf" in report, f"fresh body, not record: {report}" + # The whole-record figures (~5,000 and ~2,500) must not appear: quoting them + # would show a pair that reads as half while the sentence says much shorter. + assert "5023" not in report and "2518" not in report, report + + +def test_text_after_abstract_handles_a_record_with_no_abstract(fetcher): + """``_apply_full_text_location`` writes bare text when there is no abstract. + + The partition then lands on the first paragraph break in the body and drops + the opening paragraph. It decides a log line either way, so this pins the + behaviour rather than asserting it is ideal. + """ + from linkml_reference_validator.etl.reference_fetcher import _text_after_abstract + + assert _text_after_abstract("A body with no blank line") == "A body with no blank line" + assert _text_after_abstract(None) == "" diff --git a/tests/test_open_access_policy.py b/tests/test_open_access_policy.py new file mode 100644 index 0000000..255dbcb --- /dev/null +++ b/tests/test_open_access_policy.py @@ -0,0 +1,686 @@ +"""Only openly-licensed *files* are fetched into the public cache. + +Two rules, one policy. Both follow from the contract PR #61 established for the +Zotero provider: material a user may lawfully read is not automatically material +a project may redistribute, and the public, checked-in reference cache is +redistribution. + +**Do not scrape.** A provider that has only a landing/article *page* — an +``oa_url`` with no ``pdf_url`` — has not found a file. Fetching it means +scraping HTML from a host that did not offer it for machine retrieval, and the +hosts defend against exactly that: PMC serves a reCAPTCHA interstitial (with +HTTP 200, so nothing downstream can tell it from content). Providers now return +``None`` rather than a page URL. + +**Bronze is not open.** ``oa_status: bronze`` means free to read on the +publisher's site under no open licence. The full text may be readable; it is +not redistributable. Bronze locations are marked with a non-open +``access_type``, which ``_enrich_with_full_text`` already skips for ordinary +validation — the same route Zotero's private-library material takes. + +**Green is decided by its licence.** ``green`` names a repository, not a +permission, so a deposit is open when it states its terms and not when it merely +states its address. + +The information needed for both decisions was already being recorded +(``oa_status``) and simply was not consulted: ``access_type`` was set by the +Zotero provider alone, so everything OpenAlex and Unpaywall returned defaulted +to ``None`` and was saved as public. +""" + +import pytest +from unittest.mock import patch, MagicMock + +from linkml_reference_validator.models import ( + ReferenceIdentifiers, + ReferenceValidationConfig, +) + + +@pytest.fixture +def config(tmp_path): + return ReferenceValidationConfig( + cache_dir=tmp_path / "cache", rate_limit_delay=0.0, email="me@example.org" + ) + + +def _openalex_payload( + oa_status, pdf_url, oa_url="https://oa.example.org/landing", license="cc-by" +): + response = MagicMock() + response.status_code = 200 + response.json.return_value = { + "open_access": {"is_oa": True, "oa_status": oa_status, "oa_url": oa_url}, + "best_oa_location": {"pdf_url": pdf_url, "license": license}, + } + return response + + +def _unpaywall_payload( + oa_status, pdf_url, landing="https://oa.example.org/landing", license="cc-by" +): + response = MagicMock() + response.status_code = 200 + response.json.return_value = { + "is_oa": True, + "oa_status": oa_status, + "best_oa_location": { + "url_for_pdf": pdf_url, + "url": landing, + "license": license, + }, + } + return response + + +# -------------------------------------------------------------------------- +# Rule 1: no landing-page scraping +# -------------------------------------------------------------------------- + + +@patch("linkml_reference_validator.etl.fulltext.openalex.requests.get") +def test_openalex_does_not_return_a_landing_page(mock_get, config): + """An ``oa_url`` with no ``pdf_url`` is a page, not a file. + + Asserted on what matters -- nothing fetchable comes back -- rather than on + ``is None``. The provider reports the decline instead of returning nothing, + so the chain can tell "we refused this" from "there is none", but a declined + location carries no ``url`` and no ``text`` and is never fetched. + """ + from linkml_reference_validator.etl.fulltext.openalex import OpenAlexProvider + + mock_get.return_value = _openalex_payload("gold", pdf_url=None) + + location = OpenAlexProvider().locate(ReferenceIdentifiers(doi="10.1/x"), config) + + assert location.declined == "landing_page_only" + assert location.url is None and location.text is None + + +@patch("linkml_reference_validator.etl.fulltext.unpaywall.requests.get") +def test_unpaywall_does_not_return_a_landing_page(mock_get, config): + from linkml_reference_validator.etl.fulltext.unpaywall import UnpaywallProvider + + mock_get.return_value = _unpaywall_payload("gold", pdf_url=None) + + location = UnpaywallProvider().locate(ReferenceIdentifiers(doi="10.1/x"), config) + + assert location.declined == "landing_page_only" + assert location.url is None and location.text is None + + +@patch("linkml_reference_validator.etl.fulltext.openalex.requests.get") +def test_a_pdf_is_still_returned(mock_get, config): + """The rule removes page scraping, not open-access fetching.""" + from linkml_reference_validator.etl.fulltext.openalex import OpenAlexProvider + + mock_get.return_value = _openalex_payload( + "gold", pdf_url="https://oa.example.org/paper.pdf" + ) + + location = OpenAlexProvider().locate(ReferenceIdentifiers(doi="10.1/x"), config) + + assert location is not None + assert location.url == "https://oa.example.org/paper.pdf" + assert location.format_hint == "pdf" + + +# -------------------------------------------------------------------------- +# Rule 2: bronze is readable, not redistributable +# -------------------------------------------------------------------------- + + +@pytest.mark.parametrize("oa_status", ["gold", "diamond", "hybrid"]) +@patch("linkml_reference_validator.etl.fulltext.openalex.requests.get") +def test_openly_licensed_statuses_are_public(mock_get, config, oa_status): + """These statuses carry an open licence by definition, licence field or not.""" + from linkml_reference_validator.etl.fulltext.openalex import OpenAlexProvider + + mock_get.return_value = _openalex_payload( + oa_status, pdf_url="https://oa.example.org/paper.pdf", license=None + ) + + location = OpenAlexProvider().locate(ReferenceIdentifiers(doi="10.1/x"), config) + + assert location.access_type == "open" + + +@patch("linkml_reference_validator.etl.fulltext.openalex.requests.get") +def test_green_with_a_stated_licence_is_public(mock_get, config): + """A repository deposit that states its terms is redistributable.""" + from linkml_reference_validator.etl.fulltext.openalex import OpenAlexProvider + + mock_get.return_value = _openalex_payload( + "green", pdf_url="https://repo.example.org/paper.pdf", license="cc-by" + ) + + assert ( + OpenAlexProvider().locate(ReferenceIdentifiers(doi="10.1/x"), config).access_type + == "open" + ) + + +@patch("linkml_reference_validator.etl.fulltext.openalex.requests.get") +def test_green_without_a_licence_is_not_public(mock_get, config): + """``green`` names a repository, not a permission. + + A PMC author manuscript is free to read under a funder policy whose + redistribution terms vary by publisher -- the same shape of claim as bronze, + with a different host. + """ + from linkml_reference_validator.etl.fulltext.openalex import OpenAlexProvider + + mock_get.return_value = _openalex_payload( + "green", pdf_url="https://repo.example.org/paper.pdf", license=None + ) + + assert ( + OpenAlexProvider().locate(ReferenceIdentifiers(doi="10.1/x"), config).access_type + != "open" + ) + + +@patch("linkml_reference_validator.etl.fulltext.unpaywall.requests.get") +def test_unpaywall_green_without_a_licence_is_not_public(mock_get, config): + from linkml_reference_validator.etl.fulltext.unpaywall import UnpaywallProvider + + mock_get.return_value = _unpaywall_payload( + "green", pdf_url="https://repo.example.org/paper.pdf", license=None + ) + + assert ( + UnpaywallProvider().locate(ReferenceIdentifiers(doi="10.1/x"), config).access_type + != "open" + ) + + +@pytest.mark.parametrize("licence", ["other-oa", "implied-oa", "publisher-specific-oa"]) +@patch("linkml_reference_validator.etl.fulltext.openalex.requests.get") +def test_green_with_a_non_licence_sentinel_is_not_public(mock_get, config, licence): + """These name the *absence* of a licence statement, and they are strings. + + A truthiness test reads them as "this deposit states its terms" when they + say the opposite. ``other-oa`` alone covers 5.9M works in OpenAlex, and + Unpaywall's ``implied-oa`` is documented as a copy believed free with no + licence statement found -- which is precisely the bare funder-policy + repository deposit this rule was written about. + """ + from linkml_reference_validator.etl.fulltext.openalex import OpenAlexProvider + + mock_get.return_value = _openalex_payload( + "green", pdf_url="https://repo.example.org/paper.pdf", license=licence + ) + + assert ( + OpenAlexProvider().locate(ReferenceIdentifiers(doi="10.1/x"), config).access_type + != "open" + ) + + +@pytest.mark.parametrize( + "licence", ["cc-by", "cc-by-nc", "cc-by-nc-nd", "cc-by-sa", "cc0", "public-domain"] +) +@patch("linkml_reference_validator.etl.fulltext.openalex.requests.get") +def test_green_with_a_real_licence_is_public(mock_get, config, licence): + """Every CC variant grants verbatim redistribution, which is what caching is. + + ``nc`` restricts commercial use and ``nd`` restricts derivatives; neither + restricts holding a copy, so the whole family qualifies. + """ + from linkml_reference_validator.etl.fulltext.openalex import OpenAlexProvider + + mock_get.return_value = _openalex_payload( + "green", pdf_url="https://repo.example.org/paper.pdf", license=licence + ) + + assert ( + OpenAlexProvider().locate(ReferenceIdentifiers(doi="10.1/x"), config).access_type + == "open" + ) + + +@patch("linkml_reference_validator.etl.fulltext.openalex.requests.get") +def test_a_licence_does_not_rescue_bronze(mock_get, config): + """Only licence-dependent statuses consult the licence.""" + from linkml_reference_validator.etl.fulltext.openalex import OpenAlexProvider + + mock_get.return_value = _openalex_payload( + "bronze", pdf_url="https://publisher.example.org/paper.pdf", license="cc-by" + ) + + assert ( + OpenAlexProvider().locate(ReferenceIdentifiers(doi="10.1/x"), config).access_type + != "open" + ) + + +@patch("linkml_reference_validator.etl.fulltext.openalex.requests.get") +def test_bronze_is_marked_non_open(mock_get, config): + """Free to read on the publisher's site, under no open licence.""" + from linkml_reference_validator.etl.fulltext.openalex import OpenAlexProvider + + mock_get.return_value = _openalex_payload( + "bronze", pdf_url="https://publisher.example.org/paper.pdf" + ) + + location = OpenAlexProvider().locate(ReferenceIdentifiers(doi="10.1/x"), config) + + assert location is not None, "bronze is located, then declined downstream" + assert location.oa_status == "bronze" + assert location.access_type != "open" + + +@patch("linkml_reference_validator.etl.fulltext.unpaywall.requests.get") +def test_unpaywall_bronze_is_marked_non_open(mock_get, config): + from linkml_reference_validator.etl.fulltext.unpaywall import UnpaywallProvider + + mock_get.return_value = _unpaywall_payload( + "bronze", pdf_url="https://publisher.example.org/paper.pdf" + ) + + location = UnpaywallProvider().locate(ReferenceIdentifiers(doi="10.1/x"), config) + + assert location.access_type != "open" + + +@patch("linkml_reference_validator.etl.fulltext.openalex.requests.get") +def test_an_unrecognised_status_is_not_assumed_open(mock_get, config): + """A status we do not know about must not default to redistributable.""" + from linkml_reference_validator.etl.fulltext.openalex import OpenAlexProvider + + mock_get.return_value = _openalex_payload( + "something-new", pdf_url="https://x.example.org/paper.pdf" + ) + + location = OpenAlexProvider().locate(ReferenceIdentifiers(doi="10.1/x"), config) + + assert location.access_type != "open" + + +# -------------------------------------------------------------------------- +# The two rules meet the enrichment chain +# -------------------------------------------------------------------------- + + +def test_bronze_is_skipped_by_ordinary_validation(tmp_path): + """``_enrich_with_full_text`` already declines non-open locations. + + This pins the join: marking bronze non-open is only useful because that + gate exists, and it is the same gate Zotero's private material uses. + """ + from linkml_reference_validator.etl.reference_fetcher import ReferenceFetcher + from linkml_reference_validator.models import ( + FullTextLocation, + ReferenceContent, + ) + + fetcher = ReferenceFetcher(ReferenceValidationConfig(cache_dir=tmp_path)) + content = ReferenceContent( + reference_id="DOI:10.1/x", + content="Abstract only.", + content_type="abstract_only", + doi="10.1/x", + ) + bronze = FullTextLocation( + url="https://publisher.example.org/paper.pdf", + format_hint="pdf", + oa_status="bronze", + access_type="publisher_free", + provider="openalex", + ) + + with patch.object(fetcher, "_apply_full_text_location") as apply_location: + with patch( + "linkml_reference_validator.etl.fulltext.base.FullTextProviderRegistry.get" + ) as get_provider: + provider = MagicMock() + provider.locate.return_value = bronze + get_provider.return_value = provider + fetcher._enrich_with_full_text(content) + + apply_location.assert_not_called() + assert content.content_type == "abstract_only" + + +# -------------------------------------------------------------------------- +# Provenance for a non-open location +# -------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + ("access_type", "expected_url"), + [ + ("open", "https://oa.example.org/paper.pdf"), + ("publisher_free", "https://oa.example.org/paper.pdf"), + ("user_library", None), + ("institutional", None), + # An access type this version has not heard of stays fail-closed, the + # same way an unknown oa_status and an unknown licence do. + ("some-future-scheme", None), + ], +) +def test_a_public_url_is_recorded_even_when_the_licence_is_not_open( + tmp_path, access_type, expected_url +): + """Not redistributable is not the same as not citable. + + The rule that drops ``full_text_url`` was written for a private-library + endpoint -- a localhost Zotero attachment, which is session-specific and + meaningless to anyone else. A bronze publisher PDF link is neither: it is a + stable public URL to a free-to-read article, and the reason its *content* + stays out of the public cache is licensing, not secrecy. Dropping it would + lose real provenance for no privacy gain. + """ + from linkml_reference_validator.etl.reference_fetcher import ReferenceFetcher + from linkml_reference_validator.models import FullTextLocation, ReferenceContent + + fetcher = ReferenceFetcher(ReferenceValidationConfig(cache_dir=tmp_path)) + content = ReferenceContent( + reference_id="DOI:10.1/x", content="Abstract.", content_type="abstract_only" + ) + location = FullTextLocation( + url="https://oa.example.org/paper.pdf", + format_hint="pdf", + oa_status="bronze", + access_type=access_type, + provider="openalex", + ) + + with patch.object( + fetcher, "_materialize", return_value=("body text " * 200, "pdf", None, False) + ): + fetcher._apply_full_text_location(content, "Abstract.", location, "openalex") + + assert content.full_text_url == expected_url + + +# -------------------------------------------------------------------------- +# A decision we made is not a fact about the article +# -------------------------------------------------------------------------- + + +def _fetch_with_location(tmp_path, location): + """Fetch a DOI whose provider chain yields ``location`` (or nothing).""" + from linkml_reference_validator.etl.reference_fetcher import ReferenceFetcher + from linkml_reference_validator.models import ReferenceContent + + fetcher = ReferenceFetcher( + ReferenceValidationConfig(cache_dir=tmp_path, email="me@example.org") + ) + abstract = ReferenceContent( + reference_id="DOI:10.1/x", + content="An abstract.", + content_type="abstract_only", + doi="10.1/x", + ) + + provider = MagicMock() + provider.locate.return_value = location + with patch( + "linkml_reference_validator.etl.fulltext.base.FullTextProviderRegistry.get", + return_value=provider, + ): + fetcher._enrich_with_full_text(abstract) + return abstract + + +def test_a_bronze_decline_does_not_record_that_no_full_text_exists(tmp_path): + """This PR's own thesis, one layer up. + + ``full_text_attempted`` means "a prior clean run concluded none is + available", and ``_maybe_retry_full_text`` uses it to never run the chain + again. A bronze location was not absent — it was found, and declined on + licence. Recording that decision as an absence means a bronze article that + later converts to gold is never noticed. + """ + from linkml_reference_validator.models import FullTextLocation + + reference = _fetch_with_location( + tmp_path, + FullTextLocation( + url="https://publisher.example.org/paper.pdf", + format_hint="pdf", + oa_status="bronze", + access_type="publisher_free", + provider="openalex", + ), + ) + + assert reference.content_type == "abstract_only", "the text is still declined" + assert not reference.full_text_attempted, ( + "a licence decision must stay retryable, as a transient error does" + ) + + +def test_a_landing_page_only_record_does_not_record_that_no_full_text_exists(tmp_path): + """The other new arrival: the provider declined to scrape, so returned nothing. + + A DOI whose only location is an article page is not a DOI with no full text. + It gains a ``pdf_url`` the day a repository copy is deposited. + """ + from linkml_reference_validator.models import FullTextLocation + + reference = _fetch_with_location( + tmp_path, + FullTextLocation( + declined="landing_page_only", oa_status="gold", provider="openalex" + ), + ) + + assert reference.content_type == "abstract_only" + assert not reference.full_text_attempted + + +def test_a_genuine_absence_is_still_recorded(tmp_path): + """The flag must keep meaning something, or the chain re-runs forever. + + A provider that ran cleanly and found nothing at all — no location, no + decline — is the case the flag was introduced for. + """ + from linkml_reference_validator.etl.reference_fetcher import ReferenceFetcher + from linkml_reference_validator.models import ReferenceContent + + fetcher = ReferenceFetcher( + ReferenceValidationConfig(cache_dir=tmp_path, email="me@example.org") + ) + abstract = ReferenceContent( + reference_id="DOI:10.1/x", content="An abstract.", + content_type="abstract_only", doi="10.1/x", + ) + + # A provider that ran and found nothing -- not an unregistered one, which + # is skipped before it is ever consulted and would pass this test even if a + # bare ``None`` also counted as a decline. + provider = MagicMock() + provider.locate.return_value = None + with patch( + "linkml_reference_validator.etl.fulltext.base.FullTextProviderRegistry.get", + return_value=provider, + ): + fetcher._enrich_with_full_text(abstract) + + provider.locate.assert_called() + assert abstract.full_text_attempted + + +def test_a_decline_is_remembered_so_the_chain_is_not_rewalked_every_run(tmp_path): + """Retryable must not mean re-walked on every run, forever. + + A transient error clears itself: the next run succeeds. A policy decline + never does — bronze stays bronze until the publisher changes it, which may + be never. Leaving nothing recorded means every run re-enters the chain for + every such reference: four providers, each opening with a + ``rate_limit_delay`` sleep, against the same hosts whose rate limiting + produces the interstitial this change exists to defend against. + + Measured on a real corpus, 23,465 cached entries are eligible; at ~2s of + sleep apiece that is around thirteen hours per validation run, before the + HTTP requests. So the decline is recorded against the extractor version: + retried when that moves, not every run. + """ + from linkml_reference_validator.etl.reference_fetcher import ReferenceFetcher + from linkml_reference_validator.models import FullTextLocation, ReferenceContent + + declined = FullTextLocation( + declined="landing_page_only", oa_status="gold", provider="openalex" + ) + calls = [] + + def _run(): + fetcher = ReferenceFetcher( + ReferenceValidationConfig(cache_dir=tmp_path, email="me@example.org") + ) + source = MagicMock() + source.return_value.fetch.return_value = ReferenceContent( + reference_id="DOI:10.1/x", content="An abstract.", + content_type="abstract_only", doi="10.1/x", + ) + provider = MagicMock() + provider.locate.side_effect = lambda *a, **k: (calls.append(1), declined)[1] + with patch( + "linkml_reference_validator.etl.reference_fetcher.ReferenceSourceRegistry.get_source", + return_value=source, + ), patch( + "linkml_reference_validator.etl.fulltext.base.FullTextProviderRegistry.get", + return_value=provider, + ): + return fetcher.fetch("DOI:10.1/x") + + _run() + first = len(calls) + assert first >= 1, "the first run consults the chain" + + _run() + assert len(calls) == first, ( + "a second run, in a fresh fetcher, must not re-walk the chain" + ) + + +def test_a_recorded_decline_still_reads_as_retryable_not_as_an_absence(tmp_path): + """The distinction round 11 established must survive being remembered. + + Suppressing the re-walk must not be done by setting ``full_text_attempted``, + whose meaning is "a clean run concluded none is available". The decline is + recorded as itself. + """ + from linkml_reference_validator.etl.reference_fetcher import ReferenceFetcher + from linkml_reference_validator.models import FullTextLocation, ReferenceContent + + fetcher = ReferenceFetcher( + ReferenceValidationConfig(cache_dir=tmp_path, email="me@example.org") + ) + abstract = ReferenceContent( + reference_id="DOI:10.1/x", content="An abstract.", + content_type="abstract_only", doi="10.1/x", + ) + provider = MagicMock() + provider.locate.return_value = FullTextLocation( + declined="landing_page_only", oa_status="gold", provider="openalex" + ) + with patch( + "linkml_reference_validator.etl.fulltext.base.FullTextProviderRegistry.get", + return_value=provider, + ): + fetcher._enrich_with_full_text(abstract) + + assert not abstract.full_text_attempted, "still not an absence" + assert abstract.full_text_declined == "landing_page_only", "but it is remembered" + + +@pytest.mark.parametrize("failure", ["rate_limit", "elink_outage"]) +def test_a_transient_pmc_failure_is_not_recorded_as_an_absence(tmp_path, failure): + """The defect this release is about, in the host that motivated it. + + PMC answers a rate-limited client with 429, and an Entrez ``elink`` outage + raises. Both used to return ``None`` from ``locate``, which the chain reads + as a clean absence and records as ``full_text_attempted`` -- so an article + that *does* have a PMC body, fetched during a rate-limit window, is + permanently marked as having none. + """ + from contextlib import ExitStack + + from linkml_reference_validator.etl.fulltext.pmc import PMCFullTextProvider + from linkml_reference_validator.etl.reference_fetcher import ReferenceFetcher + from linkml_reference_validator.models import ReferenceContent + + fetcher = ReferenceFetcher( + ReferenceValidationConfig( + cache_dir=tmp_path, email="me@example.org", rate_limit_delay=0 + ) + ) + content = ReferenceContent( + reference_id="PMID:1", content="An abstract.", content_type="abstract_only" + ) + + rate_limited = MagicMock() + rate_limited.status_code = 429 + + with ExitStack() as stack: + stack.enter_context( + patch( + "linkml_reference_validator.etl.fulltext.base." + "FullTextProviderRegistry.get", + return_value=PMCFullTextProvider(), + ) + ) + stack.enter_context( + patch.object(PMCFullTextProvider, "_fetch_pmc_xml_source", return_value=None) + ) + if failure == "rate_limit": + stack.enter_context( + patch.object(PMCFullTextProvider, "_resolve_pmcid", return_value="123456") + ) + stack.enter_context( + patch( + "linkml_reference_validator.etl.fulltext.pmc.requests.get", + return_value=rate_limited, + ) + ) + else: + stack.enter_context( + patch.object( + PMCFullTextProvider, + "_resolve_pmcid", + side_effect=RuntimeError("elink down"), + ) + ) + fetcher._enrich_with_full_text(content) + + assert not content.full_text_attempted, f"{failure} is not an absence" + + +def test_cache_enrich_does_not_count_a_declined_location_as_found(tmp_path): + """``cache enrich``'s entire output is an inventory, so a refusal is not a find. + + A declined location carries no ``url`` and no ``text``; counting it inflates + ``Found:`` by exactly the references the provider refused, and under + ``--apply`` it reports ``unusable`` rather than crashing, which reads as a + provider fault rather than a decision. + """ + from typer.testing import CliRunner + + from linkml_reference_validator.cli import app + from linkml_reference_validator.etl.reference_fetcher import ReferenceFetcher + from linkml_reference_validator.models import FullTextLocation, ReferenceContent + + cache_dir = tmp_path / "cache" + fetcher = ReferenceFetcher(ReferenceValidationConfig(cache_dir=cache_dir)) + fetcher._save_to_disk( + ReferenceContent( + reference_id="DOI:10.1/x", content="An abstract.", + content_type="abstract_only", doi="10.1/x", title="A paper", + ) + ) + + declined = FullTextLocation( + declined="landing_page_only", oa_status="gold", provider="openalex" + ) + with patch.object(ReferenceFetcher, "locate_full_text", return_value=declined): + result = CliRunner().invoke( + app, + ["cache", "enrich", "--provider", "openalex", "--dry-run", + "--cache-dir", str(cache_dir)], + ) + + assert "\tfound\t" not in result.output, result.output + assert "declined" in result.output, result.output