fix: pace concurrent NCBI research requests - #239
Merged
Conversation
NCBI allows 3 E-utilities requests/sec/IP, 10 with an API key. Pi runs sibling tool calls from one assistant message concurrently, so a single research turn fires a dozen PubMed requests at once and most come back 429. Nothing limited how fast they went out. Add a small in-process gate that spaces request starts, and forward NCBI_API_KEY, which ncbiIdentityParams() never sent even though scrubEndpoint() below it already redacted an api_key param. Measured live, twelve concurrent searches through the registered tool: 0/12 on main, 12/12 with spacing, both anonymously and with a key. Retry is deliberately not part of this. NCBI sends no Retry-After, so a rate-limited burst retries in lockstep, and with spacing in place the result is 12/12 either way. The PMC ID Converter is not sent the key: it is a separate service with no documented api_key support, and attaching one would also pace it at the keyed rate.
…ctual starts Two defects found by review, both with repros. send() cleared the abort timer as soon as fetch resolved, which is when headers arrive, so the body read ran unprotected. A stalled body hung the tool call forever where main aborted it after the request timeout. send() now takes the reader and awaits it inside the try, which also closes the same pre-existing hole in fetchText. The gate reserved absolute wake times upfront. One long tick left every reservation overdue and the whole burst then started at once: measured 0ms gaps after a 700ms event-loop block. Each waiter now measures from the previous request's actual start. Tests were passing with the gate disabled. Mutating ANONYMOUS_MIN_GAP_MS to 0 or dropping the ID Converter host went undetected. Cover the anonymous rate, the ID Converter host, loop-block resilience, and the body-read timeout, and add scripts/ncbi-burst-check.mjs so the burst is reproducible.
The budget is per-IP, so gating PubMed alone left the reported failure in place: a turn mixing pubmed with clinvar or geo still put 21 requests into one rolling second against a ceiling of 3. Route the specialty, variants, and omics-archive fetch helpers through the same gate. They carry no API key, and the interval is read from the outgoing URL, so they pace at the anonymous rate rather than the keyed one. Non-NCBI hosts still return immediately, so their other backends are unaffected. Add NCBI_MIN_REQUEST_GAP_MS to override the interval, for shared or institutional IPs that need more room. The timeout test asserted only that a stalled request had not resolved, which the regression also satisfies by hanging, so it passed with the bug reintroduced. Give it a real budget through a test seam and assert the rejection, racing a deadline so a regression fails the case instead of wedging the run. Live, twelve concurrent calls split between pubmed and clinvar: 12/12 with no rate limiting.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This was referenced Aug 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
NCBI_API_KEYto E-utilities while redacting it from provenanceThis ports the three exact code commits from #238 onto a maintainer-owned branch, then adds the missing public configuration documentation. It preserves contributor authorship on the code commits.
Closes #237.
Supersedes #238 after this branch passes maintainer CI.
Verified
main:0/12succeeded,12rate-limited12/12succeeded,0rate-limited10/10npm test:803/803npm run typechecknpm run buildnpm run architecture:check0 vulnerabilities34pagesgit diff --check