Add PubMed no-auth chat tools app#7485
Conversation
Greptile SummaryAdds a new self-contained Omi plugin app that exposes three PubMed chat tools (
Confidence Score: 3/5The app works correctly for single sequential requests but will degrade significantly under any concurrent load due to blocking HTTP calls in async handlers. Every tool endpoint makes one or two synchronous
Important Files Changed
Sequence DiagramsequenceDiagram
participant Omi as Omi Chat
participant App as PubMed App (FastAPI)
participant NCBI as NCBI E-utilities
Omi->>App: POST /tools/search_pubmed
App->>NCBI: GET esearch.fcgi (blocking)
NCBI-->>App: idlist
App->>NCBI: GET esummary.fcgi (blocking)
NCBI-->>App: summaries
App-->>Omi: ChatToolResponse
Omi->>App: POST /tools/get_pubmed_article
App->>NCBI: GET esummary.fcgi (blocking)
NCBI-->>App: record
App-->>Omi: ChatToolResponse
Omi->>App: POST /tools/get_related_pubmed
App->>NCBI: GET elink.fcgi (blocking)
NCBI-->>App: linksets
App->>NCBI: GET esummary.fcgi (blocking)
NCBI-->>App: summaries
App-->>Omi: ChatToolResponse
Reviews (1): Last reviewed commit: "Add PubMed no-auth chat tools plugin" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| "retmax": max(1, min(retmax, 20)), | ||
| "sort": "relevance", | ||
| } | ||
| resp = requests.get(f"{EUTILS}/esearch.fcgi", params=params, timeout=TIMEOUT) |
There was a problem hiding this comment.
_search_ids, _fetch_summaries, and the direct requests.get call inside get_related_pubmed (line 211) are synchronous, but they are called from async def handlers. FastAPI runs on an asyncio event loop, so a synchronous requests.get that waits up to 20 seconds (TIMEOUT) blocks all concurrent requests from being processed. Under any concurrent load the server becomes unresponsive. Replace requests with httpx.AsyncClient and await the calls, or run the blocking helpers via asyncio.to_thread().
| "db": "pubmed", | ||
| "term": query, | ||
| "retmode": "json", | ||
| "retmax": max(1, min(retmax, 20)), |
There was a problem hiding this comment.
max_results cap is inconsistent between manifest and implementation
The manifest advertises max_results as "1-10" for search_pubmed, but _search_ids clamps to min(retmax, 20), allowing up to 20. A caller trusting the manifest description and passing 10 will actually get up to 10 results, while the server silently allows twice that. The cap should match the documented range.
| "retmax": max(1, min(retmax, 20)), | |
| "retmax": max(1, min(retmax, 10)), |
| pmid = (body.get("pmid") or "").strip() | ||
| if not pmid: | ||
| return ChatToolResponse(error="pmid is required") | ||
|
|
||
| summaries = _fetch_summaries([pmid]) |
There was a problem hiding this comment.
pmid is accepted as any arbitrary string and forwarded directly to the NCBI API. A non-numeric value (e.g. "abc") results in an NCBI API error that surfaces only as a generic "Failed to fetch" message. Validating that the value is numeric before the network call gives the caller a clearer error and avoids a wasted round-trip.
| pmid = (body.get("pmid") or "").strip() | |
| if not pmid: | |
| return ChatToolResponse(error="pmid is required") | |
| summaries = _fetch_summaries([pmid]) | |
| pmid = (body.get("pmid") or "").strip() | |
| if not pmid: | |
| return ChatToolResponse(error="pmid is required") | |
| if not pmid.isdigit(): | |
| return ChatToolResponse(error="pmid must be a numeric PubMed ID") | |
| summaries = _fetch_summaries([pmid]) |
| link_resp = requests.get( | ||
| f"{EUTILS}/elink.fcgi", | ||
| params={ | ||
| "dbfrom": "pubmed", | ||
| "db": "pubmed", | ||
| "id": pmid, | ||
| "linkname": "pubmed_pubmed", | ||
| "retmode": "json", | ||
| }, | ||
| timeout=TIMEOUT, | ||
| ) | ||
| link_resp.raise_for_status() | ||
| data = link_resp.json() | ||
| linksets = data.get("linksets", []) | ||
| related = [] | ||
| if linksets: | ||
| dbs = linksets[0].get("linksetdbs", []) | ||
| if dbs: | ||
| related = [str(x) for x in dbs[0].get("links", [])[: max(1, min(max_results, 10))]] |
There was a problem hiding this comment.
NCBI E-utilities limits unauthenticated callers to 3 requests per second. Each call to get_related_pubmed makes two sequential NCBI requests (elink + esummary), and search_pubmed makes two as well. When the limit is exceeded, NCBI returns HTTP 429, raise_for_status() raises an exception, and the broad except clause returns a generic "Failed to fetch related PubMed articles: …" with no retry guidance. Adding a specific check for requests.exceptions.HTTPError with status 429 and returning a user-friendly "rate limited, try again" message would give callers actionable feedback.
|
Addressed the review points in follow-up commits:
Commits pushed to this PR branch: Validation rerun: |
1 participant
Summary
Adds a new no-auth Omi integration app:
plugins/omi-pubmed-app.This app provides practical PubMed chat tools using NCBI E-utilities:
search_pubmed(query -> top PMIDs with citations)get_pubmed_article(PMID -> citation + abstract)get_related_pubmed(PMID -> related articles)Why this fits #3120
Validation
python3 -m py_compile plugins/omi-pubmed-app/main.py plugins/omi-pubmed-app/models.pyesearchreturns PMIDs formachine learningesummaryreturns metadata for returned PMIDBounty claim
/claim #3120