Repository navigation
Verify cohort result provenance and correct fusion protein reports - #599
Conversation
iskandr
left a comment
There was a problem hiding this comment.
First review pass on d9642c7:
- Confirmed the new receipt checks cover stale small-variant and additional-task results, and the fusion offset/noncoding-acceptor regressions exercise the previously reported failures.
- Found a cross-library artifact collision: event_id alone names fusion plots and FASTAs. The existing Sid T1/T2 inventory repeats FOXO3--STRADA-CCDC47 and TPST1--CRCP, so later tasks can overwrite earlier figures before PDF construction. Filed #600; fix within this PR.
- Found reference-annotation identity loss: report lookup and FASTA headers key only on transcript ID although FusionReference identity includes annotation. A valid two-annotation fixture renders AAGPFE against the wrong donor residues and drops one reference FASTA. Filed #601; fix within this PR.
- API/CLI/docs review: reject additional tasks without sample_id and reject supplied-fusion sample provenance inconsistent with the cohort source; these are included in the initial fix. Documented legacy additional-result receipts explicitly rather than silently trusting files by name.
Full tests, visual verification and a second review of the final changes remain required.
iskandr
left a comment
There was a problem hiding this comment.
Second review completed on 23ee6e3. No remaining blocking findings in the reviewed diff.
Checked the Python/CLI contract against the documentation: changed sample/library/input/reference/settings identities fail before replacing the saved manifest or summary; cross-sample source_ids selections fail; moved/offline small-variant inputs and explicitly allowed presentation edits remain renderable. Original specimen IDs can be mapped explicitly with input_sample_id and remain unchanged in RNA witnesses. Legacy additional outputs without receipts are rejected with a documented recomputation path.
Checked fusion rendering with missing, noncoding and ambiguous acceptors; partial codons and inserted intervals on both strands; repeated event IDs across libraries; shared transcript IDs across annotations; split acceptor codons and changed downstream residues after a known frame shift. Task-specific artifacts and full recorded input tables address #600 and #602. The unavailable-source rendering path and heading pagination are covered.
Validation: the initial full suite passed 6,551 tests. Later focused contract/CLI/report suites passed (194 tests before the final additions, then 127 cohort/report tests, 105 contract/report tests, and 91 final rendering tests). Eight synthetic regression PDF pages were inspected, including the shifted-frame view. The original six HCC1395/Sid fusion reconstructions were rerun: all six scientific result objects are identical to their prior recorded results. This change corrects attribution/display rather than changing those inferred sequences.
All six tracked findings (#596, #597, #598, #600, #601, #602) are addressed in this PR. Final-head CI and the clean-master deployment gates remain required before release.
iskandr
left a comment
There was a problem hiding this comment.
Final-head review addendum for 1c43ef2: reviewed and visually verified the final nonnegative coverage-axis correction. The complete local ./test.sh run now passes all 6,572 tests, and ./lint.sh passes. The six warnings are the existing namedtuple test-helper collection warning, once per worker. No remaining blocking findings in the reviewed changes. Python 3.14 CI is green; the remaining CI jobs are still required before merge.
|
Released as Isovar 1.62.1 from merged commit e6828ae.
Compatibility remains as documented: additional-analysis outputs from 1.62.0 without provenance receipts must be recomputed into a new output directory. Existing matching small-variant receipts remain renderable. Post-release triage: #589 is the next contained foundational change because occurrence-scoped SV support unblocks downstream Vaxrank admission. Keep #595 (complete matched-normal scope) and #100 (paired patient-background peptides) open as separate evidence/model contracts, along with the broader cohort work in #590/#591. |
Cohort reports could attach old RNA results to a changed sample/library or analysis configuration. Fusion views could also crash without a coding acceptor model, shift parental residues, overwrite another library's figures, or select a reference from the wrong annotation.
The fix validates recorded source/task/input identities and artifact digests before rendering. Additional task families now record receipts, including referenced SV artifacts. An explicit input_sample_id preserves specimen IDs such as Sid T1/T2 within a patient-level report. Changed declarations, cross-sample display selections and corrupted results are rejected.
Fusion comparisons retain unavailable/ambiguous acceptor states, use the translation start and both junction endpoints, preserve annotation identity, and keep artifacts in task-specific directories. Split codons and frame-shifted tails receive consistent reference/color treatment. Report input tables include all recorded inputs, unavailable analyses render correctly, and provenance headings stay with their content.
Closes #596. Closes #597. Closes #598. Closes #600. Closes #601. Closes #602.
Validation: ./lint.sh and ./test.sh pass on the final head (6,572 tests). Contract, CLI and rendering regressions cover the reported failures. Eight synthetic regression PDF pages and the real HCC1395 fusion pages were visually inspected. All six HCC1395/Sid fusion result objects are identical to the prior scientific exports, with original witness specimen IDs preserved. The PR received repeated correctness/API/CLI/documentation reviews, with every finding addressed. Final-head CI remains the merge gate, followed by clean-master deployment gates.
Compatibility: existing small-variant receipts remain renderable when their declarations and digests match. Additional outputs from 1.62.0 that lack receipts must be recomputed into a new output directory; the renderer will not manufacture provenance for them. CLI help and docs describe the same Python API contract. Version 1.62.1.