Add science_fetch: retrieve records and files from the 42 scientific connectors - #237
Merged
Conversation
Ranks all 110 features/fixes into four tiers with a per-item status verdict audited against the tree at 52845c3: 1 done, 29 partial, 33 skill-only, 47 missing. Leads with a 'wire what already exists' section, since the dominant gap is capability that was built and never connected -- connector fetch() (42 implemented, 0 reachable), the science renderers (only kind:"image" is ever emitted), session-review.tsx (0 imports), and the OpenTelemetry flag (no exporter, emits no span). Documents why a SKILL.md is not an implementation: tool/skill.ts parses markdown and returns prose, so the 280 helper scripts across 97 skills are never executed by product code.
All 42 connectors implement fetch(); nothing calls it. science.ts exposes only search(), so the agent can find a record and not retrieve it. Design: split record retrieval from file retrieval. fetch() returns records (inline under 50KB, else spilled to .openscience/fetch/); new optional fetchFile()+formats serves files (always spilled) for the 7 connectors that can produce pdb/cif/fasta/sdf. Includes a throwaway prototype that executed all 42 fetch() impls against live APIs -- their first execution ever. It falsified the original rationale: payload size is NOT predictable per connector (hpa predicted 0.5-5MB, measured 10.6KB; the two largest, mygene 2.15MB and uniprot 1.42MB, were unpredicted), so size must be measured at runtime. 50KB lands in a real gap: nothing measured between 46.5KB and 82.4KB, and the split held across two runs. Run with: cd backend/cli && bun run prototype:fetch
Task-by-task TDD plan for docs/specs/science-fetch-design.md. Corrects three things the spec got wrong, found while reading the code: MAX_BYTES is already exported as Truncate.MAX_BYTES so nothing needs lifting out of read.ts; Tool.define auto-truncates unless metadata.truncated is set, which a tool that spills its own output must opt out of; and the eutils host has three consumers (eutils.ts, pubmed.ts, geo.ts) rather than the four connectors the spec named, so pacing only some leaves the host unpaced.
Pre-flight scan caught three spots mandating patterns the review rubric treats as defects. Task 8 asserted rateLimit by grepping source text, which passes if the string appears in a comment. The option is consumed inside http.ts and never reaches globalThis.fetch, so the honest test measures the delay it imposes -- and the eutils case now exercises all three consumers of the shared host, which is the reason that scope expanded. Task 9 skipped arxiv with a bare return, asserting nothing. Its rejection is absorbed by .catch in the loop, so the skip was unnecessary; its contract is now asserted explicitly.
Classifies what a connector fetch produced: record, file, clean miss, or
error, and whether a record goes inline or spills. Lifted from a prototype
validated against all 42 connectors' live APIs.
Sentinel handling is the load-bearing part: nine connectors signal 'not
found' with null, {}, or found:false rather than throwing, and conflating
those with failures renders an ordinary miss as an outage.
Records and files have different contracts: a record is structured and may render inline, a file is opaque and always spills. Splitting them makes the spill policy structural rather than a threshold applied to opaque data.
All 42 connectors implemented fetch() and nothing called it, so the agent could find a record and not retrieve it. Routes through the registry, so the tool never learns individual database names and the tool count stays flat as connectors grow. Sets metadata.truncated because Tool.define otherwise runs Truncate.output over a result this tool has already spilled itself.
The 429 test had no Retry-After header, so http.ts's exponential backoff (~1s + ~2s + ~4s) made it depend on the repo's --timeout 15000 override and fail under bare `bun test`'s 5000ms default. Stub Retry-After: "0", matching test/science/science-tool.test.ts:90, so the retries resolve immediately. Assertions are unchanged.
ScienceFetchTool's rate-limit classification duplicated the regex already exported as classifyError from fetch-outcome.ts (Task 1), which exists precisely so science_search and science_fetch agree and so a fix to retry detection needs one edit instead of several. Call the helper instead. Behaviour is identical - classifyError computes retryable/message the same way, so the existing 429 test is unchanged.
Without this the model has no way to discover which formats exist and would guess at them.
Replace vacuous negative assertion on '— formats:' (which can never appear in output) with a real check for '· formats:' separator. Verified with deliberate break test: guard now catches actual bugs. Also rename test title to accurately reflect what it checks: catalog projection preserves the formats key.
RCSB coordinates come from files.rcsb.org, a different host from the JSON entry API the connector already used.
The existing test used an already-uppercase ID, so the toUpperCase() transformation was never exercised. Add a test with lowercase input to properly verify the case normalization.
File URLs are only discoverable inside the JSON prediction record, and the API returns an array, so the connector fetches the record first and follows the URL it carries.
…ectors Four different URL shapes: uniprot takes a query param, pubchem an uppercase path segment, ensembl a different path, kegg a path suffix. uniprot is the clearest payoff — its default JSON record measured 1.42 MB against roughly a kilobyte for the same record as FASTA.
A second pass over all 42 connectors returned HTTP 429 from Semantic Scholar; only arxiv declared a rate limit. science_fetch makes back-to-back retrieval ordinary, so this would bite routinely. The eutils host has three separate consumers -- eutils.ts, pubmed.ts and geo.ts -- and pacing only some of them leaves the host unpaced.
Every fetch() implementation shipped unexercised. The recorder runs all 42
against live APIs; the offline suite replays the results, so fixtures are
recorded truth rather than authored guesses -- which is what stops a
connector passing its test while failing for a user.
Known-degraded connectors are asserted as such rather than skipped.
Live recorder run (2026-07-29): 37/42 returned a record.
biogrid error BioGRID access key required (BIOGRID_ACCESS_KEY unset)
depmap miss catalogue API answered {found: false} for id CRISPR
expression-atlas miss reported {found: false} for E-MTAB-5214
myvariant threw HTTP 404 for chr7:g.140453134A>T (sample id no longer resolves)
semantic-scholar threw HTTP 429, keyless tier rate-limited after 3 retries
Reproduced across two independent live runs with identical results.
Also allowlists 30 gitleaks false positives in the recorded crossref.json
fixture: CrossRef's own bibliography reference keys (e.g. "BFnature12373_CR1"),
not credentials.
The prior conformance loop mocked every connector's fetch() with a fixed 200
JSON body and asserted the outcome kind was one of a closed 4-member union --
that union is exhaustive, so the assertion passed unconditionally regardless
of what a connector actually did with real data. Meanwhile the 40 committed
fixtures had no consumer, despite the previous commit message claiming the
offline suite "replays the results."
This wires them together: the loop now reads each connector's recorded
fixture and asserts the classification the live recorder actually observed
(biogrid: error, depmap: miss, expression-atlas: miss, all else: record), plus
the record/spill disposition its size implies. That is what makes the fixture
bytes load-bearing -- mygene.json (2.3MB) and uniprot.json (1.6MB) are the
real data proving the 50KB spill threshold fires, not just stored bytes.
Also asserts, rather than leaves silent, that myvariant and semantic-scholar
produced no fixture (both threw against the live APIs; see the previous
commit's per-connector table).
Verified two mutations fail before reverting:
- editing uniprot's fixture payload to {"found": false} failed its
classification test (expected "record", got "miss")
- loosening the disposition threshold to 50MB failed exactly the 6 real
spill cases (uniprot, pfam, sifts, ensembl, mygene, bindingdb)
The validated classification logic moved to fetch-outcome.ts in the first task of this branch. The TUI shell is preserved on branch prototype/science-fetch-repl as a primary source. It earned its keep: it falsified the original rationale for the size threshold. Payload size is not predictable per connector -- hpa was estimated at 0.5-5MB and measured 10.6KB, while the two largest payloads (mygene 2.15MB, uniprot 1.42MB) were not predicted to be large at all -- so size has to be measured at runtime rather than annotated.
…lose test gaps science_fetch's spill write bypassed the permission system (plan mode could not stop it) and dropped untracked files into the user's repo with no .gitignore, unlike every other per-session artifact directory. Also fixes the mismatched error-metadata shape between the network and sentinel error paths. Closes three test gaps found in whole-branch review: no end-to-end test of the format-supplied spill path, a conformance suite whose fixture-vanishing protection depended on a byte-count check tautologically derived from the same output it verifies, and no guard against the fixture set silently shrinking.
The 40 fixtures under test/science/fixtures/ are live API responses replayed verbatim by the connector conformance suite. Normalising their formatting fights the recorded-truth property the suite depends on, so they belong with the other recorded/generated artifacts already listed here (models-snapshot.ts, the sdk gen output).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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
1 participant
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.
What does this PR do?
Adds
science_fetch, closing a gap where the agent could find a scientific record but not retrieve it.All 42 connectors in
src/science/connectors/already implemented afetch(id, opts)method, butsrc/tool/science.tsonly ever exposedsearch()— a repo-wide grep forconnector.fetchreturned zero hits. Record retrieval was fully written and completely unreachable.The tool.
science_fetch(db, id, format?)routes through the connector registry, so the tool count stays flat as connectors grow and the tool never learns individual database names. Records under 50 KB render inline; larger ones are written to.openscience/fetch/<db>/<id>.<ext>and the tool returns the path plus a key-level summary, so a 2 MB gene record can't flood the context window.File retrieval. Seven connectors now serve real files rather than JSON metadata, across five different URL shapes:
rcsb-pdbfiles.rcsb.org)pdbealphafolduniprotpubchemensemblkegg/aaseq)alphafoldis why format resolution lives inside each connector rather than in a sharedid → URLmap: a pure function of the id cannot express "read the URL out of the record you just fetched".uniprotis the clearest payoff — its default JSON record measures 1.42 MB against roughly a kilobyte for the same record as FASTA.Rate limiting. Only
arxivdeclared arateLimitbefore this.science_fetchmakes back-to-back retrieval an ordinary action, and a second full pass over the connector set returned HTTP 429 from Semantic Scholar. This adds pacing tosemantic-scholarand to all three consumers of the sharedeutils.ncbi.nlm.nih.govhost (eutils.ts,pubmed.ts,geo.ts) — pacing only some of them would leave the host unpaced.Permissions. Spill writes go through
ctx.ask({permission: "edit"}). Default rules are"*": "allow", so no normal user sees a prompt, but plan mode'sedit: {"*": "deny"}now correctly applies. The spill directory is self-ignoring (a.gitignorecontaining*), following the existing.openscience/handoffs/pattern, so fetched data never pollutesgit status.Also included:
docs/specs/science-fetch-design.md(design),docs/plans/12-science-fetch.md(implementation plan), anddocs/ROADMAP.md(all 110 agreed items tiered with audited per-item status).How did you verify your code works?
These 42
fetch()implementations had never executed. A one-time recorder (script/record-fetch-fixtures.ts, run by hand, never in CI) called all 42 against live APIs. Result, reproduced across three independent runs: 37 of 42 returned a real record. The five that did not are now documented rather than silently broken:biogridBIOGRID_ACCESS_KEYunsetexpression-atlasfound: falsedepmapfound: falsemyvariantsemantic-scholarThose 40 recorded responses are committed and replayed offline by
test/science/connector-fetch.test.ts, which asserts each connector classifies its own real API shape correctly, and asserts disposition against the 50 KB threshold. That is what makesmygene.json(2.3 MB) anduniprot.json(1.6 MB) load-bearing rather than dead weight — they are the real data proving the spill path fires. Exactly six fixtures exceed the cap:uniprot, mygene, sifts, bindingdb, pfam, ensembl.132 tests across 11 files in
test/science/, all passing. Six new test files cover the classification logic, the connector contract, the tool end-to-end (including a successfulformatrequest that writes a real.cif), the sevenfetchFileconnectors, and rate-limit pacing. Rate limits are asserted by measuring the delay they impose — the option is consumed insidehttp.tsand never reachesglobalThis.fetch, so timing is the only honest assertion available.Every new assertion was verified capable of failing by deliberately breaking the thing under test, observing the failure, and reverting — including the specific mutations most likely to slip through, such as deleting a case transform rather than inverting it.
Merge check:
origin/mainis 42 commits ahead of the merge-base. The two branches share zero changed files, a trial merge is clean, and the merged result runs 132/132 science tests and 1432 passing overall.Checklist
bun run typecheckpasses — no errors from this branchbun test(inbackend/cli) passes —test/science/132 pass / 0 failbunx prettier --check .is cleanOne note on the suite. The full
backend/clisuite shows a single failure,spa fallback > browser navigation to an unmatched route still gets the SPA index.html. I verified it fails identically on plainorigin/mainwith none of this branch's code present, so it is pre-existing and out of scope here.Follow-ups deliberately not in this PR
FetchedFile.contentTypeand.filenameare computed by all seven connectors and consumed by nothing. Worth resolving (consume or remove) when the renderer work lands.science.ts,fs.mkdirand the.gitignorewrite still run beforectx.ask, so plan mode creates an empty self-ignoring directory before correctly denying the write. No user data is written; a one-line reorder.docs/ROADMAP.mdwas audited against52845c3.mainhas since added scientific file viewers to the frontend, so at least roadmap item 16 is now out of date and deserves a re-audit.