feat(qc): manual & RDT QC persistence with validation holds (OGC-1147) - #4031
Merged
Conversation
github-project-automation
Bot
moved this to Todo
in OpenELIS Community PR Priority List
samuelmale
force-pushed
the
feat/qa-next
branch
from
August 13, 2026 11:29
3c424fc to
e077a93
Compare
Source-typed QC store and bench capture: - qc_result gains source (ASTM/MANUAL/RDT), qualitative_outcome, captured target snapshot, lab unit, and control_label; shape enforced by DB CHECK (qualitative rows carry an outcome and no value, quantitative the reverse) - POST /rest/qc/results records bench controls with analyzer-path z-score arithmetic; a failing control raises a MANUAL_FAIL violation and an NCE - Bench QC dashboard tab grouped by lab unit/test; bench lots (no analyzer) supported on MANUFACTURER_FIXED targets Validation hold (FR-C1/C3/C4): - Held rows show an explanatory "QC failed" tag (DefinitionTooltip popover carries the reason - Carbon's Tag discards title props) - qcFailBlocksValidation site flag: when on, release of covered results is withheld fail-closed, re-resolved server-side; enforced on the REST save and both legacy validation endpoints - A blocked save reports the withheld accessions on the response; the UI surfaces them as a persistent warning across the post-save reload QC lot integrity: - Lot-level Levey-Jennings chart reachable from QC Lot Management - Duplicate live lots refused twice: validator 400 naming the conflict, and partial unique index on (lot_number, test_id, control_level) among non-retired lots; existing duplicates retired keeping the row with results - Calculators use sample SD (N-1, CLSI C24) with a small-window guard; established statistics are not rewritten Settles UAT register DEF-1, DEF-2, GAP-3, GAP-5, GAP-6; GAP-4 (empty-state witness on clean data) deferred to post-merge per the register. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Local pre-commit spotless run was silently aborted by a zsh nomatch on the cache-clear glob, so CI's cold check caught four unformatted files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…and stale sequences
New test classes in this branch reshuffled surefire order, landing three
pre-existing pollution classes on the QA suites in CI:
1. Datasets naming type_of_test_result truncate it and re-insert only their
own rows, so later result_limits inserts lose their FK target (id 4).
2. Datasets renaming requester_type ids 1-2 break later
getRequesterTypeByName('organization') lookups.
3. Fixture rows carry explicit ids without advancing <table>_seq, so later
sequence-backed inserts collide (observation_history id 3).
Fix all three at the loader, following the ensureAuditSystemUser precedent:
restore the two reference vocabularies after every dataset load (by id /
by name), and resync every conventionally-named sequence of a dataset-named
table to MAX(id)+1 — correct because those tables were just truncated.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ng; forward-only sequence resync Second CI round surfaced one real OGC-1147 regression and one flaw in the previous commit's test-infra fix: - findDistinctTestInstrumentPairs now excludes bench lots (instrument_id NULL). Both consumers are per-instrument, and a NULL instrument poisoned getUnconfiguredMappings' read-only transaction (analyzer lookup threw, caught, transaction rollback-only) — the Westgard config screen died as soon as one bench lot existed. Regression test added. - The dataset-load sequence resync is now forward-only (GREATEST against the sequence's high-water mark). Pulling high sequences down to a truncated table's MAX+1 created the opposite collision with raw-JDBC literal-id inserts outside dataset loads (dictionary id 8, history id 100009 in CI). - RejectionReportServiceTest heatmap assertions made leftover-tolerant: the heatmap is a window-wide query, so global allMatch/positional assertions are order-fragile; assert the fixture's own null-location delta instead. Full local suite: 4937 tests, 0 errors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ixture loads Third pollution surface, same class: fixtures gut observation_history_type down to ids 1-5, but ObservationHistoryService caches the name->id mapping for SampleRecordStatus/PatientRecordStatus (15/16) at first use, so the analyzer accept path FK-fails on its next status write (CI: reviewerChoice_ removesTheHold). Restore the pair by exact id — the cache makes any other id wrong — and document the two exhaustive-count assertions that now see it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
samuelmale
force-pushed
the
feat/qa-manual-rdt-qc-persistence
branch
from
August 13, 2026 11:45
c28a3c4 to
69cfe39
Compare
…ry (#4058) The QC section of the expanded results row showed a footnote saying manual control capture was unavailable: qc_result could only be written by the analyzer import. OGC-1147 added the write path (POST /rest/qc/results); this adds the UI that uses it, closing the last of OGC-1025's acceptance criteria. The form is polymorphic by test type (FR-D3). A quantitative row records a measured value, a tech-entered expected value and uncertainty, Pass/Fail, and a bench control lot chosen from the lots the API will actually accept. An RDT row records the control-line outcome (Valid/Invalid) against a free-text kit lot, with no lot record and no numeric value — the shape the DB CHECK constraint enforces. Selecting a failing outcome warns that reporting will be blocked before the tech submits, and the confirmation after a failing capture says the same, because the server has by then opened a non-conformity and put a hold on the covered results. Expected value and uncertainty are typed by the tech (FR-D4). Prefilling them from a configured QC target needs OGC-1148, which is unbuilt; the hook is marked in a comment rather than guessed at. Capture is deliberately not gated on the row being editable. A control run is a QC record, not an edit of the patient result, and the hold it raises covers results already sitting at Technical Acceptance — gating on editability would mean a tech who saves the result first could never record the control for that row. Write authority is enforced server-side instead. That server-side gate moves from qa.view.qc to the RESULTS role: recording a control is a results-entry write, so it takes the same authority as the rest of results entry, and a QC-view-only user is now refused. The controller's error bodies become JSON objects so the form can surface the backend's human-readable message verbatim, with a fallback for an exception carrying no message (Map.of would otherwise turn an intended 400 into a 500). Success is keyed on the saved row's id rather than on the response status: the fetch helper reports a transport failure as status 0, so a status-only check would have told the tech a control was recorded when it never reached the server. Verified end to end on the dev stack: a failing manual control stored its z-score against the lot, raised MANUAL_FAIL, auto-opened an NCE and tagged the Validation row; an Invalid RDT control did the same without a numeric value; a passing control produced neither. Both appear under Bench QC, and only the quantitative runs plot on the lot's chart. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
github-project-automation
Bot
moved this from Todo
to Done
in OpenELIS Community PR Priority List
samuelmale
added a commit
that referenced
this pull request
Aug 17, 2026
#4031) * feat(qc): manual & RDT QC persistence with validation holds (OGC-1147) Source-typed QC store and bench capture: - qc_result gains source (ASTM/MANUAL/RDT), qualitative_outcome, captured target snapshot, lab unit, and control_label; shape enforced by DB CHECK (qualitative rows carry an outcome and no value, quantitative the reverse) - POST /rest/qc/results records bench controls with analyzer-path z-score arithmetic; a failing control raises a MANUAL_FAIL violation and an NCE - Bench QC dashboard tab grouped by lab unit/test; bench lots (no analyzer) supported on MANUFACTURER_FIXED targets Validation hold (FR-C1/C3/C4): - Held rows show an explanatory "QC failed" tag (DefinitionTooltip popover carries the reason - Carbon's Tag discards title props) - qcFailBlocksValidation site flag: when on, release of covered results is withheld fail-closed, re-resolved server-side; enforced on the REST save and both legacy validation endpoints - A blocked save reports the withheld accessions on the response; the UI surfaces them as a persistent warning across the post-save reload QC lot integrity: - Lot-level Levey-Jennings chart reachable from QC Lot Management - Duplicate live lots refused twice: validator 400 naming the conflict, and partial unique index on (lot_number, test_id, control_level) among non-retired lots; existing duplicates retired keeping the row with results - Calculators use sample SD (N-1, CLSI C24) with a small-window guard; established statistics are not rewritten Settles UAT register DEF-1, DEF-2, GAP-3, GAP-5, GAP-6; GAP-4 (empty-state witness on clean data) deferred to post-merge per the register. * style: apply spotless formatting (OGC-1147) Local pre-commit spotless run was silently aborted by a zsh nomatch on the cache-clear glob, so CI's cold check caught four unformatted files. * test: make dataset loads self-repairing against reference-seed wipes and stale sequences New test classes in this branch reshuffled surefire order, landing three pre-existing pollution classes on the QA suites in CI: 1. Datasets naming type_of_test_result truncate it and re-insert only their own rows, so later result_limits inserts lose their FK target (id 4). 2. Datasets renaming requester_type ids 1-2 break later getRequesterTypeByName('organization') lookups. 3. Fixture rows carry explicit ids without advancing <table>_seq, so later sequence-backed inserts collide (observation_history id 3). Fix all three at the loader, following the ensureAuditSystemUser precedent: restore the two reference vocabularies after every dataset load (by id / by name), and resync every conventionally-named sequence of a dataset-named table to MAX(id)+1 — correct because those tables were just truncated. * fix(qc): bench lots must not surface in per-instrument Westgard pairing; forward-only sequence resync Second CI round surfaced one real OGC-1147 regression and one flaw in the previous commit's test-infra fix: - findDistinctTestInstrumentPairs now excludes bench lots (instrument_id NULL). Both consumers are per-instrument, and a NULL instrument poisoned getUnconfiguredMappings' read-only transaction (analyzer lookup threw, caught, transaction rollback-only) — the Westgard config screen died as soon as one bench lot existed. Regression test added. - The dataset-load sequence resync is now forward-only (GREATEST against the sequence's high-water mark). Pulling high sequences down to a truncated table's MAX+1 created the opposite collision with raw-JDBC literal-id inserts outside dataset loads (dictionary id 8, history id 100009 in CI). - RejectionReportServiceTest heatmap assertions made leftover-tolerant: the heatmap is a window-wide query, so global allMatch/positional assertions are order-fragile; assert the fixture's own null-location delta instead. Full local suite: 4937 tests, 0 errors. * test: restore the record-status observation_history_type pair after fixture loads Third pollution surface, same class: fixtures gut observation_history_type down to ids 1-5, but ObservationHistoryService caches the name->id mapping for SampleRecordStatus/PatientRecordStatus (15/16) at first use, so the analyzer accept path FK-fails on its next status write (CI: reviewerChoice_ removesTheHold). Restore the pair by exact id — the cache makes any other id wrong — and document the two exhaustive-count assertions that now see it. * feat(OGC-1025): capture manual and RDT control results at results entry (#4058) The QC section of the expanded results row showed a footnote saying manual control capture was unavailable: qc_result could only be written by the analyzer import. OGC-1147 added the write path (POST /rest/qc/results); this adds the UI that uses it, closing the last of OGC-1025's acceptance criteria. The form is polymorphic by test type (FR-D3). A quantitative row records a measured value, a tech-entered expected value and uncertainty, Pass/Fail, and a bench control lot chosen from the lots the API will actually accept. An RDT row records the control-line outcome (Valid/Invalid) against a free-text kit lot, with no lot record and no numeric value — the shape the DB CHECK constraint enforces. Selecting a failing outcome warns that reporting will be blocked before the tech submits, and the confirmation after a failing capture says the same, because the server has by then opened a non-conformity and put a hold on the covered results. Expected value and uncertainty are typed by the tech (FR-D4). Prefilling them from a configured QC target needs OGC-1148, which is unbuilt; the hook is marked in a comment rather than guessed at. Capture is deliberately not gated on the row being editable. A control run is a QC record, not an edit of the patient result, and the hold it raises covers results already sitting at Technical Acceptance — gating on editability would mean a tech who saves the result first could never record the control for that row. Write authority is enforced server-side instead. That server-side gate moves from qa.view.qc to the RESULTS role: recording a control is a results-entry write, so it takes the same authority as the rest of results entry, and a QC-view-only user is now refused. The controller's error bodies become JSON objects so the form can surface the backend's human-readable message verbatim, with a fallback for an exception carrying no message (Map.of would otherwise turn an intended 400 into a 500). Success is keyed on the saved row's id rather than on the response status: the fetch helper reports a transport failure as status 0, so a status-only check would have told the tech a control was recorded when it never reached the server. Verified end to end on the dev stack: a failing manual control stored its z-score against the lot, raised MANUAL_FAIL, auto-opened an NCE and tagged the Validation row; an Invalid RDT control did the same without a numeric value; a passing control produced neither. Both appear under Bench QC, and only the quantitative runs plot on the lot's chart. ---------
6 tasks
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
1 participant
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
Persists bench-recorded QC — manual quantitative controls and RDT control lines — in the same
qc_resultstore the analyzer path uses, and closes the loop into result validation: a failing control visibly holds the patient results it covers, and a lab can opt in to blocking their release outright.Source-typed QC store (D1/D2)
qc_resultgainssource(ASTM/MANUAL/RDT),qualitative_outcome, captured target snapshot (expected_value/uncertainty),test_section_id, andcontrol_label. Every pre-existing row backfills toASTMby column default.chk_qc_result_source_shapeguarantees an RDT row carries an outcome and no number, and quantitative rows always carry a value (FR-A3 — no magic numbers).POST /rest/qc/resultsrecords bench controls with the analyzer path's z-score arithmetic; a failing control raises aMANUAL_FAILviolation (D4) and auto-opens an NCE.MANUFACTURER_FIXEDtargets (D3 — no 20-run establishment for manual methods).Validation hold (FR-C1/C3/C4)
/AccessionValidationcarry an explanatory red "QC failed" tag; the reason renders in aDefinitionTooltippopover (Carbon'sTagsilently discardstitleprops).qcFailBlocksValidationsite flag (default off = warn-only). When on, release of covered results is withheld fail-closed, re-resolved from the DB on save — a client postingqcHold=falsechanges nothing. Enforced on the REST save and both legacy validation endpoints, so there is no sidestep.QC lot integrity
uq_qc_control_lot_activeon(lot_number, test_id, COALESCE(control_level,''))among non-retired lots makes raw SQL fail too. A migration first retires existing duplicates, keeping the row that owns the results.Deviations & follow-ups
POST /rest/qc/resultsis gated byqa.view.qcfor now; a distinct write authority lands with the capture UI (OGC-1025).Test plan
QcHoldServicefail-closed semantics, bench capture contract tests against real Postgres, Westgard suites, bench dashboard summary.🤖 Generated with Claude Code