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