Conversation
…schema Groundwork for #754: CalibrationControlStatus enum (#748) and the CalibrationControl ORM model + calibration_controls table, with disease / controls_not_phi columns on score_calibrations and the ScoreCalibration.controls relationship (#749). Migration parents on the release head; no RT dependency. Validation (#751), PHI gate (#752), view models (#750), and endpoint wiring (#753) are deliberately not included.
10 tasks
CalibrationControlBase/Modify/Create/SavedCalibrationControl in a new view_models/calibration_control.py. SavedCalibrationControl derives variant_urn from the variant relationship (mirrors ScoreCalibrationWithScoreSetUrn) so internal PKs never leak, and carries record_type + audit fields. Extend the calibration view models with disease, controls_not_phi, and controls: write models take CalibrationControlCreate; the saved response returns SavedCalibrationControl (empty list by default). Exclude the controls relationship from the model_dump splat in _create_score_calibration and modify_score_calibration — passing controls=None to the ORM relationship raised "None is not list-like" and 500'd every create/modify. disease and controls_not_phi are real columns and flow through. Controls persistence and the CSV path remain #753.
Coverage Report for CI Build 35166650102Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Warning No base build found for commit Coverage: 88.977%Details
Uncovered Changes
Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
…#751) Add validate_calibration_controls_in_score_set to lib/score_calibrations.py: a batched, score-set-scoped lookup that rejects controls whose variant is unknown or belongs to a different score set (reported together, per the combined-error decision), plus a Counter-based duplicate check that fails with a readable message before the UNIQUE(calibration_id, variant_id) constraint. No-op for None/empty. Wiring the call into create/modify lands with the endpoint work in #753.
Block the private->public transition in publish_score_calibration_route when a calibration has controls but controls_not_phi is not True (None or False both block; only True clears). Zero-control calibrations are unaffected. The check lives in the router to keep HTTP semantics out of the lib layer. Tests attach controls at the ORM level since endpoint persistence is #753.
Add build_calibration_controls (validate via validate_calibration_controls_in_score_set, then resolve URNs to variants and construct CalibrationControl rows) and wire it into create_score_calibration_in_score_set and modify_score_calibration. Modify uses replace semantics: controls=None leaves existing controls untouched, a list (including []) deletes and rebuilds them. validate_calibration_controls_in_score_set now returns the validated variant URNs so build reuses them instead of rebuilding the list. Controls are read via getattr on the update/create models so the existing minimal test mocks (which predate the field) still work. CSV upload, the controls_not_phi reset-on-change, and controlsCount on list responses are the remaining slices.
…ge on modify (#753) On modify, controls_not_phi is no longer set by the generic attribute loop. An explicit value in the request always wins; otherwise replacing the controls invalidates any prior affirmation (reset to None, forcing re-acknowledgment before republishing), while leaving the controls untouched preserves it. This keeps an unrelated edit from silently wiping an affirmation and supports the editor's change-controls-and-affirm-in-one-save flow.
…753) Add validate_and_standardize_calibration_controls_dataframe (mirroring the class-based validator) plus the clinical_status column constant. The controls CSV accepts the same variant_urn / hgvs_nt / hgvs_pro columns as the class-based CSV and a clinical_status column validated against CalibrationControlStatus (case-insensitive), returning CalibrationControlCreate rows so the router feeds inline and CSV controls through one path. Because a control is a single variant while an HGVS can match many (hgvs_pro collapses distinct nucleotide variants in a nucleotide-indexed score set), resolution rejects any identifier that maps to more than one variant in the score set rather than silently choosing one.
…points (#753) Add an optional controls_file CSV upload to the create and modify routes alongside the existing classes_file, documented in the multipart OpenAPI schema and route docstrings. Providing inline controls and a controls_file together returns 422; a controls_file is parsed and validated (URN or HGVS, 1:1) and fed through the same create/modify path as inline controls. GET returns controls; list responses are addressed separately (#753 slice 4). Integration tests cover inline create, CSV create, inline+file conflict, GET, modify replace/clear/omit, and the controls_not_phi re-acknowledgment behavior.
) Add a controls_count correlated-COUNT column_property to ScoreCalibration (mirroring FunctionalClassification.variant_count). Move the full controls list off SavedScoreCalibration onto the detail models (the full ScoreCalibration and a new ScoreCalibrationDetailWithScoreSetUrn), and add controls_count to the base. The single-item calibration endpoints return the detail model (full controls + count); the two list endpoints return count-only. Net: calibration list responses and score-set-embedded SavedScoreCalibration carry controlsCount, while single-calibration detail, score-set full detail, and the public dump keep the full controls list.
Replace the free-text calibration disease column with a MONDO-coded disease_term FK into a new mondo_terms table, served on the wire as a GA4GH MappableConcept. The API input is the bare MONDO code (e.g. "MONDO:0015263"); the server resolves the canonical label and system against EBI OLS4 and get-or-creates the term on save. A null code resolves to the seeded generic "disease or disorder" root (MONDO:0000001), so the FK is never null. Validation against OLS is mandatory: an unknown code is rejected and an OLS outage raises MondoServiceError (503) rather than persisting an unvalidated label. Add a /diseases/search proxy over OLS4's /select endpoint to back the disease typeahead. The migration seeds the generic term and backfills existing calibrations before enforcing NOT NULL. Covered by unit tests that mock OLS, plus a network-marked canary suite exercising the real OLS4 service so upstream shape changes surface.
…on term (#754) With every calibration now carrying a non-null MONDO disease term, build the VA-Spec Condition directly from it instead of the hardcoded generic mappable-concept helper. The clinical-impact proposition takes the authoritative (strongest) calibration and uses its disease as the single objectCondition every evidence line on the statement targets. Removes the now-unused annotation disease constants module.
Controls resolve each variant URN against the score set's own variants, which are minted asynchronously from the scores file after the score set row exists, so none are resolvable at creation time. Reject a non-empty controls list with a 409, mirroring the existing class-based-range guard, rather than silently dropping the controls (the nested-create path calls standalone create_score_calibration, which never builds them).
…blish (#753) Publishing already blocked a calibration from going public with unaffirmed controls, but the modify route never re-checked the invariant afterward. An admin (the only user who can reach this route on an already-public calibration) could add or replace controls without re-affirming, since replacing controls resets the affirmation flag. Apply the same PHI acknowledgment gate on modify that publish already enforces.
find_or_create_mondo_term looked up an existing term by the submitted code but inserted the canonical code returned by OLS. When OLS normalizes the input (case, or an alias/obsolete id), a resubmission missed the lookup and tried to insert the canonical code again, tripping UNIQUE(system, code) with an unhandled 500. Re-check for an existing row by the resolved canonical code before inserting, so lookup and insert key on the same value and the get-or-create stays idempotent.
A published score set's scores are the record other work cites; re-uploading them would silently change results already in circulation and anything derived from the variants they recreate (mappings, calibration bin membership). Restrict SET_SCORES so owners and contributors may only set scores while the set is private; admins keep the ability as an operational escape hatch. Corrections go through a superseding set. The combined update-with-variants endpoint checks the permission before applying the update, since it commits the metadata half before enqueuing variant creation.
…ation placement Expose which of the calibration's own functional classifications a control's variant falls into by bin membership, so the concordance view can cross a control's clinical status against where its score landed. Compute it as a correlated-subquery column_property on CalibrationControl rather than storing it, so it can never drift from the bin assignments it reports. Classified bins never overlap, so at most one applies; a classified bin is preferred over a 'not_specified' one, then lowest id breaks ties, for a deterministic result.
… score re-upload Re-uploading a score set deletes and recreates its variants, breaking the foreign keys calibration controls and class-based bin membership hold into them (RESTRICT, so the delete would otherwise fail). Snapshot the irreproducible references by HGVS identity before the delete, then re-resolve them against the new variants; range-based bins are recomputed from the new scores instead. A dropped control resets controls_not_phi, and provenance (author, date) is carried across rather than reattributed to the re-uploader. Also consolidates control-variant validation and construction onto a single scoped variant query instead of one lookup to validate and another to build.
bencap
force-pushed
the
feature/bencap/754-calibration-controls
branch
from
September 18, 2026 23:30
b89f429 to
0f60989
Compare
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
2 participants
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.
Summary
Adds clinical controls to score calibrations — variants of known pathogenic/benign significance recorded as the ground truth a calibration was derived from — plus MONDO disease context for a calibration and a freeze on a published set's scores.
Clinical controls (#748–#753)
calibration_controlstable +pathogenic/benignstatus enum, with view models.controlsCounton list responses.controls_not_phiresets whenever controls change on modify.Disease context (#754)
Score freezing
SET_SCORESrestricted to private sets for owners/contributors (admin escape hatch retained); enforced before the combined update-with-variants endpoint applies.