Add Zotero private-library cache workflows

[cmungall]

Summary

• add a read-only Zotero local API provider with exact DOI, PMID, and PMCID matching, indexed-text preference, and PDF fallback
• add cache enrich with dry-run inventory and owner-only private research-cache output
• keep ordinary validation deterministic by ignoring private caches and rejecting non-public provider results
• add metadata-only, DOI/PMID-deduplicated CSL JSON export for creating Zotero collections
• safely handle Zotero localhost file redirects without permitting arbitrary file:// redirects
• document the end-to-end cache → Zotero → Find Full Text → private enrichment workflow, including an observed screenshot

Why

Users may lawfully possess closed manuscripts in Zotero, but those files must not enter a checked-in public reference cache or make validation results differ between local machines and CI. This separates private research context from public validation evidence while providing an explicit workflow for finding and inventorying PDFs.

User impact

linkml-reference-validator cache export \
  --cache-dir references_cache \
  --needs-full-text \
  --output project-zotero.json

linkml-reference-validator cache enrich \
  --provider zotero \
  --cache-dir references_cache \
  --dry-run

The export contains an explicit bibliographic metadata allowlist only. Private apply mode defaults to ~/.cache/linkml-reference-validator/private with owner-only permissions.

Validation

• just test: 647 passed; mypy and Ruff passed
• just doctest: 191 passed
• uv run mkdocs build --strict: passed
• real dismech cache smoke test: 34,675 scanned, 24,701 metadata-only records exported, 581 duplicates removed
• live Zotero local API and Find Full Text workflow exercised successfully

[claude]

Claude encountered an error after 1s —— View job

---

I'll analyze this and get back to you.

[claude]

Claude finished @cmungall's task in 3m 12s —— View job

---

Review: Zotero private-library cache workflows

• Gather context / read diff
• Review new modules (zotero.py, csl_export.py, cli/cache.py)
• Review modified modules (acquire.py, reference_fetcher.py, models.py)
• Review test coverage
• Post review

Overall this is a well-structured PR. The public/private separation is the right architecture: _load_from_disk never reads the private cache, _enrich_with_full_text rejects any location whose access_type isn't None/open, full_text_url is dropped for user_library results, and zotero is deliberately absent from full_text_providers' default chain — so ordinary validation stays reproducible across machines and CI. Docs and tests are thorough. Below are the issues I found, roughly by severity.

---

1. allow_redirects=False is a regression for all non-Zotero downloads (high)

src/linkml_reference_validator/etl/acquire.py:126-154

fetch_bytes is the shared download path for every provider (pmc, epmc_preprint, unpaywall, openalex) and for the url: source (etl/sources/url.py:74). Turning off requests' redirect handling to intercept Zotero's file:// hop changes behavior for all of them:

• Relative Location headers now fail. Location: /pdf/foo.pdf is legal per RFC 9110 §10.2.2 and common at publishers. urlparse("/pdf/foo.pdf").scheme is "", so it falls through to logger.warning("Unsupported redirect scheme") and returns (None, None) — a silent full-text loss. Fix with urljoin(url, location) before dispatching.
• Cookies are dropped across hops. With allow_redirects=True, Session.resolve_redirects carries Set-Cookie from intermediate responses into subsequent hops. Each recursive _fetch_bytes call here is a fresh requests.get with a fresh session, so publishers that set a session cookie and then redirect to the PDF will now fail.
• Redirect budget shrank from 30 to 5. doi.org → publisher → CDN → PDF chains can exceed 5.
• Each hop re-runs time.sleep(config.rate_limit_delay).

Narrowest fix: keep allow_redirects=True for ordinary URLs and only take the manual path when the target is loopback, i.e. gate on self._is_loopback_url(url). That confines the new logic to the Zotero case and leaves the existing OA fetch path untouched.

Fix this →

2. A fresh ZoteroClient is built per reference, re-downloading the whole library index each time (high)

src/linkml_reference_validator/etl/fulltext/zotero.py:196

client = self._client or ZoteroClient(config.zotero_base_url)

_identifier_index is cached on the client instance, but locate() constructs a new client on every call when none was injected. FullTextProviderRegistry holds a single provider instance, so the index could be built once — instead cache enrich over the 34k-record cache mentioned in the PR description issues 34k full items/top requests, each parsing the entire library. Caching on the provider (keyed by config.zotero_base_url, so a changed config still rebuilds) makes this O(1).

Fix this →

3. items/top is unpaginated — silent false negatives on real libraries (medium-high)

src/linkml_reference_validator/etl/fulltext/zotero.py:130

_get_identifier_index issues a single GET items/top with no limit/start. The Zotero v3 API paginates (default page size, Total-Results header, Link: rel="next"), and the local API mirrors it. The plan doc (docs/superpowers/plans/2026-08-05-private-library-full-text.md:34,92) correctly defers pagination to a later phase — but the failure mode today is silent: everything past the first page reports not_found, which is indistinguishable from a genuine miss.

Even without implementing pagination now, please read Total-Results and emit a logger.warning (or a stderr note from cache enrich) when the returned item count is less than the total, so users know the inventory is truncated.

Fix this →

4. apply_full_text_location(private=False) can write private text to the public cache (medium)

src/linkml_reference_validator/etl/reference_fetcher.py:283-299

_save_by_access derives the destination from content.full_text_access_type — good. But the new public apply_full_text_location takes private as an independent caller-supplied flag and passes it straight through to _save_to_disk. A caller that passes a user_library location with the default private=False writes closed-access manuscript text into the checked-in public cache — exactly the outcome the PR exists to prevent. Since this is now public API, make the flag a floor rather than the sole authority:

private = private or location.access_type == "user_library"

Fix this →

5. iter_cached_references materializes the entire cache in memory (medium)

src/linkml_reference_validator/etl/reference_fetcher.py:300-308

It returns a list[ReferenceContent] with every record's full body text loaded. The PR reports a 34,675-record real cache; many of those are full-text records, so this can be several GB resident. Both enrich and export only ever iterate once. Making it a generator (Iterator[ReferenceContent]) and tracking scanned with a counter would make both commands stream. The name already reads like a generator.

Fix this →

6. CSL export drops the PMID when a DOI is present (medium)

src/linkml_reference_validator/etl/csl_export.py:85-89

if doi:
    record["DOI"] = doi
elif pmid:
    record["PMID"] = pmid
    record["URL"] = ...

The documented round trip is export → Zotero → Find Full Text → cache enrich, and ZoteroClient.find_parent_keys matches on DOI and PMID/PMCID from Zotero's Extra field. Dropping the PMID for DOI-bearing records means those Zotero items carry only one matchable identifier, so any DOI mismatch (registration variants, publisher-changed DOIs) makes the item unmatchable even though the PMID would have worked. There's no reason for these to be exclusive — emit both, and keep the elif only for the URL fallback.

Fix this →

7. Smaller points

• Duplicate normalize_doi. etl/csl_export.py:9 and etl/fulltext/zotero.py:38 are two different implementations of the same concept (regex vs. prefix-strip; the zotero one also handles dx.doi.org). They will drift. Hoist one into a shared util and import it in both.
• ReferenceFetcher.needs_full_text (reference_fetcher.py:176) is a one-line public alias for _needs_full_text. Prefer promoting the private method and updating the two internal call sites over keeping both names.
• attachments[0] (zotero.py:206). When a parent has several PDF children (e.g. accepted manuscript + published version + supplement) the pick depends on Zotero's response order. A deterministic tiebreak — or at least a logger.info noting the multi-attachment case — would help reproducibility, matching the care taken with find_parent_keys' ambiguity handling.
• get_private_cache_dir chmods unconditionally (models.py:534-541). Calling chmod(0o700) on every invocation silently reverts any deliberate user permission change. Consider chmodding only on creation, or when the mode is broader than 0o700. Also note the intermediate ~/.cache/linkml-reference-validator parent is created with the default umask — harmless since the leaf is 0o700, but worth knowing.
• _read_local_file and Windows (acquire.py:196). Path(unquote(parsed.path)) on file:///C:/x.pdf yields /C:/x.pdf. Not a blocker if Windows isn't supported, but url2pathname from urllib.request handles this portably.
• enrich doesn't fail fast. If Zotero isn't running, every record hits the except Exception branch and prints an error line — 34k lines before the summary. A short-circuit after N consecutive provider errors would be kinder.
• apply_full_text_location (reference_fetcher.py:293) does self._cache[content.reference_id] = content using the raw id, whereas fetch_reference keys self._cache by normalize_reference_id(...). Unreachable today (the only caller passes private=True), but it's an inconsistency waiting to bite.

Test coverage

Good coverage overall — provider matching/ambiguity, the redirect security boundary, the size cap, private-cache permissions, and export dedup/allowlisting are all exercised, and test_fetch_bytes_refuses_remote_redirect_to_local_file is exactly the right test to have written. Gaps that map to the findings above:

• No test for a relative Location header (issue 1) — this is the case that currently breaks.
• No test that the identifier index is fetched once across multiple locate() calls (issue 2).
• No test for a multi-page items/top response (issue 3).
• No test that a user_library location with private=False stays out of the public cache (issue 4).

Nothing here blocks the design; issues 1 and 2 are the ones I'd want addressed before merge, since 1 silently degrades existing open-access fetching and 2 makes the headline cache enrich workflow quadratic in library size.
• branch paperpile

[claude]

Claude finished @cmungall's task in 3m 14s —— View job

---

Re-review: Zotero private-library cache workflows

• Check what changed since the last review (89d5c2a)
• Verify prior findings 1–7
• Look for new issues introduced by the fixes
• Post review

89d5c2a lands after my previous review and addresses all seven findings, each with a targeted test. Verification below, then a short list of small things the fixes surfaced. Nothing here blocks merge.

---

Prior findings — verified fixed

#	Finding	Status
1	allow_redirects=False regressed all non-Zotero downloads	✅ acquire.py:127 gates on manual_redirects = self._is_loopback_url(url); relative Location now goes through urljoin (:141). Tests at tests/test_acquire.py:183,204 assert allow_redirects is True for publisher URLs and correct resolution of /download/PDF1 → http://localhost:23119/download/PDF1.
2	New ZoteroClient per reference	✅ zotero.py:210-216 caches on the provider, keyed on config.zotero_base_url.rstrip("/"), and never clobbers an injected client. test_provider_reuses_default_client_between_references asserts constructor.assert_called_once_with(...).
3	items/top unpaginated	✅ Real pagination in _get_paginated_items (zotero.py:146-170) using limit/start + Total-Results, not just a warning. test_client_paginates_top_level_items checks both page params.
4	apply_full_text_location(private=False) could leak to the public cache	✅ reference_fetcher.py:289 — private = private or location.access_type not in (None, "open"). Better than what I suggested: the not in (None, "open") form generalizes to future non-open access types instead of special-casing user_library, and the same predicate is now used consistently in _save_by_access and for dropping full_text_url.
5	iter_cached_references materialized the whole cache	✅ Now an Iterator; both CLI commands count with scanned.
6	CSL export dropped PMID when DOI present	✅ Both emitted; URL kept as the PMID-only fallback.
7	Smaller points	✅ All: normalize_doi hoisted to etl/identifiers.py (regex version, keeps dx.doi.org); _needs_full_text promoted rather than aliased; deterministic min(keyed_attachments, key=...) attachment pick; conditional chmod that only tightens (mode & 0o077 guard); url2pathname; MAX_CONSECUTIVE_PROVIDER_ERRORS = 3 short-circuit; _cache keyed by normalize_reference_id.

The four test gaps I listed are all closed.

---

New/remaining observations

1. The file:// refusal branch is now unreachable in production (low)

src/linkml_reference_validator/etl/acquire.py:143-148

The 3xx block is only entered manually when manual_redirects is true, which is exactly _is_loopback_url(url) — so the inner if not self._is_loopback_url(url) guard can never fire. test_fetch_bytes_refuses_remote_redirect_to_local_file still passes, but only because the MagicMock ignores allow_redirects=True and hands back a 302; against a real server, requests would follow the hop itself and raise InvalidSchema: No connection adapters were found for 'file://...'.

The security property still holds (requests won't dispatch file://), and keeping the branch as defense-in-depth is reasonable. Two knock-on effects worth knowing:

• _materialize (reference_fetcher.py:355-359) catches that InvalidSchema and returns error=True, i.e. transient — so full_text_attempted stays unset and a server that always redirects to file:// gets re-queried on every run instead of being recorded as a definitive miss.
• etl/sources/url.py:74 calls fetch_bytes with no try, so the same exception propagates out of URLSource.fetch rather than returning None.

Both are edge cases. A one-line comment marking the guard as intentionally-redundant defense-in-depth would stop a future reader from "simplifying" it away.

Fix this →

2. _get_paginated_items has no page/iteration bound (low)

src/linkml_reference_validator/etl/fulltext/zotero.py:146-170

Termination relies on the server honouring start. If Total-Results is absent and the server returns a full page every time (ignoring start), the while True never exits and items grows without bound. Also, int(total_header) on line 164 raises a bare ValueError for a malformed header rather than the module's ZoteroAPIError — inconsistent with _require_success's otherwise careful external-boundary handling.

A max_pages (or max-items) guard plus a logger.warning when it trips would bound the failure. Same block: if a caller passes page_size > 100, Zotero clamps limit to 100, so the len(page) < self._page_size fallback (line 168) would break early and silently truncate the index when Total-Results is missing — clamping page_size to 100 in __init__ would remove that footgun.

Fix this →

3. chmod(mode & 0o700) can produce an unusable directory (very low)

src/linkml_reference_validator/models.py:535-541

The conditional chmod is the right shape and the new test pins that 0o500 is left alone. One theoretical hole: mode & 0o700 preserves whatever owner bits exist, so a pre-existing 0o070 directory becomes 0o000 and subsequent writes fail with a confusing PermissionError. (mode & 0o700) | 0o700 — or simply 0o700 when the mode is broader — would be safer, though this only arises if a user has deliberately created a directory they can't use.

4. Provider-level index caching has no invalidation (informational)

zotero.py:210-216 caches the client, and therefore _identifier_index, for the provider's lifetime. FullTextProviderRegistry holds provider instances, so in a long-lived process the index reflects the library as of first use — items added to Zotero mid-run won't be seen. That's exactly what makes issue 2 fast, and the module docstring already says "process-local", so I'd leave it. Just worth a sentence in the cache enrich docs noting that a Zotero library changed mid-run requires a re-run.

---

Test coverage

Good and improved. test_private_location_forces_private_persistence_without_flag, test_provider_selects_pdf_attachment_deterministically, test_client_paginates_top_level_items, test_fetch_bytes_resolves_relative_zotero_redirect, and test_private_cache_does_not_broaden_existing_owner_permissions each map directly onto a finding. assert iter(references) is references is a nice way to pin the generator contract so a future refactor can't quietly revert to a list.

The one soft spot is described in observation 1: the remote-redirect-to-file:// test now validates a branch the production code path can't reach.

⚠️ I could not run just test / pytest in this job — the bash commands needed approval in this environment. The verification above is from reading the diff and the code. The PR description reports 647 passing plus 191 doctests, and CI should confirm.

Overall: the two issues I wanted addressed before merge (1 and 2 from the first pass) are properly fixed, not papered over — the redirect fix is scoped to loopback exactly as suggested, and pagination was implemented rather than just warned about. The remaining points are all low-severity polish. Looks good to merge.
• branch paperpile