mirror of
https://github.com/mims-harvard/ToolUniverse.git
synced 2026-09-19 07:31:47 +08:00
bb314a29d5
* Fix drug-name lookups answering with a different drug's label
FDA_get_boxed_warning_info_by_drug_name {"drug_name":"Albuminex"} returned
BEIZRAY/docetaxel's boxed warning -- toxic deaths, neutropenia, hypersensitivity
-- presented as the boxed warning of a plasma-derived human albumin product.
All 77 FDA_*_by_drug_name tools map drug_name onto the OR-group
(openfda.brand_name, openfda.generic_name, spl_product_data_elements). The third
is the raw product/ingredient blob, so the query matched any product merely
CONTAINING albumin; AND-ing _exists_:boxed_warning then deleted both genuine
Albuminex labels -- which correctly have no boxed warning -- and left only the
wrong drug's. Measured at api.fda.gov/drug/label.json:
spl_product_data_elements:"ALBUMINEX" 3
AND _exists_:boxed_warning 1 (BEIZRAY, docetaxel)
openfda.brand_name:"ALBUMINEX" 2
AND _exists_:boxed_warning 0 (the true answer)
AND _exists_:warnings_and_cautions 2
Drop a blob-matched row when the record's own resolved openFDA names exist and
none contains the queried name: that is an excipient match, not a product match.
Rows whose openfda block is empty are KEPT -- they have no resolved identity to
contradict, and that is how genuine unresolved products appear, which preserves
the denosumab 19->25 recall fix. Sampled live, the rule drops nothing for
denosumab, aspirin, metformin, warfarin, docetaxel, Prolia, Humira, Eliquis or
Gamunex-C, and drops exactly BEIZRAY for Albuminex.
When the filter empties a payload it is turned into openFDA's own NOT_FOUND
shape, so the existing section_exists_clause re-probe still engages: Albuminex
now answers with its own 2 labels, boxed_warning null, and the
warnings_and_cautions those labels do carry.
The disclosure note states only what was done. It does not claim the answer
describes the queried drug, because rows with an unresolved openfda block cannot
be checked -- a minocycline lookup whose surviving rows are all zinc gluconate
lozenges would make that claim false exactly where it matters. It instead
reports how many returned rows could not be checked. The caveat is gated on the
guard having actually dropped something, since an unresolved block is the normal
shape for 6 of the 25 real denosumab labels and warning on every clean lookup
would train the reader to skip the note.
* Fix four over-claims in the excipient-guard disclosure
An adversarial review of the previous commit found four assertions the response
does not support. The guard's filtering logic is unchanged -- it loses no recall
across 30+ drug names sampled live -- but what it TOLD the caller was wrong.
meta.total was page-local while the note asserted it had been corrected:
'albumin human' at limit=10 reported 37 when the true post-filter total is 16,
a 2.3x overstatement, and the page-local caveat existed only in a source comment.
The note now says "reduced accordingly, to N" only when the whole result set fits
on one page; otherwise it declares the number an UPPER BOUND and quotes the
upstream hit count and the page size so the reader can audit it.
dedup_note claimed meta.total was "openFDA's upstream hit count before local
processing" in the very responses where the guard had changed it -- two
contradictory statements in one payload. It now names the reduction and the raw
upstream count.
The note named spl_product_data_elements as the sole match route and told the
reader to inspect it. On broadened queries that is false: for minocycline the
term appears in no returned row's blob -- those rows matched
indications_and_usage/description. The note now names the fields actually
searched.
The unchecked-row count was taken before the user limit was applied, so
"N of the returned label(s)" could report 12 when 3 were returned. It is now
counted over the rows actually returned.
Seven tests cover the disclosure itself, which had none -- which is why it
regressed once already. A paged fake transport makes the page-local case
observable; the previous fake made the total correction exact by construction,
so no test could have caught it.
* Fix Round 42: FAERS counts that described a page the response no longer showed
FAERS_count_reactions_by_drug_event applies `reactionmeddraverse`
client-side: openFDA ranks every reaction term on the matching reports, then
the tool keeps only the row whose term equals the one asked for.
`stratified_report_count` -- documented as "what the counts in 'results' sum
to" -- was summed from the ranking page BEFORE that narrowing, so it
described neither the row shown nor the whole facet, and it moved with
`limit` while `results` did not.
Measured live on the anonymous tier. Three round-42 personas hit this
independently, on four drug/reaction pairs:
PROMETHAZINE + somnolence: `results` is the single row SOMNOLENCE 1,438 at
every limit, while stratified_report_count read 1,438 / 2,630 / 6,492 at
limit 1 / 5 / 50.
MODAFINIL + tachycardia, limit 20: the single row is 200 against 322
matching reports -- BELOW the total -- yet coverage_note read "946 --
293.8% of it, i.e. MORE than the number of matching reports", telling a
reader to discount a near-1:1 signal as threefold double-counted.
LISDEXAMFETAMINE + Suicidal ideation, patientagegroup Adolescent: the rows
sum to 8 against 8 matching reports, reported as 35 and "437.5% of it" --
a 35 standing next to an adolescent suicidality signal whose count is 8.
Sum `results` instead of the raw page, so the documented meaning holds by
construction, and give the narrowed regime its own prose: one kept term
cannot double-count against rows that are no longer present, so the
percentage is meaningful there -- a close upper bound on the share of
reports affected, since openFDA counts recorded values rather than reports.
When the narrowing keeps nothing but reports still matched, say so and name
both causes: outside the top-`limit` slice, or matched inside longer
preferred terms (measured: MODAFINIL + "arrest", 176 reports, no preferred
term IS "arrest" -- they are CARDIAC ARREST, RESPIRATORY ARREST).
CONTRACT CHANGE, narrowed path only: stratified_report_count's value moves
to what its own schema says it is. The unnarrowed path is untouched and is
pinned by a regression test that fails if the page sum is reinstated.
Also, additive: FAERS_count_drugs_by_drug_event was the only tool of the 18
in its family with no `medicinalproduct` filter, so it could only ever
return the database-wide top-N drug ranking -- 20,692,690 reports and HUMIRA
at the top whatever the caller wanted, and no way to ask the co-reported
medication question its name implies. With the filter, INTRALIPID returns
INTRALIPID 720, INTRALIPID 10% 119, TPN 82, TEDUGLUTIDE 77, HEPARIN 73
against 961 matching reports, matching openFDA queried directly; a drug that
does not exist returns no rows and a zero total instead of a global ranking.
* Fix Round 42: gene-disease concordance counted sources that never answered
gather_gene_disease_associations queries five databases and reports, on every
association row, `concordance` (how many named the entity) beside
`total_sources_queried: 5`. That denominator counts every source attempted,
including ones that failed and ones that structurally cannot contribute, so
agreement reads as disagreement. Measured for {"gene": "CFH"} with no API
keys configured: DisGeNET and OMIM fail on missing keys and ClinVar carries
no disease name in the gene->disease direction, leaving two sources able to
answer at all -- yet the top row reports "concordance 2, total_sources_
queried 5", and BRCA1's "breast cancer" reports 1 of 5.
Two caps were silent on top of that. `per_source_results` was sliced to ten
rows per source with no flag while `num_associations` counted the full table,
so the same CFH response showed "OpenTargets: 10" against "num_associations:
29" with nothing saying OpenTargets had actually returned 25; `associations`
was sliced to fifty the same way.
All additive -- no existing field changes value or meaning:
- each row gains `total_sources_with_data`, the number of sources that
contributed anything, which is the denominator to divide `concordance`
by; `total_sources_queried` keeps its value and its meaning and now says
in the schema that it is not that denominator
- `per_source_result_counts` gives the untruncated count per source
- `truncated` follows the repo-wide truncation convention for the 50-row
association cap
- `notes` gains a line per disclosure, and only when it applies: no note
when nothing was cut, none when every source contributed
One definition of "contributed data" (`_sources_with_data`) now feeds both
the per-row denominator and the note that explains it, so they cannot drift.
Corroborated across four rounds before this one.
* Fix Round 42: pin the gene-disease disclosures, and exercise the new FAERS filter
Validation of the two commits above found the gene-disease change shipped
without tests: nothing under tests/ mentioned total_sources_with_data,
per_source_result_counts or truncated for this tool, and none of its three
test_examples reaches the 50-row association cap (live: 29 / 41 / 30), so the
truncation branch was unreachable from anything shipped. The FAERS commit had
122 lines of regression tests; this one had none.
Eight tests, all pure functions -- no network, no ToolUniverse construction:
- the honest denominator rides on every row (5 queried, 2 able to answer for
the live CFH case) and concordance never exceeds it
- a source that answers nothing is not counted as disagreeing, and when all
five answer the denominator moves to 5
- each cap names what it cut ("10 of 25" for OpenTargets; 63 against the
50-row cap), and a source sitting exactly at the cap is not listed as cut
- the denominator caveat fires only when the denominator misleads, and a
response with nothing cut and nothing skewed carries no notes at all
- the two fields dropped during review (sources_with_data,
associations_returned) stay dropped, so neither creeps back as a second
way to say what the response already carries
Each was checked against a deliberately reverted build: restoring the
all-sources-queried denominator fails the row test, and silencing
_disclosure_notes fails the three note tests.
Also: FAERS_count_drugs_by_drug_event's only test_example omitted
medicinalproduct, so `tu test` exercised just the database-wide ranking the
new description tells callers not to rely on. Added the filtered example that
the description quotes; both examples pass.
* Fix Round 43: waits that stop early and then report the shortfall as a defect
Thread.join(timeout=...) returns the same thing whether the thread finished
or the wait expired. Four test sites joined with a timeout and then asserted
on the results they had collected, so a slow thread under CI load produced
"Expected 3 results, got 2" -- reporting broken concurrency when the truth
was that we stopped waiting. test_tool_finder_edge_cases::test_concurrent_calls
failed CI on #517 this way and passed on re-run with no code change.
Add tests/helpers/threads.py::join_all: one shared deadline across the set,
then an is_alive() sweep, so a timeout is always reported as a timeout and
names the threads still running. Converted all four sites
(test_tool_finder_edge_cases, test_critical_error_handling,
test_mcp_unit_functionality, test_performance_benchmarks). The budget lives
in one place and is chosen to stay under pytest.ini's per-test timeout = 60,
which the old per-thread budgets could exceed -- 5 x 30s let pytest-timeout
kill the test with its own opaque message first. Sweeping the rest of the
tree found no other bounded wait followed by an assertion; the src/ sites
(smolagent_tool, result_cache_manager, vcf_stats_tool) already check
is_alive() or catch TimeoutExpired.
pytest.ini gains `tests` on pythonpath so `helpers` is importable by
declaration rather than as a byproduct of conftest loading (it otherwise
breaks under --import-mode=importlib), and the four files keep an explicit
sys.path bootstrap so their __main__ blocks still run.
The same shape in tool code: ClinGen_search_actionability fans the Adult and
Pediatric contexts across a thread pool and swallowed a failed context with
`pass`, leaving its list empty. The tool then answered status "success" with
pediatric_count 0 -- indistinguishable from "ClinGen has curated no pediatric
actionability for this gene". Confirmed live: BRCA1 has 6 adult and 1
pediatric curation, but with the Pediatric fetch failing the tool still
answered success/0 with nothing marking the figure as unmeasured. Failed
contexts are now recorded in failed_contexts with a note saying the 0 means
"not retrieved"; when every context fails the tool returns an error rather
than two fabricated zeroes, matching proteins_api_tool's existing policy.
The healthy path is unchanged in keys and values.
* Fix Round 44: an indication inflating a comparator's adverse-event count
FAERS_compare_drugs matched reactions against openFDA's ANALYSED
`patient.reaction.reactionmeddrapt`, whose tokenizer splits on whitespace
and hyphens, so a search for one Preferred Term also matched every compound
term containing it. Measured live 2026-08-12 for "Thrombocytopenia":
heparin 6,805 -> 2,778 on `.exact` (2.45x), bivalirudin 111 -> 27 (4.11x).
The inflation is DIFFERENTIAL, which makes it a wrong answer rather than a
conservative one: `_compare_drugs` divides the two arms' RORs, so a factor
that differs per drug moves the verdict. The contaminating term is
HEPARIN-INDUCED THROMBOCYTOPENIA (3,968 reports) -- bivalirudin's
INDICATION, not its adverse effect -- so the comparator was inflated by
reports where the drug was the response to the event. "heparin and
bivalirudin show similar-strength detected signals" becomes "heparin's is
stronger" (ROR 7.393 vs 4.563) once each arm counts only the term it claims.
Every reaction filter in the module now routes through `_faers_search_query`
so no operation can search a different variant than its siblings. `.exact`
is case-insensitive at openFDA (verified across four capitalisations), so
callers keep working; a string that is not a Preferred Term now returns a
named error listing real terms to retry with, instead of silently counting a
union of terms.
Also in FAERS_compare_drugs: a three-drug list returned status "success", a
confident two-drug verdict and comparison_caveat null, with the third drug
absent from the response. It is now refused, since the analysis is pairwise
by construction.
openFDA label tools:
- A NOT_FOUND blamed `requested_return_fields[0]`, so a boxed-warning search
was explained as "This label section ('openfda.brand_name') is absent" --
naming a field never searched -- and recommended falling back to
`spl_product_data_elements`, the ingredient blob that answers "propylene
glycol" with rabeprazole, glipizide and urea PK. Both corrected; the real
cause, contiguous-phrase matching, is now stated.
- The `_exists_` guard defaults to a tool's RETURN fields, silently dropping
labels that match the search but lack a resolved openfda block
(adverse_reactions:"keratoacanthoma" 4 -> 2, the dropped one muromonab-CD3,
a different drug class). Now disclosed, gated so section tools stay quiet.
- Keyword trimming was skipped for three hard-coded keys and applied to
everything else, including identifiers: `id` and `set_id` came back as ""
because a UUID contains no matching sentence. Replaced with a named
NEVER_KEYWORD_TRIMMED set.
Regression introduced by this change, found and closed before shipping:
moving the three sibling operations to `.exact` turned a loose match into a
raw "404 Client Error ... for url: <full openFDA URL>". All four
reaction-filtered operations now distinguish "not a Preferred Term" from
"no reports for this pair".
That 404 exposed a PRE-EXISTING leak this round also closes, rather than
having caused it: all four RequestException handlers returned `str(e)`, and
because `_with_api_key` appends `&api_key=` to the same URL that requests
quotes back, a configured FDA_API_KEY appeared in the returned error string.
Reproduced with a key set, then closed by stripping at " for url:".
* Fix Round 45: searches answering with a different entity, and probes that under-measure
Three defects, each verified against the upstream API before and after.
proteins_api_search returned an unrelated human protein for a non-human gene
symbol. `{"query": "ben-1"}` returned BANP_HUMAN. ben-1 is the C. elegans
beta-tubulin whose loss confers benzimidazole resistance. Upstream:
gene=ben-1&taxid=9606 is empty, gene=ben-1 returns Q18817_CAEEL, and
protein=ben-1&taxid=9606 returns BANP_HUMAN because the free-text search
matches "BEN domain-containing protein 1". The empty human gene search fell
straight to the free-text field while keeping the human bias, so a different
entity's annotations came back as the requested protein, with no note. The
retry now tries the same field with only the tool's own human default dropped,
accepted when a hit's gene symbol is exactly the query -- which is what keeps
a genuine protein-name query ("insulin" -> "Insulin-like Peptide 1") from
being dragged off. blaKPC, blaCTX-M-15, insulin, BRCA1 and accession routing
are unchanged.
CIViC returned pages with no denominator. Every connection asked GraphQL for
nodes only, so a caller could not tell a complete answer from a first page.
civic_search_evidence_items for melanoma returned 100 records; the true count
is 218. totalCount and pageInfo are now selected across all 10 connection
tools. The count reported matches the tool's own status:ACCEPTED filter (218),
not the wider unfiltered 370. civic_get_variants_by_gene paginates in Python
and rebuilt the block without the count; it now carries totalCount through.
Monarch phenotype tools were nondeterministic, incomplete, and reported an
upstream rejection as success. The multi-phenotype intersection sliced an
unordered set, so three identical calls for HP:0002524 gave three different
differentials and Niemann-Pick type C appeared or vanished by hash seed; it
now intersects in upstream rank order. Each leg took one hard-coded page of
500 and intersected it as the phenotype's whole disease set -- HP:0001250
(Seizure) has 5331 associations, so Seizure + Cataplexy returned one disease
and omitted Niemann-Pick C1/C2, which carry both; legs are now paged, and a
leg still short of its total is disclosed instead of silently returned. The
x3 over-fetch in get_HPO_ID_by_phenotype pushed any limit above 166 past
Monarch's ceiling of 500, and the resulting HTTP 422 body was returned as
status:success quoting an "input" of 501 the caller never supplied; the
over-fetch is clamped and an upstream validation body is now an error.
Each test was confirmed to fail with its fix reverted.
* Fix Round 45: stop echoing an internal over-fetch value back to the caller
The Monarch validation-error path put the raw upstream body into the error
details. That body quotes the value actually sent, and this tool inflates the
caller's limit by 3 before sending it -- so a caller who asked for 167 was
handed back "input": "501", a number they never supplied, presented as their
own argument. Return only the upstream messages.
Found by running the adversarial leak check over the new error path rather
than by any query; no credential is exposed, but nothing the caller did not
supply should come back to them as though they had.
* Fix Round 45: close four defects the round's own changes introduced
A validation pass over the round's earlier commits found four regressions I
had shipped. Each is reproduced, fixed, and covered by a test that fails when
the fix is reverted.
The multi-phenotype intersection returned {diseases, total_matched} without a
'warning' whenever 'limit' truncated it, but the object branch of its own
return_schema listed 'warning' as required and never declared 'total_matched'
-- so the tool violated its contract on its own shipped test_example, in both
directions at once. 'total_matched' is now declared, 'warning' is required
only where it is actually emitted, and the live response validates.
'offset' became a silent no-op on that tool. The new per-leg pager assigns its
own offset into the leg query and overwrote the caller's, so offset=0 and
offset=1 returned byte-identical lists for a parameter still advertised as
"Number of initial entries to skip" and used by the shipped test_examples.
The caller's offset now pages the intersection, which is what it documents.
Two of the ten new CIViC totalCounts were denominators that ignore the query.
browseDiseases/browseTherapies report the whole browse table regardless of the
filter -- 653 therapies for "imatinib", for "zzzzznotatherapy", and for no
filter at all -- so a search matching nothing published 653 under a schema
description telling the caller to divide by it. That is worse than no
denominator, and it was newly introduced. Those two connections now select
pageInfo only, which is filter-aware; the eight filter-aware totals stay.
Clamping the over-fetch turned a loud failure into a quiet short answer:
get_HPO_ID_by_phenotype answered limit=600 with 490 items under
status:"success" and no note, while Monarch_search_gene -- same class, no
result_id_prefix, so no clamp -- errored on the same request. A limit this
tool cannot serve is now an error on both, negative and zero limits are
rejected before reaching the wire, and a page left short by namespace
filtering says so.
Also: civic_get_variant emitted molecularProfiles.totalCount without declaring
it; the widening gate's docstring claimed "gene symbol" while the code also
matches synonyms and locus names; the phenotype intersection kept paging legs
after it was already empty, costing up to 20 upstream requests each.
Two tests were true by construction and have been rewritten to assert the
query on the wire rather than a fixture that already contained the answer.
* Fix Round 46: filters ChEMBL never applied, returned as full-table successes
ChEMBLRESTTool sent every leftover argument to ChEMBL as a query parameter.
ChEMBL ignores parameters it does not recognise and answers with the whole
resource, so an unsupported filter came back as status:success over a table
answering a different question. Verified live against /target.json: no filter,
?species=Schistosoma+mansoni and ?bananaparam=x all return the identical
total_count of 18552 with Homo sapiens rows on top, while the real parameter
?organism=Schistosoma+mansoni returns 12. A search for schistosome drug targets
was answered with 18,552 mostly-human ones and told it was a success.
The check now consults ChEMBL's own published filter list per resource
(/<resource>/schema.json, key "filtering") rather than the tool config, because
the two disagree in both directions and each direction was live. assay_organism
is a real /assay.json filter that ChEMBL_search_assays does not declare -- a
config-only guard rejected 591 genuine Leishmania major assays, which a persona
caught mid-round. organism__icontains looks like ChEMBL filter syntax, but
organism is not an /assay.json field, so ChEMBL drops it exactly like
bananaparam and returns all 1,970,438 assays; trusting the __ suffix would have
let the defect back in through the escape hatch the error message itself
recommends.
The built query is checked rather than the caller's arguments, so rewrites are
covered too. That found five more instances of the same defect shipped in the
tools' own configs, each confirmed by comparing filtered and unfiltered
responses upstream:
/binding_site.json?target_chembl_id__exact=CHEMBL2074 same row as no filter
/protein_classification.json?target_chembl_id__exact=. total_count 905 either way
/activity.json?compound_record_id__exact=1 activity_id 31863, as unfiltered
/document.json?document_id=1 byte-identical to no filter
/compound_structural_alert.json?alert_name__contains=. alert_id 1, as unfiltered
compound_record_id is now sent as record_id, the column /activity.json actually
has (?record_id=1 returns activity_id 57025) -- the tool's own *required*
parameter had been returning the whole activity table. document_id becomes
document_chembl_id and alert_name__contains becomes alert__alert_name__icontains
(394,234 rows of 5,020,133, versus no filtering at all). binding_site and
protein_classification cannot be filtered by target at all, so the parameter
promising it is gone and the descriptions name the tool to use instead.
Round 45's fix in this area was incomplete: ChEMBL_get_drug_mechanisms mapped
drug_chembl_id__exact to the correct parent_molecule_chembl_id but left the
alias in the arguments it rebuilt from, so the request still carried a parameter
ChEMBL discards. The alias is now dropped.
CIViC's variant pager had no page budget. Its only exits were "hasNextPage is
false" and "no endCursor", both the server's to give, so a zero-node page with
hasNextPage true never grew the list and a repeated cursor never advanced --
either one spins forever inside a tool the harness itself calls. The budget is
derived from the caller's own limit, so it cannot truncate a request that is
progressing; BRAF still returns all 114 variants and TP53 all 273.
CKD-EPI called an impossible eGFR normal. creatinine 0.01 mg/dL in an
82-year-old returned "eGFR 240.2 -- CKD stage G1 (normal, >=90)"; 240 is above
any human value including pregnancy hyperfiltration, so that label is wrong
rather than unqualified. A negative creatinine answered with the Python
internals of the resulting complex number, naming neither the parameter nor the
constraint. And creatinine 0.3 at age 82 -- the textbook low-muscle-mass case
where creatinine-based eGFR overestimates GFR, the direction that overdoses
renally-cleared drugs -- came back "normal" with nothing said. The score is
unchanged in every case; values above 150 are now reported as not stageable, and
inputs outside the equation's validated range carry a declared caveats list.
Each test was confirmed to fail with its fix reverted, per assertion rather than
per file: 5 of 10 ChEMBL assertions fail with the guard and the record_id
mapping disabled, and the remaining 5 are controls that must pass either way.
Two pre-existing ChEMBL tests began making live requests because the schema
lookup bypasses the request layer they mock; both are now offline, verified with
a socket guard reporting 0 connection attempts.
* Fix Round 46: close what validation found in the round's own changes
A validation pass over the round-46 commit found three defects. One is the same
class the commit set out to close, left half-closed by it.
ChEMBL_get_drug_mechanisms reads two `__exact` aliases as the drug ID and the
commit dropped only one of them after mapping. `molecule_chembl_id__exact` IS a
real /mechanism filter, so the new guard cannot see it, and it survived as a
second AND filter beside the parent lookup -- silently removing the mechanisms
recorded against other salt forms. Same drug, same intent, both status:success:
CHEMBL20 drug_chembl_id -> 8 mechanisms
CHEMBL20 molecule_chembl_id__exact -> 4 mechanisms
Confirmed upstream: parent_molecule_chembl_id=CHEMBL1382 returns 2 rows, and
adding molecule_chembl_id__exact=CHEMBL1382 returns 0. All five accepted
spellings now agree, verified across CHEMBL20 (8/8/8/8/8), CHEMBL1201354 and
CHEMBL1382 (2/2/2/2/2) rather than on the one drug the report named.
CKD-EPI applied its own new principle to creatinine but not to age. An
impossible creatinine became an error and an impossible eGFR stopped being
called normal, yet age=-5 still returned "eGFR 96.6 -- CKD stage G1 (normal)",
qualified by a caveat about paediatric validity that is a category error for a
negative number. A non-positive age is now rejected the same way.
The creatinine caveats were one-sided. Below 0.2 mg/dL the response said the
value is under the assay's reporting limit; 1000 mg/dL returned "CKD stage G5
(kidney failure)" with nothing said at all, so the same silence remained in the
direction a unit mix-up produces. Values above 20 now say so and name the
umol/L conversion.
Also: `ruff format` on the files this round touched, which pre-commit would
otherwise have rewritten.
Both new assertions were confirmed to fail with their fix reverted. Full unit
suite: 5058 passed, 27 skipped, 0 failed; src/tooluniverse/tools/ and the lazy
registry stayed clean.
* Fix Round 47: failed calls reported as empty answers, and filters that matched the wrong disease
Five fixes, all reproduced against the live upstream APIs before being changed.
Fix-47-1 / Fix-47-5 (openfda_tool, 157 tools): search_openfda handled
NOT_FOUND richly and let every OTHER upstream error fall off the end of the
branch as a bare None, which the CLI renders as {"result": null}. Confirmed
live against api.fda.gov/drug/label.json: one undeclared query parameter is
answered by openFDA with HTTP 400 {"code":"BAD_REQUEST","message":"Invalid
parameter: section"}, and FDA_get_boxed_warning_info_by_drug_name
{"drug_name":"tramadol","section":"boxed"} returned {"result": null} --
indistinguishable from "tramadol has no boxed warning", for a drug whose
label carries one. openFDA reports rate limiting (its anonymous tier is
~40 req/min), server errors and invalid keys the same way. The upstream error
is now returned in the same envelope the NOT_FOUND branch already used, so no
consumer sees a new shape. The three requests.get calls also carried no
timeout and decoded with a bare .json(), so openFDA's HTML error pages
surfaced as "Validation error: Expecting value: line 1 column 1" with
retriable=false; they now share one helper that adds a timeout and reports
transport failures as transport failures.
Fix-47-4 (tool_discovery_tools, compose_tool): the same artifact reached
callers through the dispatch wrappers independently of openFDA -- any tool
returning None became {"result": null} with exit status 0. execute_tool
dispatches through ExecuteToolTool.run; ComposeTool._call_tool is the second
wrapper, used by composition scripts. Both now report a failure as a failure,
through one shared invariant in base_tool.
Fix-47-2 (panelapp_tool): _word_matches bounded only how much was trimmed off
the SHORTER word to reach the shared prefix, and never bounded what was left
dangling off the LONGER one, so any short-ish query became a prefix search
with three characters of slack. Measured against the live 433-panel list,
search="hernia" returned 15 panels, every one a "Hereditary ..." panel with no
hernia content, under a count of 15; search="myopia" returned 3 myopathy
panels. Also matched: sarcoma/sarcoidosis, anaemia/anaesthesia,
neuropathy/{neuronal,neuron,neural}, disease/distal. Bounding both ends
rejects all of them while leaving plurals and the medical adjectival and
-osis/-otic forms untouched. Tokenization now strips punctuation, so a term
followed by a comma or inside a hyphenated compound is still matched.
Fix-47-3 (ncbi_datasets_tool): the species parameter is named `taxon`, but the
tool's own description asks for an "organism" and the sibling species-scoped
tool calls it that. A caller following either had the value dropped and the
lookup fell back to human, then echoed query_taxon: "human" as if that filter
had been applied: {"symbol":"NR2F2","organism":"mouse"} returned human NR2F2
(gene_id 7026, tax_id 9606), and organism:"Vulcan" returned the same human
gene just as confidently. Both spellings are now accepted and declared in the
schema, and an applied default is disclosed as query_taxon_defaulted.
* Fix Round 47: close what validation found in the round's own changes
Three defects found by validating round 47's own changes, plus one piece of
dead code the validation exposed.
1. An overstated claim. _word_matches' docstring said the surviving matches
were "unchanged, verified against the live list". They are not. Measured
over 214 queries against the live 433-panel list, the new bound removes
~198 wrong panel-hits but also loses ~51 real ones, concentrated where
PanelApp's own disease_group vocabulary is a longer derived form:
cardiac/cardiology (15 panels, including Brugada, ARVC and the long/short-QT
panels), immune/immunology (11), muscle/muscular (12),
arrhythmia/arrhythmogenic (6), pigmentation/pigmentary (3),
retinopathy/retinal (1). No threshold separates these from the collisions
the round set out to remove -- cardiac/cardiology needs five dangling
characters and sarcoma/sarcoidosis has six -- so the bound stays and the
cost is now stated instead of denied. The zero-result note names the
remedy, so a caller who searches "cardiac" is told to retry "cardiology"
rather than being left with a bare empty result.
2. A test that passed either way. test_short_words_never_fuzzy_match claimed
to pin the _MIN_WORD_LEN floor while both its inputs were above the floor;
_MIN_WORD_LEN could be changed from 6 to 4 with the whole file still green.
Replaced with pairs that are the right SHAPE to match and are rejected
solely for being too short (renal/renin, colon/colitis), which does fail
when the floor is lowered. The upper end of the suffix bound was also
unpinned; a 3 -> 4 mutation now fails two collision cases.
3. A gained false positive, previously undocumented. Splitting hyphens means
"non-syndromic" yields "syndromic", so search="syndrome" now matches three
panels that are explicitly NON-syndromic. Word-level matching has no notion
of negation and the alternative is losing every hyphenated compound, so
this is recorded as a known cost rather than fixed.
4. Dead code. _MAX_STEM_DROP=3 could not affect any outcome given
_MAX_SUFFIX_LEFT=3: longer - prefix <= N and prefix <= shorter <= longer
together give prefix >= shorter - N, so the trim bound can never reject a
pair the suffix bound accepts. That is why validation could not pin its
upper end, and it means the previous commit's comment crediting it with
holding the -osis/-otic class was wrong -- those pairs pass on the suffix
bound alone. Removed, after checking 298,378 pairs over the live panel
vocabulary and 400,000 synthetic pairs for zero behavioural difference.
The unreachable apostrophe branch of the tokenizer went with it.
* Fix Round 48: restrictions accepted, ignored, and answered as though applied
Four tools returned a confident answer to a question they had not been asked.
FAERS analytics accepted any argument it did not read and returned the
unrestricted result as a success. Measured live 2026-08-13, byte-identical with
and without the filters:
FAERS_calculate_disproportionality {"drug_name": "warfarin",
"adverse_event": "Haemorrhage"} a_drug_and_event = 4313
... plus {"patientagegroup": "6", "concomitant_drug": "aspirin"} = 4313
FAERS_filter_serious_events {"drug_name": "warfarin",
"seriousness_type": "death"} total_serious_events = 17327
... plus {"patientsex": "1", "occurcountry": "US"} = 17327
So a whole-database pharmacovigilance signal was handed to a caller who asked
for an age-stratified or interaction-adjusted one. The spellings are not
invented: this family's own sibling configs teach them -- patientagegroup
appears 60 times in fda_drug_adverse_event_tools.json, whose descriptions read
"all other filters (patientsex, patientagegroup, occurcountry, serious,
seriousnessdeath) are optional". The guard refuses before spending a request
and names the tool that does stratify.
search_clinical_trials reported an upstream refusal as an empty answer.
ClinicalTrials.gov answers {"intervention": "[177Lu]Lu-PSMA-617"} with HTTP 400
"Error parsing query in Other terms: extraneous input '['"; the tool discarded
that and replied "No studies found for the given query parameters", going on to
blame its own phase filter. Dropping the brackets returns 46 studies. The
bracketed form is standard EANM/IUPAC radiopharmaceutical nomenclature and
appears in trial titles this tool itself returns. execute_RESTful_query
collapsed every failure into False and printed the reason to stdout where no
caller could reach it; execute_RESTful_query_detailed returns it instead. The
boolean wrapper is unchanged for its other callers, including the Monarch 422
path that needs a 4xx JSON body handed back parsed.
Clinical calculators scored physiologically impossible inputs. ASCVD with
total_cholesterol=0 answered the raw text "math domain error"; Child-Pugh with
albumin=-1 scored the albumin component in its worst band and returned "Class B
(score 7): significant functional compromise"; HAS-BLED and CURB-65 with
age=-70 returned confident low-risk scores. Fix-R46 closed this for CKD-EPI
from inside that one equation -- the constraint now lives on the shared
_req_number helper, and CKD-EPI's hand-written guards were migrated onto it so
the two cannot drift.
LOINC asked its index for a field name the index does not use. df=SHORT_NAME
returns "" for every code; the index spells it SHORTNAME and returns
"Potassium SerPl-sCnc" for 2823-3. Seven further fields (SYSTEM, SCALE_TYP,
CLASS, STATUS, TIME_ASPCT, METHOD_TYP, COMMON_TEST_RANK) were probed one at a
time and are absent from this index entirely, yet were emitted as empty strings
-- SYSTEM being the field that separates serum potassium from urine potassium.
They are now disclosed, at the shared parse point so all four operations get it.
Two regressions were introduced and closed during the round. Policing arguments
against the declared schema alone refused a valid stratify_by="country" behind
a partial test config, so acceptance is the union of the schema and the module
vocabulary. Moving the CTG search onto the new transport without moving its
own _fetch_total_count probe left that probe reaching the live network under a
test that mocked the search; both now use one seam.
tests/unit: 0 failures. Each new test was confirmed to fail with its fix
reverted -- 7 of 11 FAERS assertions bite, the other 4 are controls that must
pass either way.
* Fix Round 48: close what validation found in the round's own changes
Four defects in the Round 48 changes, found by validating them.
BLOCKER -- execute_RESTful_query stopped handing back a 4xx JSON body. The
round split the transport into a detail-returning function and a boolean
wrapper whose docstring claimed it was "kept exactly as it was". It was not:
the new code consulted status_code after parsing, so a 422 carrying a valid
FastAPI body returned False where it had returned the parsed dict. That killed
the Fix-R16A path in MonarchTool, whose _validation_error_detail turns that
body into "lower your limit, or page with 'offset'" -- instead run() reached
`if "facet_fields" in response` with False and raised
TypeError: argument of type 'bool' is not iterable
across all six shipped Monarch tools. A decodable body is now always returned
whatever the status, and the status code is consulted only when there is no
body to read -- which is the ClinicalTrials.gov case the round was fixing,
whose 400 carries plain text rather than JSON. Both properties were then
confirmed to hold at once. Network-level exceptions propagate again, as they
always had; the round had been swallowing them into a (False, reason) pair.
The two tests that gave false assurance are fixed too. The round's own
test_boolean_wrapper_contract_is_unchanged exercised only 400-with-non-JSON
and 200-dict -- precisely the two cases that had NOT changed -- so it passed
while the contract was broken. It now covers 4xx-with-JSON-body at the real
seam.
FAERS -- the refusal message made a false referral. It told callers "the
FAERS_count_* tools DO filter on concomitant_drug"; concomitant_drug and
reactionmeddraversepa appear in no config under data/ at all. Both are dropped
from the record-filter list and now get the generic refusal. The remaining
eleven names were each checked to appear in a shipped config, since that is
what the sentence claims.
FAERS -- the message also advertised the wrong accepted set. It listed the
union across all six operations, so a disproportionality rejection named
drug1, drugs, stratify_by and seriousness_type as accepted -- arguments that
operation accepts and ignores. The error was advertising the very failure mode
it exists to close. It now names the calling operation's own declared
parameters.
LOINC -- the SHORT_NAME to SHORTNAME rename had been applied to the code but
not to the return_schema, which still declared the field that is never
populated and did not declare fields_unavailable or fields_unavailable_note.
ruff format had drifted on five files; all nine changed files are clean again.
tests/unit: 0 failures.
* Fix Round 49: two LOINC operations that could not succeed but reported success
LOINC_search_forms sent sf=CLASS and then kept only rows whose CLASS contained
"survey"/"panel"/"form". Neither half could ever hold. sf selects which fields
are searched, and the API documents the choices as "text, COMPONENT,
CONSUMER_NAME, RELATEDNAMES2, METHOD_TYP, SHORTNAME, LONG_COMMON_NAME,
SURVEY_QUEST_TEXT, LOINC_NUM" -- CLASS is not among them, so the search matched
nothing:
search?terms=PHQ-9&sf=CLASS -> [0,[],null,[]]
search?terms=PHQ-9 -> 58 hits
and CLASS is separately empty for every code in this index, so the post-filter
would have discarded every row even had rows arrived. All three shipped
test_examples returned count 0 presented as success: the tool's own documented
invocations demonstrated only its failure mode. Restricting with the documented
type parameter instead returns the real instruments -- terms=PHQ-9&type=form
gives the two PHQ-9 panels. A search that now legitimately matches no
instrument says so and points at LOINC_search_tests, so "no instrument has this
name" is distinguishable from a failed query.
LOINC_get_answer_list had the same shape one step further on. It sent
type=answer, which is not an implemented value; unrecognised values are dropped
rather than rejected, so the call degraded into an ordinary keyword search whose
rows were returned as "answer-type codes". Totals were identical for
type=answer and for an invented type=zzzgarbage on every term tried (PHQ-9
58/58, potassium 145/145, hemoglobin 526/526), and the index holds no answer
codes at all (terms=LA6568-5 -> [0,[],null,[]]). Asked for the ABO group answers
it returned the ABO group question. Answer lists are published through
ef=AnswerLists, which does carry them: 883-9 now returns list LL2419-1 with
Group A / B / O / AB, and each row carries the datatype that says whether an
answer list should exist at all. answer_lists_found is reported separately from
count so that matched codes without a list cannot read as lists.
Both changes keep every key these operations already returned; the added keys
are AnswerLists, datatype, answer_lists_found and note. The schemas gain those,
plus the fields_unavailable pair that search_tests and search_forms already
emitted without declaring. Two test_examples that could only ever show the
failure mode are replaced with ones that exercise the working path and the
honest-empty path.
Tests: nine of the ten new assertions were run against the pre-fix class and
fail on it; the tenth is a labelled control that must pass on both. The mapping
of ef values is covered with two rows so that attributing one code's
permissible values to another cannot pass.
* Fix Round 49: impossible inputs scored confidently at the top of the range
Fix Round 48 closed the bottom of every calculator's input range and left the
top open, so the same class survived at the other end. Reproduced through the
CLI before this change:
ClinicalCalc_MELD_Na {"bilirubin":1.0,"creatinine":1.0,"inr":1.0,
"sodium":134000}
-> status success, "MELD-Na 6: low (~1.9% 90-day mortality)"
ClinicalCalc_Child_Pugh {"bilirubin":1.0,"albumin":1000,"inr":1.0}
-> status success, "Class A (score 5): well-compensated disease"
and sodium 5 mmol/L, which is not compatible with life, was clamped into the
MELD-Na formula and reported as a risk band rather than refused -- zero is not
a floor for a concentration, so Fix-R48's must_exceed=0 could not catch it.
The bound belongs to the quantity, not the call site: bilirubin means the same
thing in Child-Pugh and MELD-Na, four of the eight bounded quantities are read
by more than one calculator, and looking the range up by parameter name means a
calculator added later inherits it by naming its input. Each maximum sits an
order of magnitude beyond the most extreme value reported in a living patient
-- 1000 mg/dL for bilirubin against a highest-recorded 80, 200 years for age
against a verified 122 -- and a control case of a genuinely crashing patient
(creatinine 8.0, bilirubin 45, INR 6.5, sodium 118) is asserted to still score,
so the bounds are strict without refusing a real crisis.
Cost: a value beyond the bound now hard-errors instead of being scored. That is
the intent for a unit mix-up, but it is a refusal rather than a wrong answer,
and a future quantity whose real range exceeds these numbers would be refused
too.
creatinine is deliberately excluded. Refusal is the right protection only where
the calculator cannot disclose, and both calculators taking a creatinine
already do: Fix-R46 gave CKD-EPI a caveat naming units, and MELD-Na bounds
creatinine per UNOS and reports the bounded value. Adding a maximum silently
reversed that shipped decision -- round 46's own test is what caught it -- so
the entry was withdrawn and a test now pins the disclosure behaviour of both.
Separately, MELD-Na's components reported a sodium the formula had not used.
The sodium term applies only above a MELD of 11 and applies a value bounded to
[125, 137], but sodium_used named the raw argument, so a patient with sodium
118 was told the score used 118 when it used 125, and a patient below the
threshold was told a sodium was used when none was. Its three siblings already
reported the bounded values they used, and the breakdown exists to make the
score auditable. sodium_used now holds what entered the formula (null when the
term did not apply), sodium_reported holds the argument, and a note explains
any difference.
* Fix Round 49: a disclosure that contradicted the rows it was attached to
Three personas independently hit the same thing: LOINC_search_tests returns
rows reading METHOD_TYP "PhenX", "ISE", "LC/MS/MS" or "Confirm", directly above
a note asserting that METHOD_TYP is "returned empty for every code and their
emptiness carries no information about this code".
Fix-R48 built that list by probing field-by-field against a single code, 2823-3
(serum potassium), and generalised its empty answer to the whole index.
Re-measured over 570 rows drawn from 19 unrelated search terms:
METHOD_TYP 260/570 populated (45.6%)
SYSTEM 0/570
SCALE_TYP 0/570
CLASS 0/570
STATUS 0/570
TIME_ASPCT 0/570
COMMON_TEST_RANK 0/570
The other six survive the wider sample; METHOD_TYP was never absent. It is a
costly field to disclaim -- it is what separates a presumptive immunoassay
screen from a confirmatory LC/MS/MS quantitation -- and the note was telling
callers to disregard it. The disclosure is now also checked against the rows it
is attached to, so no field can be named unavailable while any returned row
carries a value for it. That is the cheap invariant that would have caught this
at runtime rather than three rounds later. And it is no longer emitted at all
on a zero-result response, where it described the fields of no codes.
Also, from review of this round's own two commits:
MELD-Na now discloses every bounded input, not just sodium. Reporting only the
post-clamp value is what let creatinine 900 mg/dL come back as creatinine_used
4.0, status success, with 900 appearing nowhere -- the same defect this round
had just called unacceptable for sodium, left in place for its three siblings.
A single bounded_inputs_note names every input whose supplied value differs
from the value used, so one breakdown no longer carries two disclosure
conventions. The comment excluding creatinine from the bounds table asserted
that MELD-Na "reports the bounded value"; that is now true of what the caller
is actually told, not just of a number in the payload.
The Clinical Tables response parser reads its ef fields off slot 2 rather than
being handed the field list a second time, since naming that list twice at one
call site is a way for ef values to vanish silently, and it adopts the null
hardening its sibling client clinical_tables_tool.py already had in place of a
length guard that could not fire. The sodium parameter description no longer
restates the numeric bound that the table holds. A comment claiming the API
recognises only question/form/panel is corrected: form_and_section is also
accepted, and panel is the inert one.
One assertion added earlier this round could never have failed -- the error
path returns no data key, so "score" not in result["data"] was true by
construction. It now asserts the absence of data itself. Of the five new
assertions here, all five fail against the pre-change class; two of those fail
on the changed signature rather than on behaviour, which is weaker evidence,
and the three that matter fail behaviourally.
* Fix Round 49: close what validation found in this round's own changes
Validation of the three commits above found two Medium defects, both in the
guard those commits introduced, and both invisible to the tests shipped with
them.
The runtime cross-check that refuses to name a field unavailable while a
returned row carries a value for it was enforced in one of two call sites.
`_get_code_details` returns the row rather than the envelope, so it rebuilds
the disclosure itself and passed only the field list. With `results` defaulting
to empty, `not any(...)` is vacuously true and the check never ran: a row
carrying SYSTEM "Ser/Plas", SCALE_TYP "Qn", CLASS "CHEM" and STATUS "ACTIVE"
came back in the same dict as a note asserting all four are empty for every
code. Latent live, since those four really are empty in this index -- but it is
the exact mechanism that produced the METHOD_TYP defect this round set out to
fix, and the comment above the shared parser claimed every operation was
covered.
Worse, the test whose docstring read "The bite: the claim is checked against
the rows, not just asserted" probed with METHOD_TYP -- which the same commit
had just removed from the static list, so the static list excluded it unaided
and the row argument was never consulted. Deleting the cross-check entirely
left all 50 tests green. It is the pattern this harness keeps rediscovering: a
test that covers the case that did not change. It now probes with SYSTEM, which
the static list does name, so only the row check can withhold it; a second test
proves one populated row among several is enough to retract an index-wide
claim; a third covers the details path directly with a row that must have one
field withheld and another still named. Deleting the cross-check, or reverting
the details call site, or making the datatype hint unconditional now each kill
tests -- verified by mutating all three.
Also from that review:
The refusal message rounded the offending value through %g, so sodium
49.999999999 was refused with "must be at least 50, got 50" -- a contradiction,
in the one message these bounds exist to make actionable. The bound is still
formatted that way, being a round number written by hand; the value is not.
The sodium floor was the one bound not an order of magnitude clear of reality.
Survivable hyponatremia is reported into the high 70s mmol/L, so 80 sat on top
of patients who exist. It is now 50, still far above the sodium of 5 that
motivated it.
The 45.6% populated rate quoted for METHOD_TYP did not reproduce; an
independent sample of the same size put it at 26.7%, because which codes a term
matches varies. Both are recorded rather than averaged. Only the zeroes are
load-bearing: these entries claim a field is never populated, and the evidence
for that is that no sample has ever found a value. The test now demands a
measurement exist for any field added to the list.
Finally, the no-answer-list note referred callers to `datatype` for the
explanation. When upstream sends no ef block at all, no row has a datatype, so
the note pointed at something absent. It is appended only when the rows carry
one, and two tests cover the ef shapes that were correct but unexercised: a
slot-2 list shorter than the code list, and a null slot 2.
* Fix Round 50: counts and filters that described a page instead of the query
ClinGen_get_variant_classifications reported `total: 25` for PAH, a gene with
817 expert-curated classifications -- a 32x understatement presented as a
complete success. Re-probing the Evidence Repository API showed two causes,
neither visible from the code alone:
* `classifications` pages at 25 when `matchLimit` is unset, and returns no
total, so `len(items)` counted one page and called it the answer
* `variant` is not an endpoint parameter at all. It is accepted and ignored
-- a deliberately bogus `variant=ZZZNOTAVARIANT` returns the same rows as
sending no filter -- so the request degraded to an unfiltered listing and
the tool narrowed it client-side over whatever page came back
The second cause was the worse one. CAID CA16020993 (PAH c.1315+1G>T) is row
401 of PAH's 817 and is classified Likely Pathogenic by the Phenylketonuria
VCEP, but sat outside the 25-row page, so the tool answered "No variant
classifications found ... Not all genes have active VCEPs" -- a false negative
delivered with a confident and incorrect explanation.
The endpoint does implement `caid`, `variationId` and `hgvs`, each an exact
server-side filter (verified live: all three resolve CA16020993 to one row,
and `hgvs` matches protein-change strings such as p.Arg408Trp). Variant
lookups now name whichever one fits the identifier supplied, so no client-side
narrowing remains. `total` is now the query's count, `returned` the published
count, and the gap between them is disclosed rather than left to be inferred.
PANTHER_ortholog had the same shape in miniature: matchortho returns a list
when several orthologs match, and `mapped[0]` discarded the rest even under
`ortholog_type='O'`, whose own description promises "all orthologs". Human
ABCB1 -> mouse returns Abcb1b and Abcb1a; only Abcb1b was published, losing
half of the P-glycoprotein pair that makes the mouse the standard model for
human ABCB1. `mappings` and `total_mappings` are additive; `mapping` still
holds the first match so existing callers are unaffected.
Verified live before and after against both upstream APIs. Each new assertion
was confirmed to fail with the fix reverted, except the two that deliberately
pin unchanged behaviour.
* Fix Round 50: state the row cap beside the total on every ClinGen listing
Sweeping the rest of the ClinGen family for the shape fixed in the previous
commit found it three more times in the same file. `_get_gene_validity`,
`_get_dosage_sensitivity` and the actionability listing each published
`data: curations[:100]` next to `total: len(curations)` with no flag, no
`returned` count and no way to page.
Confirmed live: `ClinGen_get_gene_validity {}` reported `total: 3659` directly
above exactly 100 rows, on a tool whose description promises a "comprehensive
list". The cap is deliberate -- the gene-validity CSV is thousands of rows --
but saying `3659` beside 100 rows and nothing else leaves the caller to detect
the gap by counting.
All four listings now go through one `_published` helper, so the classification
handler and its three siblings disclose truncation identically instead of three
copies of a slice drifting apart. Each carries a hint naming the parameter that
actually narrows that listing, since a truncation flag with no remedy only
relocates the dead end.
Verified live before and after. Every new assertion was confirmed to fail with
the fix reverted except the one pinning that a narrowed query stays unflagged.
* Fix Round 50: MeSH_get_descriptor answered for records that do not exist
NLM's linked-data server returns HTTP 200 with a bare `{ }` for any well-formed
but unassigned MeSH identifier; only malformed ones 404, and that path was
already handled. The tool mapped the empty body onto its full field layout and
published the result as a success, so `D999999` came back as a complete-looking
record with `label: ""`, `tree_numbers: []` and `active: null`. A descriptor
that does not exist was indistinguishable from one whose label is blank.
Separately, descriptor records carry the name in `label` while Term and Concept
records carry it in `prefLabel`. Reading only `label` discarded a name the API
plainly supplies: `T010724` returns `prefLabel: "Dysphagia"`, and the tool
published `label: ""` while at the same moment reporting `type: "Term"` -- it
had already determined the record was not a descriptor and still reported
success with the name thrown away.
Between them these made an empty `label` ambiguous across three unrelated
situations. Now an empty body is an error naming the identifier and pointing at
MeSH_search_descriptors, `prefLabel` is read as the fallback name, and a record
that resolves to something other than a topical descriptor says so and names
the tool that resolves the descriptor owning it.
Both bodies confirmed live at id.nlm.nih.gov before and after. Topical
descriptors are unchanged, with a test pinning that they gain no note.
* Fix Round 50: say that serious-event reaction rows are not a breakdown of the total
FAERS_filter_serious_events publishes total_serious_events beside
top_serious_reactions, and the two count different things: the total counts
reports, while each reaction row counts reports mentioning one term. A single
report lists several terms -- confirmed live that warfarin death report
10037626 carries four -- so the rows overlap and routinely sum past the total.
The numbers were right; nothing said so. PRUSSIAN BLUE + death publishes
total: 1 above four rows of count 1 with truncated: false, which reads as four
fatal cases rather than one report carrying four reactions. The stratification
operation in this same module already spells the hazard out at length, so the
family disclosed it in one place and not the other, and the arithmetic has been
filed as a defect in the count more than once -- including in the brief for
this round, which is what prompted re-deriving it at the API rather than
changing anything.
Additive: one coverage_note field, no number changes.
* Fix Round 50: close what the quality review found in this round's own changes
Four review passes over the round's diff. Three findings were substantive:
Truncation disclosure used a vocabulary unique to this file. The repo already
publishes `truncated` in 26 files and `truncation_note` in 15, with two shared
implementations (cbioportal_tool._truncation_fields, clinvar_tool.
_truncation_disclosure). The new keys `data_truncated` / `data_truncation_note`
/ `total_is_a_lower_bound` / `total_note` each greped to exactly one file, so a
caller reading truncation generically would have needed a spelling that existed
only here, and one response could carry six keys for one idea. Renamed to the
convention and folded the request-cap case into the same two keys, so
`_published` now reports `truncated` explicitly either way.
PANTHER_gene_info had the same collapse `_ortholog` was fixed for, 180 lines
above it in the same file: `gene_data[0]` under a comment that says "multiple
genes". Confirmed live that gene_id "TP53,BRCA1" echoed both names back while
publishing only BRCA1's PTHR13763 -- one gene's family and GO annotations
served as the answer for another. Publishing one instance of a two-instance
class makes the pattern read as fixed when it is half fixed. `genes` and
`total_genes` are additive; the three top-level fields still describe the first
match.
The empty-result notes pointed at `clinvar_search_variants`; the registered
name is `ClinVar_search_variants`, so the remedy offered to a caller who had
just hit a dead end was itself a dead end.
Also corrected a docstring claiming ~1s response times: measured live, asking
for every row costs 1.5 MB / ~5s for PAH and 8.1 MB / ~15s for RUNX1, the
repository's largest gene, against this tool's 120s timeout. That is the price
of `total` being a count, and the docstring now states it rather than quoting
the old page-sized figure.
Two findings were declined and are recorded in the round report: lifting
`_published` into shared infrastructure (the review measured 42 undisclosed
sites across 32 files -- real, but far past this round's scope), and declaring
`coverage_note` in FAERS_filter_serious_events' return_schema, which is
`{"type": "object"}` with no properties at all, where naming one field and
omitting eight would read as a contract it does not have.
* Fix Round 50: close what validation found in this round's own changes
Three substantive defects, all introduced by this round.
MeSH_get_descriptor's new note was factually false for three record classes.
It fired for anything that was not a TopicalDescriptor and asserted the record
"has no tree numbers or annotation of its own" -- true only of Term and
Concept. PublicationType, GeographicalDescriptor and Qualifier are indexed in
their own right: D016428 returns tree V02.600 and a full annotation, so the
note printed a denial directly above the populated fields contradicting it,
and offered an entry-term remedy to a record no descriptor owns. Worse, the
test file covered only Term -- the single class for which the claim happened
to hold -- so the falsehood was invisible to a green suite. The note is now
written per class, and PublicationType has a fixture.
A whitespace-only `variant` defeated the guard that exists to prevent an
unbounded request. `" "` is truthy, so it satisfied "at least one filter",
then stripped to an empty `hgvs=` and sent exactly the unfiltered listing the
docstring says must never be sent -- measured at a full 120s timeout, with the
cost paid upstream too. Arguments are now normalised before the guard rather
than after it.
PANTHER_gene_info published `gene_symbol`, a key `geneinfo` entries do not
have, so it was null on every row while being newly declared in the schema.
The identifier the caller typed lives in `mapped_id_list`, and PANTHER does
not return rows in input order -- "TP53,BRCA1" comes back BRCA1 first -- so
the previous commit's fix was only half done: it stopped dropping genes but
left rows the caller could not attribute without decoding an accession string.
Rows now carry `input_id`, plus `family_name`/`subfamily_name`, which the
original inline code had also discarded.
Also corrected: the no-result note called the lookup "exact" when `hgvs`
matches on substring; the `variant` parameter description never mentioned that
CAIDs and ClinVar VariationIDs are accepted, which is the headline fix and was
undiscoverable from the schema, and its test_examples exercised only `gene`,
so no example ever ran the identifier routing; PANTHER_ortholog's description
still promised a single ortholog; and MeSH's `note` was undeclared in its
return_schema while this same round declared PANTHER's new keys.
Each of the seven new assertions was confirmed to fail with its fix reverted.
* Fix Round 51: FAERS answering about a different product than the one asked for
Three fixes, all in the wrong-entity seam: a query that matches something other
than the drug named, and reports its data as that drug's.
1. FAERS drug-name counts now disclose how much of the answer is a different
product (additive; 18 count tools).
Every FAERS drug-name lookup searches the union of FAERS_DRUG_NAME_FIELDS,
and two of those three fields are openFDA's SPL annotation rather than the
text the reporter wrote. openFDA tags a drug entry with EVERY brand and
generic name registered for its active ingredient, so a brand-name query
returns every product sharing that ingredient. The union itself is correct
and stays (dropping a field undercounts SEROQUEL and XELJANZ); what was
missing was any statement of what it swept in.
Measured live 2026-08-12 against meta.results.total, "named" =
patient.drug.medicinalproduct alone, "matched" = the union:
IOSAT 6 / 1,395 99.6% named a different product
CYANOKIT 238 / 4,119 94.2%
SUBLOCADE 5,134 / 28,481 82.0%
ADVIL 67,159 / 264,100 74.6%
XGEVA 36,039 / 51,441 29.9%
OZEMPIC 66,161 / 66,161 0.0%
MAKENA 22,729 / 22,729 0.0%
CYANOKIT is the cyanide-poisoning antidote; hydroxocobalamin is also a
routine vitamin B12 supplement, and 3,881 of its 4,119 matched reports are
B12 reports (sampled: safetyreportid 10016092, medicinalproduct
HYDROXOCOBALAMIN, drugindication MICROCYTIC ANAEMIA). Before this,
FAERS_count_reactions_by_drug_event{"medicinalproduct": "CYANOKIT"} answered
ACUTE KIDNEY INJURY 211 against total_reports_matching_query 4,119 with
nothing saying the denominator was mostly vitamin recipients.
The OZEMPIC and MAKENA rows are why the share is measured per call rather
than warned about statically. Responses gain reports_naming_queried_drug,
reports_matched_by_name_resolution_only and, only when the query really did
widen, drug_name_scope_note. All three are declared in the return_schema of
the 18 tools that emit them, since agents plan off the schema.
Cost: one extra openFDA request per drug-name count call (three, not two),
skipped when no drug name was passed, when the name searches a single field,
when the facet is empty, or when the denominator is unavailable. Worst-case
latency rises from 55s to 80s; both probes fail soft.
2. OpenFDA_search_drug_events no longer discards the drug it was given
(contract change; 1 tool).
drug_name and reaction were honoured only when a raw `search` was absent, so
supplying both counted the whole of FAERS. Measured live, {"drug_name": D,
"search": "serious:1", "count": <reaction facet>} returned a byte-identical
payload topped by DEATH 846,360 for "levetiracetam", for "lacosamide" and
for the nonexistent "ZZZ_NOT_A_DRUG_XYZ" -- each "status": "success". They
are now AND-ed, which can only narrow: levetiracetam gives SEIZURE 14,934,
lacosamide 5,969, the nonexistent drug 0. The raw-search-only and
drug-name-only paths are byte-for-byte unchanged.
3. FAERS_search_serious_reports_by_drug can now actually ask for serious
reports (additive; 1 tool).
Despite its name the tool had no `serious` parameter and no seriousness
field in its search map, and returned byte-identical output to its
unfiltered sibling. Measured over the first 100 reports, non-serious was 12%
for acamprosate, 24% levetiracetam, 39% ibuprofen and 79% gadobutrol. The
parameter its five sibling tools already had is added, via the existing
HUMAN_TO_FDA_MAP["serious"] mapping; the default still returns both classes,
so the description, return_schema and skill reference now say so instead of
implying otherwise. The generated SDK wrapper is updated to match, so the
parameter is reachable from the Python API.
* Fix Round 51: close what validation found in this round's own changes
Validation of the round-51 commit found two defects in it and three inaccurate
claims. All five are closed here; the two defects now have regression tests that
fail when the fix alone is reverted.
1. The negative-difference guard published the figure it said it suppressed.
`reports_naming_queried_drug` was written immediately after the scope probe
and was never removed on the bail-out, so a probe disagreement shipped a
subset count LARGER than the superset it is drawn from -- simulated at
named=5,000 against matched=4,119, the envelope carried
reports_naming_queried_drug 5,000 beside total_reports_matching_query 4,119,
contradicting both the comment ("Publish nothing rather than a figure that
cannot be true") and the return_schema. Both keys are now written after the
check; a failed probe still publishes an explicit null, which is a different
statement from "not applicable".
2. `_drug_name_fields`' documented fallback was not real on the parent path.
The accessor promises that a config which forgets its name map still asks
the same question as every other FAERS tool, but only the multi-drug
subclass went through it: the parent read `self.search_fields` directly and
dropped the drug name entirely, while the scope probe -- which keys off
DRUG_PARAMETER rather than off the map -- still applied it. With the map
removed the facet searched `search=` (nothing at all) and the note then
blamed SPL annotation for 899,900 reports. The parent now uses the accessor,
so the promise holds where it is made.
3. `OpenFDADrugEventsTool`'s class docstring and the `OpenFDA_search_drug_events`
tool description both still said the tool accepts a raw `search` string OR
the convenience parameters, thirty lines above the code that AND-s them --
so nothing an agent reads said the combination now works, which was the
point of the fix.
4. Two wording corrections: the disclosure reuses the denominator as its
denominator, not "as its own numerator"; and the return_schema for
`reports_naming_queried_drug` listed two of the four conditions under which
the key is absent, where the runtime docstring lists all four.
tests/unit + tests/integration: 5,641 passed, 36 skipped, 0 failed.
* Fix Round 52: a safety inference computed over a cohort that is 94% a different drug
FAERS_calculate_disproportionality built its 2x2 contingency table from a
drug-name union that also searches openFDA's SPL annotation, which tags a
report with every brand and generic name registered for the active ingredient
of a product the report named. A brand query therefore collects every sibling
product sharing that ingredient, and the tool published an ROR labelled with
the queried brand.
Measured live 2026-08-12/13. CYANOKIT, a cyanide antidote, has 4,119 reports in
that union and 238 under the reported product name; the top names in the union
are HYDROXOCOBALAMIN 3,882, ATORVASTATIN 775, BISOPROLOL 538 -- vitamin B12
supplementation and its patients' polypharmacy. Against DEATH:
union cohort a=50 b=4,069 ROR 0.287 [0.217, 0.379]
reported-name cohort a=12 b=226 ROR 1.24 [0.694, 2.216]
The union interval lies entirely below 1, so the tool told a poison control
physician that death is reported disproportionately LESS often for a cyanide
antidote. Restricted to reports that named CYANOKIT the answer is inconclusive.
Not a shifted number: the opposite verdict, with a confidence interval on it.
The union stays the analysed cohort. The size of the gap cannot distinguish
brand contamination from a generic correctly collecting its own brands --
TOFACITINIB is 93.0% non-naming and CYANOKIT is 94.2%, and the extra reports
are genuine exposure in the first case and a different drug in the second -- so
no rule can pick a cohort automatically. What changes is that the split is now
measured per call and published as `cohort_scope`, with the ROR restricted to
reports that named the drug alongside it when the two differ. Additive: no
existing figure moves; one clause is appended to `note`. FAERS_compare_drugs
carries it on each arm, where it matters more, because the verdict there is a
ratio of two cohorts.
Also in this change:
- The probes are paid for. `_get_faers_count` now asks with `limit=0` instead
of `limit=1`, which returns the same `meta.results.total` without a report
body: one analysis moved 212,426 bytes before and 3,157 after, while going
from 4 requests to 6. It also shares the FAERS count tools' report-total
memo, so a second analysis of the same drug costs 3 requests, not 6.
- That memo is new, and closes a cost the count tools' own disclosure
introduced: a four-tool CYANOKIT workup sent 12 requests for 6 distinct
questions, each probe fetched four times. It now sends 6 and repeats nothing.
Built on the package's own LRUCache, keyed on (endpoint, search) rather than
on the built URL so the two modules' spellings of one question collapse to
one entry and no api_key ends up in a process-global dict key. Successes are
cached with a 300s TTL; failures never are, so one rate-limited moment cannot
stick. Not single-flight: concurrent misses still duplicate, and the 12->6
figure is measured for sequential calls.
- ClinicalTrials.gov reports totalCount only on a first page, and the fallback
`data.get("totalCount") or len(studies)` filled the gap with the page size.
query_cond "silicosis" at page_size 3 reported total_count 20 on page 1 and 3
on page 2; mesothelioma reported 64 then 5. Paging a query to the end left
the caller holding a denominator equal to their last page. Now null plus a
note saying where the real total is, matching the sibling tool. The guard is
`is None`, not falsiness, so a genuine totalCount of 0 survives.
27 tests added, 21 of which fail when the fix they cover is reverted; the other
6 assert that existing behaviour did not move.
* Fix Round 52: close what validation found in this round's own changes
The validator ran the full unit suite, which this round had not, and re-derived
every numeric claim in the previous commit against the live API. It found 20
regressions and one false claim. All are closed here.
20 unit-test regressions, all introduced by the previous commit:
- 13 came from the new report-total memo being a process global with no reset
hook. A test that let a probe succeed handed its answer to a later test that
stubbed the same probe to FAIL, and the assertion under test is usually "a
failed measurement reads as null, not as a number" -- so the leak supplied
exactly the number that hides the bug. test_faers_count_facet_coverage
asserted a null denominator and got 4741, cached by an earlier test in its own
file. The new test file had its own fixture; the six pre-existing FAERS files
had no reason to know the memo exists. The reset is now autouse in
tests/conftest.py.
- 7 came from the limit=1 -> limit=0 switch and the two extra probes breaking
mocks that keyed on the literal "&limit=1" or supplied a response sequence
sized for four probes. Updated in place; the behaviour they cover is unchanged.
tests/unit is now 5351 passed, 0 failed, 0 errors, 28 skipped.
tests/integration is unchanged from before this round: the same 3 failures in
test_mcp_protocol.py and 7 errors in test_smcp_http_server.py, both pre-existing
on the base commit.
The false claim: the previous commit said keying the memo on (endpoint, search)
lets faers_analytics_tool reuse entries the count tools put there. It does not,
for any single-word drug name -- which is every drug in the worked examples. The
two modules spell the same question differently: faers_drug_name_clause always
quotes the drug name, while the count tools' _render_clause quotes only when the
value contains a space. Verified: "SODIUM CHLORIDE" collapses to one entry,
CYANOKIT does not. Normalising the quoting in the key would be wrong rather than
cosmetic, because quoted and unquoted are genuinely different openFDA queries
for a hyphenated name -- a bare hyphen is a Lucene operator, which is why
faers_drug_name_clause quotes at all. So the claim is corrected in all four
places it was made, and what the memo actually buys is stated instead: repeat
calls within a module. A second analysis of the same drug still costs 3 requests
rather than 6, and the count tools' own 12-to-6 saving is unaffected. The
limit=0 change, which is the larger part of the payback, was verified exact:
212,426 bytes before, 3,157 after.
The test that claimed to prove cross-module sharing was hollow -- it seeded the
memo through the analytics module's own key builder, so it could only show the
tool reading back what it had written. Replaced with three tests that assert
what is true, including one that pins the non-sharing so the comments cannot
drift back.
Also from the validator:
- faers_analytics_tool._get_faers_count defaulted a missing meta.results.total
to 0. That was already wrong; memoising it would have made a fabricated zero
sticky for the TTL. It now returns None, routing into the same "count query
failed" path as a transport error.
- FAERS_calculate_disproportionality's return_schema was {"type": "object"}, so
the round's new output object was documented only in prose. cohort_scope and
its members are now declared.
Corrections to the previous commit message: the mesothelioma figure quoted there
as "64 then 5" is not reproducible -- that query returns total_count 604 today.
The silicosis case (20 then 3) is confirmed, as is the underlying behaviour that
ClinicalTrials.gov omits totalCount on a paged response. The test count should
have read 28 added, 22 of which fail on a reverted tree, not 27 and 21.
* Fix Round 53: totals invented from the rows, and disclosures that described a different measurement
Four fixes, all of the same shape: a number or a sentence that was not
measured from what it claimed to describe.
1. ClinicalTrials_get_field_values invented its own facet size.
`uniqueValuesCount` fell back to `len(all_values)` when ClinicalTrials.gov
did not send one, and four lines later `upstream_truncated` was derived
from that fallback -- so the test was `len(all_values) < len(all_values)`,
False for every input, and any field without an upstream count certified
itself complete.
Measured live over the whole /api/v2/stats/fieldValues payload
(2026-08-13): 149 of the 418 fields carry neither `topValues` nor
`uniqueValuesCount` -- 92 INTEGER, 29 DATE, 28 BOOLEAN. All 269 STRING and
ENUM fields carry the count, so the fallback fired only where it was wrong.
Those 149 returned `unique_values_count: 0`, `values: []`,
`truncated: false`: a positive claim that the field has no distinct values.
They also each carried a summary that was parsed past and dropped --
trueCount/falseCount for BOOLEAN, min/max/avg for INTEGER, min/max/formats
for DATE. HasResults is the case that matters: a caller asking how many
registered studies have posted results was handed an empty list, while
upstream held 79,677 true and 518,832 false, summing exactly to the
registry total.
BOOLEAN facets are now materialized from the two counts. INTEGER and DATE
report `unique_values_count: null`, `truncated: true` and a new
`value_summary` carrying the range. The coverage note for those fields
previously asserted, in one sentence, both "leaving 598,509 studies
represented" and "The rows sum to 0."
2. The FAERS cohort disclosure covered two operations out of six.
Round 52 added `cohort_scope` to the two inferential operations. Four
siblings build the identical union cohort from the identical `_drug_clause`
and published nothing: `_stratify_by_demographics`, `_filter_serious_events`,
`_analyze_temporal_trends`, `_rollup_meddra_hierarchy`.
`_analyze_temporal_trends` is the sharpest: it emits a verdict. Live,
CYANOKIT (a cyanide antidote, 4,119 reports of which 238 named it) returned
trend "Increasing", percent_change 19900.0 -- the trend in vitamin B12
supplement reporting, published under the antidote's name.
The split moved into a shared `_cohort_split`; the restricted-ROR arm did
not move with it, since there is no restricted form of a stratification.
Each call site supplies prose describing its own output. Cost: one extra
openFDA request per operation for stratify and rollup, two for serious
events and temporal trends, all `limit=0` and memoised per process.
3. PubMed read one of NCBI's two disclosure containers.
esearch reports dropped terms in `warninglist` when the search ends with
zero hits and in `errorlist` when terms were dropped but hits REMAIN. Only
`warninglist` was read, so the disclosure fired exactly when the caller
could not be misled and stayed silent exactly when they could. Measured
live: "benzene hematotoxicity NQO1 GSTT1 rs180056600" returns 4 articles
with errorlist.phrasesnotfound ["rs180056600"] and a null warninglist;
"zzzqqqxyz toluene diisocyanate nonexistentterm12345" returns 1,477 with
two thirds of the query discarded. Both now report `terms_not_found`; an
unrecognised [field] tag reports `search_fields_not_found`.
4. Two ClinicalTrials.gov disclosures described a different measurement.
`relaxed_match_check` counts a probe built from the original wording plus
every other filter, then published only the rewritten parameters as
`relaxed_query`. Live, query_intr="levetiracetam" +
query_term="subcutaneous palliative" published `{"query.intr":
"levetiracetam"}` beside a 0 and the words "a genuine absence of matching
trials" -- while that displayed query matches 253 studies on its own. It
now publishes the query that was counted.
The coverage note's "The rows sum to 481,189" is a statement about the
whole facet; with page_size=2 the reader sees two rows summing to 324,064.
The sentence now says which rows it means.
* Fix Round 53 (validation): correct a false cost claim, remove a redundant request, harden three null paths
Applied from this round's own quality review of its first commit.
A false claim in the round's own code. `_cohort_split`'s docstring said "ONE
extra openFDA request per operation". Only `_stratify_by_demographics` pays
one. `_rollup_meddra_hierarchy` and `_filter_serious_events` pay two, because
a `count=` response carries no meta.results.total and the serious-filtered
total measures a different cohort. The docstring now gives the count per
operation, with the reason each needs what it needs.
A second inverted claim, same commit. The comment at `_filter_serious_events`
said probing the serious subset "would cost a request to answer the same
question". It would cost ONE added request (the numerator; `total_serious` is
already its denominator) against the two spent here. The union pair is bought
for memo reuse across sibling operations, not because it is cheaper, and the
comment now says that.
A redundant request removed. `_analyze_temporal_trends` fetched a union total
it already held: the `count=receivedate` facet is unpaged and complete, so
`sum(yearly_counts.values())` is both free and the more faithful denominator --
it is exactly the reports the trend was computed over. Measured live
2026-08-13 that the two agree, because `receivedate` is populated on every
report: CYANOKIT 4,119/4,119, IVERMECTIN 6,367/6,367, RIFAPENTINE 521/521,
OZEMPIC 66,161/66,161, HYDROMORPHONE 130,429/130,429. Net cost of Fix-R53-2
is now +1 request for stratify and temporal, +2 for serious events and rollup.
A regression this round introduced, found by the full unit run rather than by
the targeted one. `test_faers_rollup_pt_truncation` pins the request budget of
`_filter_serious_events` at 2; the disclosure makes it 4. The assertion is
updated to 4 rather than deleted -- it is the guard that catches a fifth
request appearing by accident -- and now also checks that three of the four
are `limit=0` probes carrying no report bodies.
Three null paths hardened, none of them reachable today, all of them resting
on invariants that spanned two functions:
- the coverage note's no-ranking branch is guarded on `unique_values_count
is None` rather than on `value_summary`, so a BOOLEAN missing its
trueCount/falseCount cannot fall through into the completeness prose that
would tell the caller zero rows "partition the studies that record it";
- sections 3 and 4 of that note state their own `isinstance` precondition
instead of relying on the early return above them to have fired;
- the falsy-denominator guard moved from four call sites into
`_cohort_split`, which also removes a divergence the four had already
drifted into -- three emitted `cohort_scope: null`, one omitted the key.
`cohort_scope` is now declared in `return_schema` for the three operations
that emitted it with no contract entry at all, following the precedent set by
FAERS_calculate_disproportionality of declaring the one key that matters.
Also drops a redundant early return in `_search_warning_metadata` that the
existing empty check below it already subsumes.
* Fix Round 53 (validation 2): close four contradictions validation found, three of them this round's own
Behaviour is unchanged except where noted; these are corrections to claims.
A no-op rewrite, and a comment that was wrong about Python. This round changed
`truncated` from `page_truncated or upstream_truncated` to `True if
page_truncated else upstream_truncated`, justified by "`upstream_truncated` is
tri-state, so `or` would coerce an unknown facet size to False". Python's `or`
returns its second operand when the first is falsy, so `False or None` is
None: the two forms agree on all six inputs, the rewrite was a no-op, and no
test could have distinguished them. Reverted to the original expression, with
a comment stating why it needed no change.
A test whose name asserted something unreachable.
`test_page_size_truncation_still_wins_over_an_unknown_facet_size` used a
fixture carrying `uniqueValuesCount: 3` -- a known size -- and the case its
name described cannot occur at all, since an unknown size implies zero rows
and zero rows cannot be page-truncated. Renamed to what it tests.
The genuinely reachable unknown-size case now has a test, and it earns its
place: a BOOLEAN field arriving without trueCount/falseCount has no rows to
count, no upstream count and no range to fall back on. Reverting the coverage
note's guard to the `value_summary` form this round shipped first sends that
payload into the completeness prose -- "they partition the studies that record
it", "The rows sum to 0" -- which is what widening the guard to
`unique_values_count is None` was for. The widening was load-bearing, not
defensive.
An arithmetic error in the cost disclosure this round wrote to replace an
earlier wrong one. The per-operation table sums to six additions; the sentence
below it said seven. Corrected, and the memo saving is now stated as measured
rather than as a single number: across all six operations on CYANOKIT + DEATH
in one process, 11 requests against 11 unchanged -- the four descriptive
operations add three and disproportionality then drops from six to two on the
probes they already answered. A drug-only workup is +1. Three is the worst
case, not the typical one.
The inverted cost claim corrected in the previous commit was still live in the
test file that commit edited. `test_serious_events_says_which_cohort_it_
measured` still said probing the narrower cohort "would spend a request to
answer the same question", so source and test disagreed about the same
measurement. The test docstring now matches the source: the wider cohort is
chosen for memo reuse, not because it is cheaper.
A live figure quoted from the wrong query. The Fix-53A-1 comment said the old
`relaxed_query` value matches 253 studies on its own; sent verbatim to
/studies it returns 306. 253 is what that query returns after the tool's own
AREA rewrite -- the query the comment is contrasting it against.
* Fix Round 54: NCBI esearch answered a broader query than asked, silently
esearch drops search terms it cannot match and answers the REMAINING query,
returning a full result set for a question the caller never submitted. Round 53
taught PubMed to read NCBI's disclosure of this. Every other eutils database
behaves identically and eleven other modules parsed esearch responses without
reading either disclosure container.
Measured live across fourteen eutils databases, each probe compared against the
same query with the nonsense terms deleted: all fourteen reported both dropped
phrases in errorlist.phrasesnotfound and returned a count IDENTICAL to the
query with those phrases removed -- the dropped terms constrained nothing.
All fourteen stayed silent (errorlist: null) when nothing was dropped.
The parsing now lives in one place, ncbi_eutils_tool.esearch_query_disclosure,
which carries the measurements; pubmed_tool delegates to it. Wired into six
modules covering seven tools. Three of them were worse than silent: they named
a field for the query that ran and put the SUBMITTED query in it --
NCBI_search_nucleotide and NCBI_SRA_search_runs (data.search_term) and
SRA_search_experiments (data.query_used). Additive: the key appears only when
NCBI reported something, and costs no extra request, since the containers are
already in the response every caller parses.
The sweep is not complete and pubmed_tool now says so: clinvar, dbsnp, pmc, geo
and unified_guideline_tools remain unwired, pmc and icite because they return a
bare list with nowhere additive to put the disclosure.
Also, a hyphenated drug name reached openFDA unquoted. _render_clause quoted a
value only when it contained a SPACE, and a hyphen is a Lucene operator, so a
single-token hyphenated name was reparsed into a union of DIFFERENT drugs while
the response reported in prose how many reports "named" the product.
faers_drug_name_clause had always quoted and gave this exact case as its
reason, so the two paths disagreed on the one input each was written about, and
a third comment called the divergence deliberate.
medicinalproduct:sumatriptan-naproxen 124792 unquoted vs 13 quoted
medicinalproduct:CO-TRIMOXAZOLE 24806 unquoted vs 1416 quoted
FAERS_count_reactions_by_drug_event reported 219,048 reports for
sumatriptan-naproxen -- sumatriptan (51,295) OR naproxen (172,094) -- for a
product with 1,438 reports under its real brand name; 852,859 for the
non-existent MANNITOL-SULFATE; and 141 for an invented product, affirming that
141 reports named it. All three now return the correct count, 0 for the latter
two. Found independently by two personas.
Quoting is now the default with the Lucene range as the only carve-out. The
cost was measured over every other value shape these builders send -- drug
names, generic/brand name, single and multi-word reaction terms, all eight
coded HUMAN_TO_FDA_MAP values, occurcountry, numeric age, and the single-date
receivedate form: quoted and unquoted return identical totals for all of them,
so no query changes except the ones that were wrong. The range carve-out was
re-confirmed the same way (197,676 unquoted, HTTP 404 quoted).
faers_drug_name_clause now calls _render_clause instead of restating the rule,
so the drift cannot recur, and the memo comment that called the divergence
deliberate is corrected: both modules now spell the same question identically
and share one cache entry per drug instead of two.
Nine pre-existing assertions pinned the old unquoted query strings; each was
updated, not deleted, and every structural property they guard -- group
parenthesization, AND/OR nesting, encode-once, range-unquoted -- still holds.
* Fix Round 54 (validation): a zero-hit search is not a broadened query
Validation found that `outputmessages` must not gate the disclosure. NCBI
emits `warninglist.outputmessages: ["No items found."]` on EVERY zero-hit
search, including one it executed exactly as submitted. Measured live:
term "cystic fibrosis AND 1500[dp]"
-> count 0, errorlist null,
warninglist {"phrasesignored": [], "quotedphrasesnotfound": [],
"outputmessages": ["No items found."]}
which produced `query_not_executed_as_submitted: True` and the warning "No
items found." The query WAS run as asked; it simply matched nothing, so the
flag asserted the opposite of the truth, and it was loudest exactly where it
was most wrong. Round 53 shipped this for PubMed alone; this round would have
spread it to seven tools, where every empty MedGen, GEO, SRA and nuccore
search would have gained a block claiming its query was not run as submitted.
Only an actual alteration -- a dropped phrase, an ignored phrase, a dropped
term or an unrecognised field -- now creates the block. `outputmessages` is
still carried alongside one of those, so the zero-hit case that DID drop a
term keeps its disclosure; both directions are pinned by tests, each proven to
fail when the guard is reverted.
Also corrected, all found by the same validation, all authored by this round:
* the shared docstring claimed unaffected responses keep their shape, which
the `outputmessages` gate made untrue on every empty search;
* the unwired-module inventory said "eleven other modules" and omitted
`clinical_society_tools`, whose `_pubmed_search` takes a caller-influenced
term and discards both containers -- it is twelve, and both it and
`icite_tool` are now named;
* `_render_clause` said a range is "the only carve-out" while the code has
two, the second being non-string values, which a test four lines below the
one asserting "the ONLY exemption" already pinned. Both now say so;
* the nuccore test fixture was labelled verbatim but spliced the count from
the "BRCA1 human" probe (7359) onto the errorlist of the "BRCA1" probe
(66893). Corrected to the probe it claims to be;
* the taxonomy fixture cited a measurement absent from the table it defers
to. That fifteenth, zero-hit probe is now recorded there;
* the trimmed PubMed test header still narrated a probe whose fixture was
deleted.
tests/unit + tests/integration: 3 failed, 7 errors, all pre-existing on the
branch base (test_mcp_protocol, and one test_smcp_http_server fixture),
verified by stashing this round's changes and re-running.
* Fix Round 55: result sets truncated without saying so, and pointers to tools that do not exist
CPIC_list_drugs returned 200 of 324 drugs and said nothing. The missing
124 include efavirenz and clozapine, and the omission is not theoretical:
a persona in round 54 concluded from this tool that efavirenz has no CPIC
guidance. Upstream reports `content-range: 0-323/*`; the endpoint had a
paging value hard-coded in its URL template while its own schema also
declared one, so it was sending `limit=200&limit=5`.
Two class invariants now guard the general case rather than the instance:
no endpoint may hard-code a paging value its schema also declares, and no
GeneOntologyTool may declare a parameter absent from its endpoint
template, which `_build_url` can only ever ignore. Both pass repo-wide.
The FAERS_search_* detail family returned a bare array with no total, so
a caller could not distinguish ten reports from the first ten of
thousands -- `rifampin`+`methadone` returned 10 rows against an upstream
total of 35. They now return an envelope carrying `count` and
`total_available`, using the vocabulary `BaseRESTTool` and
`CBioPortalRESTTool` already share rather than inventing a fifth spelling
for the same idea.
Two dead pointers were found by replacing a heuristic with a rule. The
existing pointer test used difflib similarity, which measurably would
have missed 5 of the 9 names the file already pins; a registry-derived
rule (second segment must be a verb shared across at least ten vendor
prefixes) catches those and surfaced two nothing had caught:
ChEMBL_get_molecule_by_chembl_id, quoted in four --help epilog examples
where a user copy-pasting it would exclude nothing, and
UniProt_get_uniref_search in a tool description. Real names are
ChEMBL_get_molecule and UniProt_search_uniref.
FDADrugInteractionDetailTool held 318 lines byte-identical to
FDADrugAdverseEventDetailTool, and this change had been paying every edit
into both copies. It now subclasses and overrides only the query builder,
the same relationship FDACountAdditiveReactionsTool already has with its
parent: 366 lines removed, 56 added.
Verified: CPIC returns 324 with efavirenz and clozapine present, matching
upstream exactly; the six FAERS tools re-run live with identical totals;
46 new tests pass and each fails on revert.
* Fix Round 56: a search that never searched, and results that hid their own obsolescence
WHO_Guidelines_Search returned byte-identical results for every query.
The query was slugified into who.int/health-topics/<slug>; confirmed live
that this 404s for anything WHO does not treat as a health topic
("methadone", "clozapine" and the nonsense token "zzqqxxwubble" all 404,
while "tuberculosis"/"malaria"/"trachoma" return 200). On a 404 the tool
scraped WHO's generic recent-guidelines listing and returned those as the
answer, each tagged is_guideline: true, with nothing marking the query as
unmatched. A differential sweep over all ten guideline search tools found
this was the only one whose output did not vary with the query (SIGN and
CTFPHC return empty for these terms because those small national sets
genuinely lack them, which the sweep confirmed against terms they do
cover).
Topic misses now search WHO IRIS, WHO's official institutional
repository, over its DSpace REST API -- confirmed genuinely
query-responsive: methadone 1027 hits, clozapine 324, trachoma 3964,
"zzqqxxwubble" 0. IRIS has no "guideline" document type, so is_guideline
there reflects the record rather than being asserted for every hit, and
each result reports in matched_via which backend answered it.
Mondo_get_disease on an obsolete term returned success with a fully empty
record -- indistinguishable from a live disease with no annotations. The
only signal was the word "obsolete" inside the name. This is reachable,
not hypothetical: GenCC returns disease_curie MONDO:0014978 for PADI6,
which feeds straight in. The Monarch payload already answers it
(deprecated: True, versus None for the live term) and the tool discarded
the field; the same field was discarded on the /search rows, so a search
listed an obsolete term beside its replacement with nothing to tell them
apart. Obsolete terms now say so and, via OLS4's term_replaced_by, point
at the current term (MONDO:0014978 -> MONDO:1010200, which carries the
PADI6 gene). Costs one extra request on the deprecated path only; live
lookups are unchanged.
ClinVar published a url that described no request: the bare
esummary.fcgi endpoint, identical for every variant and an error when
replayed, because the query lived in params=. Now reports the URL as
sent. A repo-wide AST sweep found 11 sites across 7 files with this
shape; the remaining 10 are listed in the round report, unverified and
not touched here.
All 14 new tests fail when their fix is reverted, per assertion.
* Fix Round 56 (quality): one meaning for is_guideline across both WHO backends
Follow-up from the round's review pass, which found the previous commit
had left is_guideline meaning two different things in adjacent branches
of the same class: derived from the record on the new IRIS path, still
hardcoded True on the health-topic path. Both feed one result list, so a
consumer filtering on the field got an answer that depended on which
backend happened to answer.
Both backends now emit through one _result() shaper. Measured cost, live:
tuberculosis 5/10 rows keep is_guideline True (the consolidated
guidelines; the operational handbooks correctly drop to False), while
trachoma 0/2, malaria 0/5 and onchocerciasis 0/3 go to zero -- those
topic pages list epidemiological updates, procurement guidance and
surveillance toolkits, none of which are guidelines. Callers filtering on
is_guideline see a large drop in yield; the previous yield was asserted
rather than established. The flag is a title/document-type test, which
will miss WHO guidance published as handbooks or manuals, and the tool
description now says so.
Also from the review:
- _replaced_by is MONDO-only. The OBO PURL it builds is wrong by
construction for Orphanet (orpha.net/ORDO, OLS4 ontology "ordo"), EFO
and OMIM, all of which reach _normalize_curie, so those would have
spent a request to learn nothing.
- its timeout is capped at 10s independently of the Monarch timeout, so a
slow OLS4 cannot stall an answer that is already complete.
- the 0.5s sleep now spaces the second who.int request instead of
delaying the first request of every call.
- the per-item closure in _search_iris is a module-level _dc_value.
- corrected this round's own test docstring: chebi_tool and hpo_tool
disclose silent ID substitutions, they do not resolve replacements.
15 tests, all failing when their fix is reverted.
* Fix Round 56 (validation): declare the fields the round added, and correct two claims
From the round's validator pass.
- Monarch_get_entity and Monarch_search now emit `deprecated`, but only
mondo_tools.json declared it; monarch_new_tools.json listed every other
field and omitted this one, so the schema under-described the payload.
- WHO_Guidelines_Search declared `items: {type: object}` with no
properties at all, so none of matched_via, document_type, date_issued
or is_guideline was described anywhere a caller would look. Declared,
with is_guideline stating plainly what it does not detect.
- The WHO description claimed topic pages are used "when the query names
a WHO health topic". _topic_slug also retries on the query's first
significant word: live, "malaria vaccine" is answered by the whole
malaria topic page. matched_via already disclosed the broadening in the
payload; the description now does too.
- Corrected this round's own test docstring, which cited
api-v3.monarchinitiative.org while the tool calls
api.monarchinitiative.org. Re-checked both hosts: deprecated: True on
each, so the finding stands, but the citation now names the host under
test.
- Scoped the ClinVar test's claim to the success and rate-limited paths.
The two exception paths hold no response object and still report the
pre-request URL, which the previous framing did not admit.
* Fix Round 57: empty results that hid a dead identifier, and a URL that described no request
Round 56 fixed Mondo_get_disease to report deprecated/replaced_by, having
found Monarch's payload already carried the field. Its siblings answer the
same dead identifier with an empty success. Confirmed live on
api.monarchinitiative.org for three separate obsolete CURIEs, each with a
populated replacement one hop away: MONDO:0014978 -> MONDO:1010200 (2
phenotypes, PADI6), MONDO:0002906 -> MONDO:0019340 (190 phenotypes),
MONDO:0001111 -> MONDO:0018896 (83 phenotypes). Reachable, not
hypothetical: GenCC_search_gene {"gene_symbol": "PADI6"} hands back
MONDO:0014978, verified in this tree.
These endpoints return rows and nothing else, so there is no deprecated
field in the response to surface -- obsolescence has to be asked for
separately. Hence a hook on the empty path rather than a response shaper.
Mondo_get_disease_phenotypes, MonarchV3_get_histopheno,
MonarchV3_get_mappings and MonarchV3_get_associations now say the CURIE is
obsolete and name its replacement. Cost: one /entity request (measured 259
ms) only when the result set is empty, plus the OLS4 lookup (355 ms) only
when the term really is obsolete; non-empty results and non-MONDO CURIEs
are untouched.
The same shape one family over: HPO_get_diseases_by_phenotype and
HPO_get_genes_by_phenotype returned an empty success for a merged term
while HPO_get_term, in the same module, already detected the merge.
HP:0003114 -> 0 diseases; its replacement HP:0001626 -> 4962. The JAX
annotation endpoint answers a merged ID with HTTP 200 and empty arrays, so
upstream genuinely has nothing and the tool must disclose rather than
recover. The detection is the existing one: /api/hp/terms/<id> silently
serves the replacement's record, so a requested-vs-returned id mismatch is
the merge. The note text moved into a shared helper so both callers keep
one wording.
Round 56 fixed ClinVar's bare esummary.fcgi URL and listed ten more sites
unverified. Verifying them found the listed KEGG and HAL sites are not
instances -- KEGG builds its query into the path, so its published URL is
already complete (rest.kegg.jp/find/pathway/glycolysis), and HAL's is a
per-document link. The real instance is the shared NCBI E-utilities base,
reachable from four tools across three modules: geo_search_datasets,
geo_get_dataset_info, dbsnp_search_by_gene and NCBI_search_nucleotide all
published the bare endpoint, identical for every query, and replaying it
returns "Empty term and query_key - nothing todo". Now reports the URI as
sent; the replayed URL reproduces the request exactly. E-utilities takes no
credentials on these paths, so nothing secret is exposed.
PubMed_Guidelines_Search fetched the number of matching guidelines and
discarded it with a dead expression statement: "therapeutic plasma
exchange" returned 3 rows while esearch reported 94. It already returns a
dict envelope, so publishing metadata is additive here, unlike its four
sibling guideline tools whose bare-list returns keep them deferred.
retrieved is reported separately from returned because the rows are
filtered client-side against the query terms, so only total > retrieved is
upstream truncation.
All four fixes are additive; no contract changed. 31 new tests; each fix's
assertions were proven to fail against its own reverted source.
* Fix Round 57 (quality): one home for three duplicated idioms
Follow-up to the round-57 fixes, from the simplify pass. No behaviour
change except where noted.
The "resolved request URI, else the endpoint" helper existed three times
with two different coercion rules: tcia_tool's _requested_url, the new one
in ncbi_eutils_tool, and base_rest_tool's echo_request_url gate. The first
two were byte-equivalent, so they now share request_url() in base_tool.py.
base_rest_tool's gated variant is left alone -- defaulting it on is a
behaviour change across every BaseRESTTool, recorded as a follow-up rather
than smuggled into a cleanup.
The Mondo deprecation note was built twice, and the wording had already
drifted between the two copies in the same round. Both now call
_deprecation_metadata(), which takes the clause naming what is empty so one
sentence serves a stripped record and a stripped result set. This also
carries the OLS4 timeout cap, which _obsolescence_metadata had copied and
recomputed, to both callers.
The three row-returning Monarch endpoints had grown identical
build-metadata / probe-if-empty / envelope triples; they share _rows().
HPO's merge disclosure was factored down to the note string only, leaving
the three-key dict written twice. _merge_disclosure() now owns the keys and
the id-mismatch test, so _get_term and _merge_metadata cannot drift.
Two real improvements fell out of the review rather than the round:
_merge_metadata was passing the full 30s timeout where its Monarch twin
caps at 10s, and the JAX annotation payload carries genes, diseases and
assays together -- so a non-empty sibling array already proves the term
resolves, and the probe is now skipped in that case instead of spending a
round-trip to learn nothing.
PubMedGuidelinesTool._search_pubmed_guidelines returned a 2-tuple whose
first element was either a list or an error dict, so run() unpacked a value
it discarded on one branch and two error paths padded a {} nobody read. It
now returns the finished envelope and run() returns it directly.
That last change broke test_pubmed_guidelines_envelope.py, which stubbed
the private helper's old return shape -- found in the full suite, not in
the targeted re-run. The Fix-R9E-1 guard it encodes is still correct and
still wanted, so the three tests were rewritten to stub the HTTP session
instead of the helper; they now assert what run() hands a caller, which is
what R9E-1 was about, and no longer break on an internal refactor.
Skipped, with reasons: memoizing the deprecation probe (process-global
state across tests, against this repo's hermeticity rule); dropping
`truncated` as derivable (sibling search tools publish it, so consistency
wins); wiring esearch_query_disclosure into the guideline search (a
behaviour addition, not a cleanup); publishing totals for the four sibling
guideline tools (contract change, deferred).
* Fix Round 57 (validation): pin the shared note, and make the probe guard match its comment
From the round's validator pass. All three were reported as observations,
not gate failures; the suite was green before and after.
The Mondo deprecation note's tail changed wording when Fix-R57-4 moved the
sentence into a helper, and nothing caught it: the existing test asserted
only that "obsolete" and the replacement CURIE appeared in the string. HPO's
equivalent wording was already pinned, so the asymmetry meant Mondo's could
drift again silently. Both clauses are now pinned verbatim -- the one naming
what is empty is exactly the part that differs per caller, so it is the part
worth holding.
The HPO probe guard's comment claimed the annotation payload's four arrays
are all checked; the code checked three. Rather than narrow the comment, the
guard now checks medicalActions too, so a term annotated only with medical
actions no longer spends a round-trip to learn it is live.
Both PubMed error-path tests raised a bare OSError, which lands in the
blanket handler rather than the requests.RequestException branch the test
names describe -- so the documented branch was asserted only via the
catch-all. Parametrized over both.
Targeted suite: 229 passed, 0 failed.
* Fix Round 58: three values parsed and dropped, one of them naming the wrong species
BVBRC_search_specialty_genes selected `organism` -- BV-BRC's reference
annotation organism -- and not `genome_name`, the genome the hit was found
in. The two disagree routinely, so "which species carries blaKPC" was
answered with the wrong species: sp_gene row genome_id 550.1655 reported
organism "Klebsiella pneumoniae" for a genome named "Enterobacter cloacae
strain 1-RC-17-04409-1", contradicting its own adjacent taxon_id 550. The
sibling BVBRC_search_amr already selects genome_name; select it here too and
keep `organism` in place.
gnomad_get_gene_constraints asked only for gnomad_constraint and
exac_constraint, which gnomAD leaves null for mitochondrially-encoded genes
because it scores them under mitochondrial_constraint. MT-ND4 therefore
returned success with both null and no note, reading as "unconstrained" for
a gene gnomAD scores at oe_lof_upper 0.022. Request the field, both union
branches (protein and RNA), and say plainly when a gene has none of the
three rather than leaving nulls to be misread. The live query lives in the
shipped config, so the fix is there; the class default is kept in step.
UniProt_id_mapping hardcoded failed_ids to [] and reported the returned page
size as mapped_count, so Q9F663+NOTAREALACC99+P0DTC2 came back
"mapped_count 25, failed_ids []" with one valid accession and one typo'd one
silently absent, against an upstream x-total-results of 2236. Read failedIds
and the total, and stop treating the 303-redirected status page as final --
the resolution Fix-R20A-1 already applied to the sibling
UniProtIDMappingTool, which is a second copy of this logic. Q9F663 now
returns 83 of 83 rather than 25.
Return schemas updated to match, including gnomad_constraint's nullability,
which the old schema forbade.
* Fix Round 58: guideline searches reported a page of rows as the whole corpus
Six guideline backends publish a match count. Five parsed it and dropped it
with a bare expression statement -- Europe PMC `data.get("hitCount", 0)`,
TRIP `int(total.text) if total is not None else 0`, OpenAlex
`data.get("meta", {})` -- and NICE and WHO IRIS never read theirs. Each tool
then returned a bare list, so there was nowhere to put the figure even once
read, and `limit` rows were indistinguishable from everything that matched.
Measured live against the hosts the tools call: NICE "infection" is 1057
documents, of which the tool served 15; TRIP vancomycin 12638; WHO IRIS
tuberculosis 28435; OpenAlex 18266; Europe PMC 9972; PubMed 94.
Round 57 fixed PubMed_Guidelines_Search alone and deferred the rest rather
than leave the family half-converted. That left it worse than half: the new
envelope validated against neither branch of the tool's own declared
return_schema (`oneOf: [array, {required: [error]}]`), and
examples/workflows/clinical_guidelines_search.py, which unwraps `guidelines`
and `results` but not `data`, silently dropped every PubMed row.
So convert the whole set to the {status, data, metadata} envelope that
PubMed_Guidelines_Search and EuropePMC_search_articles already use, and fix
all six return_schemas -- including PubMed's, which was wrong before this
change. `retrieved` is separate from `returned` because several of these
filter rows client-side, so only total > retrieved is upstream truncation.
TRIP's <count> is deliberately not used: limit=3 answers <count>20</count>,
a page size, which is the defect being removed.
GIN, CMA, SIGN and CTFPHC keep their bare-list contract. Their sources
publish no match count, so an envelope could only report total ==
len(results) -- manufacturing the same defect in six more tools. The split
is by whether a real total exists, and is stated in the guide.
Also fixes a NICE test that asserts guidance ids start with ng/cg. NICE
serves in-development guidance under /guidance/indevelopment/<id> and ranks
one into "diabetes" today, so that assertion fails on unmodified main; this
was confirmed by stashing the change and re-running.
* Fix Round 58 (validation): restore the entry objects a wider page had flattened
The validator caught a regression in the round's own UniProt change. For
`to_db=UniProtKB` the job's 303 redirect lands on
/idmapping/uniprotkb/results/, which streams full entries; the generic
/idmapping/results/ returns bare accession strings. Re-fetching the generic
URL to get a bigger page therefore replaced every {accession, id, gene_name}
object with a plain string -- P04637 came back "P04637" instead of carrying
P53_HUMAN. Re-fetch through the URL the redirect actually landed on, which
keeps the shape while still asking for a full page, and skip the second
request entirely when the status response already carried every row (its
x-total-results and failedIds are on that response too). P04637 -> PDB still
returns 295 of 295, against 25 before the round.
`to` as a bare string was never admitted by the declared return_schema even
though the tool has always returned it for non-UniProtKB targets, so the
schema now carries both forms -- the same "output matching neither branch"
defect this round set out to fix, found one layer down.
Quality pass over the round's own diff:
* `_guideline_envelope` now pairs `truncated` with a `truncation_note`.
Every other truncation discloser in the repo says what to do about it;
a bare boolean states the fact and withholds the remedy.
* Dropped two empty-result early returns in Europe PMC and TRIP that
produced byte-identical output to the normal exit, and the PubMed
envelope closure, now a bare partial application over two call sites.
* `_note_absent_constraints`/`_CONSTRAINT_KEYS` moved to module level in
gnomad_tool.py beside `_missing_callset_note` and `describe_dataset`,
which handle the same genre of concern; also fixes a ruff-format
violation the validator flagged.
* Deduplicated a six-fold copy-pasted unwrap in test_guideline_tools.py.
* Removed test_both_organism_fields_reach_the_caller: it stubbed the
request helper with a hand-built row, so the select_fields list under
test was never consulted and it passed against pre-fix code.
* test_guideline_search_totals.py now asserts every search tool in the
config is classified, so a new one cannot land unguarded.
Declared BVBRC `genome_name` and gnomAD `note` in their return schemas,
which the validator found undeclared.
* Fix Round 59: CPIC_get_recommendations dropped rows without saying so
CPIC_get_recommendations applies a documented default `limit` of 50 but
reported only `count`, which is the size of the returned page. Confirmed live
that guideline 100416 (CYP2D6/OPRM1/COMT and opioids) has 66 recommendation
rows, so the default response was `count: 50` with no note and no total --
indistinguishable from the guideline having exactly 50, while 16 rows were
dropped. Codeine, tramadol and hydrocodone guidance all lives on that
guideline.
tu run CPIC_get_recommendations '{"guideline_id": 100416}'
before: count 50 (no total, no note)
after: count 50, total_count 66, offset 0,
note "Showing recommendations 1-50 of 66 ... offset=50 ..."
This is the same defect CPICGetAllelesTool in the same module already fixed;
its class docstring describes it. That tool's Content-Range helper is hoisted
to module level and reused, and the near-identical paging sentence both tools
build is now one `_paging_note` helper -- the alleles note comes out
byte-identical, verified live. The exact server-side COUNT is requested only
on the unfiltered path: when gene/phenotype filtering is applied the request
already pulls limit=1000 and the total that matters is the number of matching
rows, counted locally, so asking PostgREST for a count there would be work
thrown away. Purely additive -- no existing key changes value.
CPIC_search_gene_drug_pairs documented `limit` as "(default 50)". There is no
such default: confirmed live that an unfiltered search returns all 635 pairs
and a CYP2D6 search returns all 81. Returning everything is the better
behaviour, so the description is what was corrected.
Not done, deliberately: the nine CPIC return_schemas were NOT rewritten to
describe the {status, data, ...} envelope. That change was made, then reverted
after it was proven to break `tu test` 2/2 on CPIC_get_gene_info. Per issue
#246 the repo convention is that return_schema describes the inner `data`
payload -- cli.py validates `result["data"]` against it, and
tests/tools/test_return_schema_data_shapes.py asserts an envelope-shaped
schema must fail. 319 of the 328 envelope-returning configs already follow
the payload convention. The nine CPIC schemas were correct as shipped.
* Fix Round 60: Crossref searches reported a page of works as the whole corpus
CrossrefRESTTool._process_response extracted message["items"] and set
`count` to len(items), discarding message["total-results"] -- the only number
Crossref publishes for how many works actually match. With the documented
`limit` defaults (10 for works, 20 for funders/members) that made the page
size look like the size of the result set, on all four Crossref list
endpoints.
tu run Crossref_search_works '{"query": "CRISPR gene editing", "limit": 3}'
before: count 3 (no total, no note)
after: count 3, total_count 1037336,
note "Showing results 1-3 of 1037336 ... offset=3 ..."
tu run Crossref_search_members '{"query": "university", "limit": 3}'
before: count 3 after: count 3, total_count 4032
tu run Crossref_list_funders '{"query": "cancer", "limit": 3}'
before: count 3 after: count 3, total_count 746
Swept all eight CrossrefRESTTool configs. Four are list endpoints and all
four publish total-results upstream (works, funders, members, types); the
other four are detail lookups whose message carries no `items` and are left
byte-identical. /types is the one list endpoint that never diverges -- it
returns all 30 rows -- so it reports total_count 30 and correctly emits no
truncation note.
`offset` is added to the three searchable list tools so the note's advice is
actionable; verified live that Crossref pages on it (funders "cancer"
offset=0 -> Canceropôle PACA, Instituto Nacional de Cancerología; offset=2 ->
Cancerfonden, Syöpäsäätiö) and that the note counts from the caller's offset,
not from zero. The offset is read back off the URL requests actually
assembled, so it cannot drift from what was sent.
Purely additive: `data`, `count` and `url` keep the values they always had,
and total_count/note are omitted entirely when Crossref sends no usable
total. return_schema is untouched -- per issue #246 it describes the inner
`data` payload, and the new keys live in the envelope.
* Fix Round 60: a merged UniProt taxon was answered as the one that was asked for
UniProtTaxonomy_get_taxon fetched rest.uniprot.org/taxonomy/{id} and read
taxonId straight out of the body. UniProt answers a merged (retired) taxon
with an HTTP 303 to the node it was merged into, so requests followed the
redirect and the tool returned a different taxon than the caller asked for --
with the requested id appearing nowhere in the response, making the
substitution undetectable.
tu run UniProtTaxonomy_get_taxon '{"taxon_id": "46170"}'
before: taxon_id 1280, "Staphylococcus aureus" (46170 absent entirely)
after: query_taxon_id 46170, taxon_id 1280, merged_from 46170,
metadata.note "...merged it into 1280 ... the protein counts
are for the merged node, which may sit at a higher rank..."
Taxon 46170 is Staphylococcus aureus subsp. aureus, the subspecies node many
older BioSample/BioProject MRSA records are filed under; 1280 is the species.
Silently swapping one for the other inflates the protein-count denominator
any per-subspecies estimate is built on. UniProt publishes the merge plainly
-- `HTTP/2 303 ... location: /taxonomy/1280?from=46170`, and with redirects
disabled the body is
`{"taxonId":46170,"inactiveReason":{"inactiveReasonType":"MERGED","mergedTo":1280}}`
-- so the information was available and simply discarded.
Same shape as the obsolete-identifier disclosure already shipped for Mondo,
Monarch and HPO: answer with the current record, but say so. Detection
compares the requested id against the returned one, which needs no extra
request, so the fix costs nothing on the wire. String and integer ids compare
equal ('9606' and 9606 are one taxon) so an active lookup can never be
mislabelled a merge, and a non-numeric id degrades to no comparison rather
than an exception.
Purely additive: every field that was there before keeps its value, and
query_taxon_id is echoed on the happy path too so a caller can always join
request to response. The sibling UniProtTaxonomy_search is untouched.
* Fix Round 60: HGNC alias resolution note read 'a alias symbol'
The symbol-resolution note was templated as "a {relation}", and one of the
two relation labels starts with a vowel, so every alias hit -- the more
common of the two -- produced "a alias symbol".
tu run HGNC_fetch_gene_by_symbol '{"symbol": "CAH1"}'
before: "...listed as a alias symbol of 'CYP21A2'..."
after: "...listed as an alias symbol of 'CYP21A2'..."
tu run HGNC_fetch_gene_by_symbol '{"symbol": "CYP21"}'
before/after: "...listed as a previous symbol of 'CYP21A2'..." (unchanged)
Cosmetic only -- no key, value or control flow changes -- but this note is
the tool's disclosure that it answered with a different gene than the one
asked for, so it is worth having it read as written prose. The existing test
asserted only that 'alias symbol' appears in the note, which the defect
satisfied; it now pins the article, and a new case pins the consonant-initial
label so the helper cannot regress the other direction.
* Fix Round 60: OpenFoodFacts_search_products documented a default it never applied
page_size was described as "Default: 10". No default was ever applied: the
schema declares none, so BaseRESTTool._build_params has nothing to inject and
Open Food Facts' own default for /cgi/search.pl is what callers get.
tu run OpenFoodFacts_search_products '{"search_terms": "almond milk"}'
documented: 10 products
actual: page_size 50, 50 products
Swept both Open Food Facts tools that document a page_size default. The
sibling OpenFoodFacts_filter_products_by_tags runs against a different
endpoint and returns exactly the 20 it documents -- confirmed live -- so only
one of the two descriptions was wrong, and the correction is deliberately not
applied to both.
Returning 50 is the better behaviour (a caller who wants 10 passes 10), so
the description is what was corrected rather than the behaviour -- no
response changes value. The test pins the two descriptions against each other
so the wrong number cannot come back, and asserts this tool still declares no
schema default, since adding one would override upstream's and silently make
the description wrong again.
Not changed: this tool already reports Open Food Facts' true match count as
'count' (2577 for "almond milk" against a 50-row page), so it does not have
the page-size-as-total defect.
* Fix Round 60 (review): adopt the repo disclosure vocabulary; close a regression
Applied the findings from the simplify review and the tool-validator run.
Crossref:
- `note` -> `truncation_note`, plus a machine-readable `truncated` bool, so
a caller never has to string-match prose to learn it got a page. `note` was
also the key BaseRESTTool.run already uses for `fields.empty_result_note`,
i.e. the same key with two meanings in one class hierarchy.
- the sentence moves into a named module-level `_truncation_fields()`, so it
has one home rather than being an f-string buried in `_process_response`.
- the note no longer advises a page that cannot be fetched: Crossref caps
offset+rows at 10000 (measured: offset=9000 succeeds, offset=9999 returns
HTTP 400), so for a larger result set it now says only the first 10000 are
reachable by paging and points at 'filter'. Confirmed live that the caveat
appears for the 1037336-match query and not for the 746-match one.
- Crossref_search_works.limit gains the minimum/maximum its two siblings
already declare, since the note advertises "max 100".
Regression found, proven and closed: dropping `TypeError` from the offset
parser broke tests/unit/test_scientometrics_depth.py::test_parses_member_list.
That suite drives the tool through a MagicMock response whose `url` attribute
is a truthy Mock rather than a string, and urlparse raises TypeError on it --
turning the whole call into status: error. Both URL parsers now check
isinstance(url, str) explicitly, which is load-bearing rather than defensive
typing, and a new test pins the MagicMock path so it cannot recur.
UniProt taxonomy:
- the disclosure now reads the evidence UniProt publishes instead of
inferring it. The redirected URL carries ?from=<old> and the inactive stub
carries inactiveReason.mergedTo; when either is present the note states the
merge, and when only the ids differ it says UniProt did not state why. The
previous version asserted "UniProt has merged it into N" for any mismatch,
which is a claim it had not checked -- the same hedge ChEBI's redirect
disclosure already makes. Still zero extra requests.
- `query_taxon_id` is normalized, so it is directly comparable to
`taxon_id` without the caller coercing "9606" against 9606.
- `common_name` becomes ["null", "string"], matching the sibling
UniProtTaxonomy_search schema. Pre-existing, but this round promotes 46170
as the flagship example and 1280 has no common name, so the one example the
description advertises did not validate.
- `_as_taxon_int` is annotated Optional[int] rather than Any.
HGNC: the a/an choice becomes a two-entry table beside _RELATION_LABEL rather
than a general English article rule, since the input is those two fixed labels.
OpenFoodFacts: the schema now declares default: 50 rather than only describing
Open Food Facts' unversioned server default, so BaseRESTTool sends it
explicitly and the number is owned here. Verified live the response is
unchanged (page_size 50, count 2577) and an explicit page_size: 10 is still
honoured.
Long comments that merely restated the test docstrings are trimmed to the
mechanism and the decision; the live transcripts now live only in the tests.
All 8 Crossref tools, both UniProt taxonomy tools: built-in examples green.
* Fix Round 61: OMA paging parameters that were declared but not applied
OMA_get_orthologs forwarded `per_page` to /protein/{id}/orthologs/, which
accepts it, returns HTTP 200, and ignores it. Measured against P04637,
per_page of 3, 20 and 100 each returned all 157 orthologs, and the response
carries no Link, X-Total-Count or content-range header to page with. The
schema promised "default: 20, max: 100"; a caller asking for 3 received 157,
and the repo's own test_examples ask for 10.
The limit is now applied after fetching, since the full set is already in
hand, and the response says so: `count`, `total_count` and `truncated`, the
vocabulary round 60 settled on, plus a `truncation_note` naming a request
that actually works. `total_orthologs` is removed rather than redefined --
it meant "rows returned", the two coincided only while nothing was sliced,
and a key that silently changes meaning is worse than one that is absent.
No cap is applied here: with no server-side paging, capping at the
advertised 100 would strand orthologs 101-157 rather than merely defer them,
so the parameter description drops a "max" that was never enforced anyway.
Separately, all three nullable paging parameters crashed on their own
declared value. oma_tools.json types them ["integer", "null"], and each call
site then ran min(value, 100), raising TypeError: '<' not supported between
instances of 'int' and 'NoneType'. That reached the user as "Unexpected
error querying OMA" naming neither the parameter nor the cause. Confirmed
live on OMA_get_orthologs, OMA_resolve_xref and OMA_get_genome_pair_orthologs;
one `_page_size` helper now treats absent and null alike and tolerates the
strings CLI arguments arrive as.
Scope is one endpoint, not the module: /pairs/{g1}/{g2}/ does honour
per_page (measured: 3 -> 3 rows, 20 -> 20, each with a Link header), so
OMA_get_genome_pair_orthologs keeps forwarding it and gains only the null
fix, and OMA_resolve_xref already sliced client-side and disclosed its total.
Verified live after the change: per_page=3 -> 3 rows, default -> 20,
per_page=null -> 20, per_page=157 -> 157 with truncated false; pairs still
3 -> 3 and 20 -> 20; xref still 5 rows with total_matches 4357. 24 new tests
pass, 9 of them fail when the behaviour is reverted; full unit suite 5596
passed, 27 skipped.
* Fix Round 61 (correction): the null-argument fix is hardening, not a user bug
The previous commit claimed the TypeError on a null paging argument "reached
the user as Unexpected error querying OMA". That is wrong and this corrects
the record rather than quietly restating it.
execute_function.py:3115 strips None arguments before dispatch -- "None means
not provided -- simply omit such keys" -- so run_one_function never delivered
one. Verified against the pre-fix code through that path: all three OMA tools
returned success with a null paging argument. The crash is reachable only by
constructing the tool class directly, as this repo's own unit tests do. The
fix is kept, because a tool that accepts its own declared argument on only one
entry point is a trap for the next caller, but it is hardening and is now
labelled as such in both the helper and the test module.
Found while sweeping the rest of the repo for the same shape: 59 min(<arg>, N)
sites fed by arguments.get, 34 of them with no null guard, 14 (tool, param)
pairs where the parameter is also declared nullable. All 7 reachable ones were
live-tested through run_one_function and none crashed, which is what prompted
re-testing the OMA claim by the same route. That sweep is a negative result:
this class of defect is contained by the framework, and no further module
needs the change.
What the previous commit got right is unaffected and was re-confirmed through
run_one_function: on the pre-fix code OMA_get_orthologs returned all 157
orthologs for per_page=3 and for per_page=20, exactly as a user would hit it.