feat: Adding a clinical trial agent notebook including test data#196
feat: Adding a clinical trial agent notebook including test data#196Hipnozata wants to merge 6 commits into
Conversation
|
Thanks for this contribution, @Hipnozata — the clinical-trial matching agent is a great addition! 🙌 One small suggestion before merge: the notebook and README pin (No rush — happy to help if useful.) |
|
@scouturier just updated with the newest model, nice catch! Looking forward to seeing it go public :) |
There was a problem hiding this comment.
Thanks @Hipnozata. I did a thorough pass and also ran the notebook end-to-end against Bedrock in us-east-1. The overall design is sound, but there's a set of items to address before merge — mostly correctness/safety details in the eligibility logic, plus one that stops the notebook running on a clean install. Summarizing here so you have the full picture, with specifics inline. Where I note "confirmed by running it," that's observed behavior from the live run, not just a code read.
Should fix before merge (correctness / safety):
- Notebook fails on a clean install —
BedrockModelis created with nomax_tokens, andrequirements.txtpinsstrands-agents>=0.1.0(unbounded). A fresh install pulls current strands, and the flagshipagent("Match trials for patient P-001")cell raisesMaxTokensReachedExceptionbefore producing output. Setting an explicitmax_tokens(8192 worked for me) and bounding the strands pin fixes it — confirmed by running it. - Inverted exclusion logic —
aggregate_verdictreturnsnot_eligiblewhen a patient passes an exclusion (i.e. is not excluded), the opposite of the prompt's convention. Confirmed by running it: every result came backnot_eligibleeven when all criteria passed. - Fabricated eligibility criteria in Quick Start —
_transform_studydrops the API'seligibilityModule, and the KB is off by default, so the agent infers criteria from the trial title rather than real ones. Confirmed by running it: allretrieve_trial_kbcalls returnedKB_NOT_CONFIGUREDand the reasoning cited only title-derived criteria. - Parse failure defaults to
eligible— an empty criteria parse yields a confident eligible match with zero criteria; safer asneeds_review. overall_match_scoreis never computed — it's the ranking key (and the README's "Score: 92%"), but it's pure LLM free-text.- Biomarker borderline check is dead — shipped values are unit-bearing (
"30%","850 pg/mL"), which fail_is_numeric, so the "within 10% of cutoff" rule never fires. - Quick Start clone URL 404s — points at a repo that doesn't exist.
With all seven addressed, the notebook runs clean end-to-end (I verified the runtime path once max_tokens was set), so this should be the last correctness pass.
Nice to tidy up (non-blocking):
- A few unrelated changes rode along (LICENSE year bumps in other folders, a
package.json/yarn.lockdep removal, anext-env.d.tsedit) — worth splitting into a separate housekeeping PR.
Happy to help with any of these.
| "\n", | ||
| " if ctype == \"inclusion\" and outcome == \"fail\":\n", | ||
| " return \"not_eligible\"\n", | ||
| " if ctype == \"exclusion\" and outcome == \"pass\":\n", |
There was a problem hiding this comment.
Inverted exclusion logic (should fix). The prompt defines an exclusion "pass" as the patient does NOT meet the exclusion — i.e. a good outcome — and "fail" as the patient MEETS the exclusion (should be excluded). This branch returns not_eligible on exclusion "pass", and nothing acts on exclusion "fail", so the result is exactly backwards: patients who clear an exclusion are marked ineligible, while patients who actually hit an exclusion pass through.
Confirmed by running it: every match for P-001 came back not_eligible even though all criteria were pass/unknown with no fail — the exclusion "pass" entries flipped the verdict.
Suggest flipping to outcome == "fail" for the exclusion branch, and updating the docstring rules + the cell's assert aggregate_verdict([{"type": "exclusion", "outcome": "pass"}]) == "not_eligible" (it currently encodes the inverted convention).
| " status_module = protocol.get(\"statusModule\", {})\n", | ||
| " conditions_module = protocol.get(\"conditionsModule\", {})\n", | ||
| " phases = design_module.get(\"phases\", [])\n", | ||
| " return {\n", |
There was a problem hiding this comment.
Fabricated eligibility criteria in the default path (should fix). _transform_study returns only nct_id/title/phase/status/conditions/locations and never reads protocolSection.eligibilityModule.eligibilityCriteria, which the ClinicalTrials.gov v2 response already includes (the request sets no fields filter). Since retrieve_trial_kb is disabled in Quick Start (KNOWLEDGE_BASE_ID=""), the agent has no factual source of criteria and infers them from the trial title.
Suggest carrying eligibilityCriteria through here and parsing it with split_eligibility_criteria (used in setup_knowledge_base.ipynb; it'll need to be copied into this notebook since notebooks don't share a namespace), then surfacing the parsed inclusion/exclusion lists in the tool output so the agent evaluates real criteria.
| " if outcome == \"unknown\":\n", | ||
| " has_unknown = True\n", | ||
| "\n", | ||
| " return \"needs_review\" if has_unknown else \"eligible\"\n", |
There was a problem hiding this comment.
Empty criteria defaults to eligible (should fix). When _parse_criteria_response can't extract JSON it returns [], and an empty list reaches this line with has_unknown still False — so a malformed model response becomes a confident eligible verdict with zero criteria evaluated. For a clinical-eligibility tool the fail-safe should be needs_review.
Suggest an early guard (if not criteria: return "needs_review") or detecting the empty parse in evaluate_eligibility. Worth also wrapping the json.loads calls in the regex branches of _parse_criteria_response, which can raise JSONDecodeError uncaught.
| "`evaluate_eligibility` with the patient profile and the trial's inclusion \\\n", | ||
| "and exclusion criteria. This returns a per-criterion assessment with an \\\n", | ||
| "overall verdict (`eligible`, `not_eligible`, or `needs_review`).\n", | ||
| "5. **Rank and Return Top 5** — Rank trials by `overall_match_score` in \\\n", |
There was a problem hiding this comment.
overall_match_score is never computed (should fix). It's the ranking key here (and appears as "Score: 92%" in the README), and the output contract declares it a float in [0, 1] — but no code produces it. EligibilityAssessment.to_dict() returns only overall_verdict/criteria/borderline_flag, and display_results just reads result.get("overall_match_score", 0) straight from the model's free-text output. So the value that orders the top-5 results is an ungrounded LLM number in an otherwise deterministic pipeline.
Suggest computing it deterministically from the criterion outcomes and writing it into the assessment dict so the ranking is grounded.
| "\n", | ||
| " for name, threshold in (criterion.get(\"biomarker_thresholds\") or {}).items():\n", | ||
| " patient_value = biomarkers.get(name)\n", | ||
| " if patient_value is None or not _is_numeric(patient_value) or not _is_numeric(threshold):\n", |
There was a problem hiding this comment.
Biomarker borderline check is effectively dead (should fix). The shipped patient profiles store unit-bearing biomarker values ("lvef": "30%", "bnp": "850 pg/mL", "hba1c": "8.2%"). _is_numeric("30%") is False, so this guard continues for every unit-bearing value and the "within 10% of a numeric cutoff" logic never runs — only the age rule can trigger a borderline flag, despite the README advertising the biomarker case.
Suggest stripping units before the numeric compare, or storing numeric values with a separate units field (more robust against unit mismatch between patient value and threshold).
| @@ -0,0 +1,5 @@ | |||
| strands-agents>=0.1.0 | |||
There was a problem hiding this comment.
Unbounded strands-agents pin (should fix — pairs with the max_tokens fix). >=0.1.0 has no upper bound, so a fresh install resolves to whatever strands is current (1.45.x today), whose max_tokens handling differs from whatever this notebook was authored against — which is how the primary demo cell ends up raising MaxTokensReachedException. Suggest pinning to a tested range you've verified, e.g. strands-agents>=1.0,<2.0 (adjust to whatever you confirm works), so the sample is reproducible over time.
Co-authored-by: Sebastien <sebcouturier@gmail.com>
…gent.ipynb Co-authored-by: Sebastien <sebcouturier@gmail.com>
feat: Adding a sample for HCLS specifically clinical trials matching agent. The agent works with randomly generated patient profiles and matches them to publicly available clinical trials. Included in the folder you can find an necessary requirements, the generated data and a notebook with the code itself.