Close three ICI/TMB evidence-provenance gaps - #520
Merged
Conversation
Each of these let a lookup return a confidently wrong or silently absent answer where the curation contract says it should be explicit. 1. STAD_MSI inherited the pooled stomach median. STAD's curated 5.0 mut/Mb is the Chalmers intestinal-type panel value (Table 1, n=58); MSI-H gastric tumours are hypermutated, so the ancestor walk reported 5.0 for the one gastric subtype whose TMB drives checkpoint reasoning. Neither Chalmers nor TCGA-STAD publishes an MSI-stratified gastric median (TCGA only separates 74 hypermutated tumours above an 11.4 mut/Mb threshold), so this is curated as an audited gap rather than an invented estimate. CRC_MSI and UCEC_MSI, which do have published subtype values, are unchanged. 2. The ICI response table could not express an audited gap. Blank-ORR rows were dropped by the value maps and rejected outright by the estimates-join integrity check, so every uncurated code returned a bare "missing" -- a reviewed "no defensible aggregate exists" was indistinguishable from a code nobody had looked at. The missing_reason column existed but was always None. ICI now carries the same contract oncoref.tmb has: curated gap rows resolve with inheritance_kind="direct_missing" and has_ici_response_source=True, carry source_scope/missing_reason, and outrank ancestor inheritance. 3. CRC, RCC, BRCA and SARC reported no ICI evidence at all while their children were richly curated (CRC_MSI has 35 estimate rows, KIRC 29). For these the aggregate is not merely unavailable, it is not meaningful: response is determined by MMR status, renal histology, receptor subtype and sarcoma histology respectively. Each is now an explicit gap naming the stratified code to use instead. Behaviour for valued rows is unchanged; three existing tests were scoped to valued rows because gap rows have no regimen, trial, or backing estimate. Claude-Session: https://claude.ai/code/session_01UAsZy5ieKeAVtePA6nnGHr
Follow-up on the review of the ICI/TMB evidence-gap work. The contract was right in shape but too permissive, and two curation notes were wrong. Contract: - Only codes declared in _ICI_EVIDENCE_OVERRIDES may carry a blank orr_pct. Previously any blank value skipped the estimates-join integrity check and was stamped with fall-through provenance, so a data-entry slip minted a fabricated "audited gap" -- the inverse of the contract's purpose. Undeclared blanks raise again, and response_anchor_evidence_df takes an explicit gap_codes set so the shared aPD-1 table keeps the strict behaviour. - Gap rows must leave regimen blank and a code cannot be both valued and a gap; both are enforced. This keeps the code-keyed gap lookup unambiguous instead of letting a per-regimen blank suppress a code's other regimens. - The gap check now runs before the inherit=False early return, so an audited gap is reported whether or not inheritance is enabled, matching tmb._resolve_tmb_row's ordering. - cancer_ici_response_record returns the gap record instead of None, matching tmb.cancer_tmb_record, so the record surface can tell a reviewed gap from an uncurated code. The fallback=False value path got the guard it was missing. - Gap rows no longer publish a therapy_regimen_class or claim themselves as their own evidence_source_code, and the ValueError message survives a blank regimen (fillna before astype; the raise had no test coverage). Curation: - The CRC note cited KEYNOTE-016; the CRC_MSI PD-1 anchor is KEYNOTE-177. It also sent readers to COAD/READ, which are prevalence-weighted derived_blend values over the MSI-H and MSS populations -- the averaging the note argues against. Both corrected. - STAD_MSI was half-audited: an explicit TMB gap but still inheriting STAD's all-comer ORR. It now carries the matching ICI gap, which also gives the inheritance-blocking test a code with a real parent to exercise (the four original gap codes are registry roots, so that test could not fail). Docs: the parent walk borrows from an ancestor, not from a child; wording corrected and the declared-gap requirement documented. Claude-Session: https://claude.ai/code/session_01UAsZy5ieKeAVtePA6nnGHr
A code declared as an audited gap must not also carry a valued anchor -- that contradiction now raises instead of letting the code resolve two ways. The review also flagged that duplicate gap rows for one code would silently collapse the code-keyed lookup. That hole does not exist: pandas counts duplicate NaN merge keys as duplicates in the left frame, so the existing validate="one_to_one" merge already rejects it. The guard I added for it was unreachable and is removed; the test now asserts the behaviour that actually holds. Claude-Session: https://claude.ai/code/session_01UAsZy5ieKeAVtePA6nnGHr
Curation accuracy. Every PMID cited in the new gap notes was checked against NCBI, and the quantitative claims against the primary sources. Three notes were wrong: - RCC claimed clear-cell and non-clear-cell have "materially different response". They do not: KIRC PD-1 is 25.0% and RCC_NCC 26.7%. The real justification is that the member histologies carry anchors from separate trials spanning 9.5% (KICH) to 42% (KIRC nivolumab+ipilimumab), and no trial enrolled the histology-spanning aggregate. Rewritten to say that. - SARC attributed SARC_GIST's 0% to "SARC028 and the AcSe/DART programmes"; it is Alliance A091401 (PMID:39343511). It also cited SARC_UPS's 23% to the SARC028 primary paper, but that value comes from the expansion cohorts (DOI:10.1200/JCO.2019.37.15_suppl.11015). Both corrected. - BRCA asserted hormone-receptor-positive disease "has no comparable single-agent anchor", a claim about the literature rather than the table. Narrowed to what is verifiable: nothing is curated for those subtypes, so an all-comer breast ORR would extrapolate TNBC evidence to populations the cited trials did not enrol. The TMB note now cites the decisive primary-source fact instead of a paraphrase: TCGA-STAD analysed "the 215 tumours with mutation rates below 11.4 mutations per megabase (none of which were MSI-positive)" separately from 74 hypermutated tumours (PMID:25079317), so the pooled 5.0 mut/Mb demonstrably cannot cover MSI-H gastric. Chalmers' gastric-specific statement replaces the pan-cancer 83%/97% figures, which were true but not about stomach. Two new tests keep the prose honest: every PMID/DOI cited in a gap note must resolve to a real anchor row, and every percentage quoted must equal the row it describes. Both were confirmed to fail when a citation or value is perturbed. Remaining review items: - Test exemptions now key on the declared gap set rather than "any blank value", so a stray blank cannot skip the regimen, estimate-coverage, or trial-format checks. - cancer_ici_response_df rebuilt the full merge on every call, and the resolver calls it once per lookup. The frame is cached behind _register_derived_cache and public callers still get a defensive copy: 15.20 ms -> 0.24 ms per call, resolve_ici_response_source 16.71 ms -> 6.61 ms, full suite 132 s -> 119 s. Claude-Session: https://claude.ai/code/session_01UAsZy5ieKeAVtePA6nnGHr
The declared-gap check added in the previous commit keyed off the estimates join (response_metric.isna()) rather than off the blank value. Every curated cell has a matching primary estimate, so clearing an orr_pct on an existing row still joined cleanly, the guard never fired, and _gap_rows() promoted it anyway because it only tested orr_pct.isna(). Verified: blanking LUAD's ORR produced inheritance_kind="direct_missing", has_ici_response_source=True and missing_reason=None -- a fabricated audited gap, exactly what the check was meant to prevent. It now raises. _gap_rows() filters on the declared set too, so only a reviewed code can resolve as a gap whatever produced the frame. Contract: - response_anchor_evidence_df takes gap_overrides (the reason mapping) rather than a bare code set; _apply_gap_evidence read the module-level ICI dict, so a code passed by another caller raised KeyError and the shared helper could never serve apd1. It defaults to no gaps, keeping the aPD-1 table strict. - Gap rows carried response_value_matches_anchor=False, asserting a mismatch that was never evaluated; it is now NA (the column widens to nullable boolean) alongside the other not-applicable fields. - cancer_ici_response_df and ici_response_anchor_df now document the blank rows, as cancer_tmb_df already did for its own. - _gap_rows carries the read-only note its sibling _regimen_maps has. Curation and docs: - The STAD_MSI note's closing clause described the pre-fix behaviour in the present tense, reading as if the row still inherits an ORR. Reworded. - docs/api.md cited ici_response_record(...), which does not exist (the real name is cancer_ici_response_record); fixed here and at the pre-existing occurrence above it. The gap list is now a table carrying each reason and its replacement code, RCC's entry no longer repeats the corrected "anchors differ" claim, and STAD_MSI is marked as the one gap with no replacement. - The "distinguishable at every surface" claim was too strong: the per-regimen views return an empty mapping, since a gap names no regimen. Stated instead. apd1: cancer_apd1_response_df ran the same uncached shared join, once per code inside the include_inherited loops. Cached like the ICI frame: 0.11 ms per call, full suite 119 s -> 81 s. Tests: the blank-value test now covers the realistic slip (clearing a value on a row that keeps its estimate) rather than only the shape the old guard caught, and a new test asserts all four resolution paths agree for every declared gap, since the guard is repeated across them. This commit also restores the data files byte-exactly from main and re-appends only the new rows. cancer-ici-response.csv has mixed CRLF/LF endings and my earlier full-file csv.writer and text-mode rewrites normalized all of them, inflating the diff to 81/76 for 5 new rows and touching unrelated NSCLC/NET/NEC rows in blame. The data diff is now 5 and 1 pure additions. Claude-Session: https://claude.ai/code/session_01UAsZy5ieKeAVtePA6nnGHr
Schema: response_value_matches_anchor was widened to nullable boolean only when a gap row existed, so the shared helper emitted two schemas (ICI boolean, aPD-1 bool) and .astype(bool) raised on one of them. It is now cast unconditionally, so both tables agree and the NA on gap rows means "never evaluated" rather than the False that asserted a mismatch nobody measured. Resolution: - With inherit=False an uncurated code was labelled "direct" — the docstring this branch's own rewrite added says "missing" is the unreviewed case, and apd1 already reported "missing" here. Fixed. - cancer_ici_response_record(include_inherited=True) had started emitting the five gap codes with orr_pct=None while cancer_ici_response(include_inherited= True) excluded them, so the paired accessors no longer had the same key set and float(rec["orr_pct"]) broke on five codes. Bulk maps now exclude gaps, matching tmb.cancer_tmb_record, whose bulk map is direct source rows only. - _gap_record injected available_regimens and has_ici_response_source into the record surface, where they are not part of the contract, so a gap record had two extra keys. It now takes resolver= and only the resolver gets them. Validation: gap rows are now required to leave drug/trial_name/trial_alias/ trial_nct/setting blank (the derived-field reset only cleared computed columns, so a leftover trial would have reached the public record for a code asserting no trial describes it), and an override missing source_scope or missing_reason raises a ValueError instead of a bare KeyError from inside the frame builder. CLI: `oncoref ici CRC` printed the same generic "No ICI ORR" as an uncurated code, so the reason never reached the surface that most needs it. It now prints the missing_reason and the curated note. Curation: the STAD_MSI note said the code "reported no TMB" before these gaps. It did not — it inherited STAD's pooled 5.0 mut/Mb, exactly as it inherited the ORR. Corrected in the note and the mirroring comment. Docs: the new H3 was inserted mid-section and re-parented the ci_basis and value_basis documentation under "Audited response gaps"; moved to the end of the ICI section. The SARC row said the curated span was 0-25% when SARC_KS is 62.1%. The distinguishability claim now says which surfaces it holds for. Tests: the citation check only required a PMID to exist somewhere in the table, so attributing one trial's PMID to another code would have passed; it now requires each citation to belong to a code the note itself names, and was confirmed to fail when KEYNOTE-177's PMID is swapped for cSCC's. Perf: the dropna'd value view is cached alongside the join in both modules. Claude-Session: https://claude.ai/code/session_01UAsZy5ieKeAVtePA6nnGHr
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This closes evidence-provenance gaps where lookups returned a confidently wrong inherited value or could not distinguish reviewed missing evidence from an uncurated code.
1.
STAD_MSIno longer inherits the pooled stomach TMB mediancancer_tmb("STAD_MSI")previously inherited 5.0 mut/Mb fromSTAD. That Chalmers value is the stomach intestinal-type panel median and does not represent MSI-H gastric disease. TCGA-STAD separated 74 hypermutated tumors from 215 tumors below 11.4 mut/Mb, none of which were MSI-positive, but neither source publishes an MSI-stratified gastric median.STAD_MSIis therefore an audited TMB gap withmissing_reason="no_supported_subtype_median".CRC_MSIandUCEC_MSI, which do have published subtype estimates, are unchanged.2. ICI can now represent audited gaps
Blank-ORR rows were previously dropped by value maps and rejected by the evidence join, so reviewed missing evidence was indistinguishable from an uncurated code. The ICI layer now mirrors the TMB contract:
inheritance_kind="direct_missing"source_scopeandmissing_reasonmissingwith no sourceThe audited aggregate gaps are
CRC,RCC,BRCA, andSARC; each directs callers to subtype or histology anchors instead of inventing an umbrella ORR.3.
STAD_MSIhas a real ICI subtype anchorThe final source audit found that KEYNOTE-059 itself reports an MSI-high gastric/GEJ subgroup ORR of 57.1% (4/7; 95% CI 18.4–90.1; PMID:29543932). Treating
STAD_MSIas an ICI gap was therefore incorrect.Both the regimen-aware ICI table and the anti-PD-1 compatibility table now carry the direct 57.1% subtype anchor, backed by a primary estimate row and source-locator audit. This prevents both public response accessors from inheriting the 12% all-comer
STADORR. The TMB gap remains because the response result does not supply a subtype TMB median.Hardening and review follow-ups
response_value_matches_anchornow has one schema on ICI and aPD-1 tables; audited gaps useNAbecause no comparison was performedValueErrorNonefor both reviewed and uncurated missesThe four legacy resolution walks remain a worthwhile separate refactor; exhaustive cross-surface tests guard their behavior here.
Validation
1198 passedruff check oncoref testsruff format --check oncoref testsmainremains pure additions: 5 compact ICI rows, 1 TMB row, and 1 supporting row each in the aPD-1, ICI estimate, and source-audit tablesAlso adds
tmp/to.gitignorefor local scratch downloads.