Skip to content

fix(bib): a PubMed entry was reported as having no identifier; it has a PMID (#500) - #576

Merged
sotashimozono merged 5 commits into
nextfrom
fix/500-say-which-identifier
Aug 31, 2026
Merged

sotashimozono merged 5 commits into
nextfrom
fix/500-say-which-identifier

Conversation

@sotashimozono

Copy link
Copy Markdown
Member

Refs #500 — the reporting half. Ref::Pmid is blocked on a cross-repo contract; see the end. The issue stays open.

What was wrong

entry has no DOI / arXiv id is accurate about what the parser did and wrong about the entry. A user reading it goes and edits a .bib that was fine, because the sentence puts the deficiency on their side. It is on ours.

An existing test pinned exactly that claim:

fn bibtex_non_arxiv_eprinttype_is_skipped() {
    // eprinttype names a different server → not an arXiv id, and
    // there is no DOI, so the entry has no resolvable identifier.
    let body = "@article{x, eprint = {12345678}, eprinttype = {pubmed}}";
    assert!(matches!(res, Err(ParseError::NoIdentifier { .. })));
}

The entry has a PMID. It is not resolvable by doiget, which is a different statement.

The change

Both PubMed shapes are named: the pmid = {...} field PubMed's own BibTeX export writes, and the BibLaTeX eprint + eprinttype = {pubmed} pair that arxiv_eligible already refused — correctly, and until now silently. pmcid too.

NOT_IMPLEMENTED, not INVALID_REF. The input is valid and the support is absent, and the two codes carry opposite advice: "wait for a release" versus "correct your input". Disposition terminal follows.

An entry with genuinely nothing still reports NoIdentifier, and a DOI alongside a PMID still wins — the check runs only after every supported identifier has been tried, so it cannot divert an entry doiget could have resolved. Both are tested.

ParseError is #[non_exhaustive], so the compiler was no help

Adding the variant compiled everywhere. All three consumers have wildcards, and in the MCP one the new variant would have fallen into Err(_) => "unhandled bibliography parse error" — losing the identifier it had just identified. Explicit arms added at all three, found by reading them rather than by the build.

Why Ref::Pmid is not here

I checked the internal contract before writing any of it, and it is the blocker rather than NCBI.

docs/SAFEKEY.md opens with:

Status: NORMATIVE (shared spec). Binding for both doiget and BiblioFetch.jl. Any change requires a coordinated ADR and an update to the reference test vectors.

and its reference Julia implementation discriminates ref classes as:

raw = startswith(ref, "10.") ? "doi_$ref" : "arxiv_$ref"

A PMID is neither. 9659853 would be keyed arxiv_9659853 by BiblioFetch.jl and pmid_9659853 by doiget — a store one tool writes that the other misreads. Adding the class needs a coordinated ADR, a vectors.json update, and a matching change in another repository, which is not mine to land unilaterally.

NCBI is fine — verified, not assumed

Against the primary documentation (NBK25497) and a live round-trip, before designing anything:

base https://eutils.ncbi.nlm.nih.gov/entrez/eutils/
rate 3 req/sec without a key, 10 with one
key optional
tool / email encouraged; mandatory only to lift an IP block
large jobs weekends, or 21:00–05:00 ET on weekdays
esearch.fcgi?db=pubmed&term=10.1176/ajp.155.7.895[AID]   →  9659853
esummary.fcgi?db=pubmed&id=9659853                       →  articleids[] doi = 10.1176/ajp.155.7.895

So the resolver #500 proposes does work, and it is the same Coryell 1998 paper #536 was about. Worth noting for whoever picks up step 2: doiget's global cap is 5/sec while NCBI's keyless limit is 3, so it needs a SOURCE_RATE_OVERRIDES entry the way arXiv does — the cap alone would exceed NCBI's published rate.

Local: fmt, clippy -D warnings, workspace suite 47/47, version-bump-gate.sh.

PMID

`entry has no DOI / arXiv id` is accurate about what the parser did and
wrong about the entry. A user reading it goes and edits a bibliography
that was fine, because the sentence puts the deficiency on their side.
It is on ours.

An existing test pinned exactly that claim -- `eprinttype = {pubmed}`,
asserted as `NoIdentifier`, with the comment "the entry has no
resolvable identifier". The entry has a PMID. It is not resolvable BY
DOIGET, which is a different statement and the one #500 is about.

Both PubMed shapes are named now: the `pmid = {...}` field PubMed's own
BibTeX export writes, and the BibLaTeX `eprint` + `eprinttype = {pubmed}`
pair that `arxiv_eligible` already refused -- correctly, and until now
silently. `pmcid` too.

NOT_IMPLEMENTED, not INVALID_REF. The input is valid and the support is
absent, and the two codes carry opposite advice: "wait for a release"
versus "correct your input". Disposition `terminal` follows (ADR-0055).

`ParseError` is `#[non_exhaustive]`, so adding a variant compiled
everywhere and the compiler could not help. All three consumers had
wildcards, and in the MCP one the new variant would have become
"unhandled bibliography parse error" -- losing the identifier it had
just identified. Explicit arms added at all three; found by reading them
rather than by the build.

Scope: this is the REPORTING half of #500. `Ref::Pmid` is blocked on
something the issue did not anticipate, and I checked before writing any
of it: `docs/SAFEKEY.md` is "NORMATIVE (shared spec). Binding for both
doiget and BiblioFetch.jl", its reference Julia implementation
discriminates ref classes as `startswith(ref, "10.") ? "doi_" :
"arxiv_"`, and a PMID is neither. Adding the class needs a coordinated
ADR, a vectors update, and a matching change in another repository.
Details on the issue.

Verified against NCBI's primary documentation and a live round-trip
before designing any of it: base `https://eutils.ncbi.nlm.nih.gov/
entrez/eutils/`, 3 req/sec without a key and 10 with one, key optional,
`tool` and `email` encouraged. `esearch?term=<DOI>[AID]` returns PMID
9659853 for `10.1176/ajp.155.7.895`, and `esummary` returns that DOI
back under `articleids[].idtype == "doi"` -- so the resolver the issue
proposes does work. It is the safekey contract that blocks it, not NCBI.

Refs #500

Signed-off-by: Souta <shimozono-sota631@g.ecc.u-tokyo.ac.jp>
…ntifier

Signed-off-by: Souta <shimozono-sota631@g.ecc.u-tokyo.ac.jp>

# Conflicts:
#	CHANGELOG.md
#	Cargo.lock
#	Cargo.toml
@codecov

codecov Bot commented Aug 31, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 67.83217% with 46 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
crates/doiget-cli/src/commands/verify.rs 0.00% 18 Missing ⚠️
crates/doiget-mcp/src/lib.rs 0.00% 15 Missing ⚠️
crates/doiget-cli/src/commands/batch.rs 0.00% 7 Missing ⚠️
crates/doiget-core/src/refs.rs 94.05% 6 Missing ⚠️

📢 Thoughts on this report? Let us know!

…ntifier

Signed-off-by: Souta <shimozono-sota631@g.ecc.u-tokyo.ac.jp>

# Conflicts:
#	Cargo.lock
#	Cargo.toml
…s lost

codecov/patch was red on this PR and it was right: nothing drove the
three arms this change adds. `refs.rs` tests the parser; the JSONL
placeholder, the verify row and the MCP envelope -- the things a user
actually reads -- were asserted nowhere.

Looking at them found the reason to care. The same sentence had been
hand-copied to `verify.rs` and `doiget-mcp`, and both copies had been
wrapped across source lines and re-joined with the indentation still in
them:

    "...which doiget                          cannot resolve yet..."

That is the defect the #580 review fixed elsewhere, reintroduced twice
by me in the same cycle, and it shipped because no test read the string.

So the fix is the cause, not the symptom: `unsupported_identifier_claim`
in `doiget_core::refs`, called by both sites, tested once.
`ParseError`'s own `#[error]` already carried the claim -- the copies
existed only to drop the `entry {entry_key:?}` prefix, which each caller
puts in a field of its own.

Three more of the same class turned up, all pre-existing on next and all
user-facing: two `= note:` lines in `fetch`'s widening advice, and the
`config.toml could not be read` warning in `commands/mod.rs`. Fixed.

A workspace-wide lint for the pattern is deliberately NOT here. Written,
measured, dropped: it flags `config doctor`'s aligned two-column output
and test assertion messages at every space threshold from four to ten
(12 hits, 3 of them real), and the aligned output uses the same 8+ space
runs the defect does. There is no structural discriminator, and a lint
that cries wolf is a lint someone deletes. Removing the duplication is
the half that lasts.

Refs #500

Signed-off-by: Souta <shimozono-sota631@g.ecc.u-tokyo.ac.jp>
The shared-helper call fits on one line; I had left it wrapped the way
the `format!` it replaced was.

Signed-off-by: Souta <shimozono-sota631@g.ecc.u-tokyo.ac.jp>
@sotashimozono
sotashimozono merged commit 5aeef76 into next Aug 31, 2026
47 of 48 checks passed
github-actions Bot added a commit that referenced this pull request Aug 31, 2026
…tifier

fix(bib): a PubMed entry was reported as having no identifier; it has a PMID (#500) 5aeef76
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant