Repository navigation
feat: Agilent OpenLab CDS - report sample type - #1273
Merged
Merged
Conversation
The sequence table's sample type (SampleSetup -> DAParam -> Type) was only mapped for "Blank" and "Sample". "Calibration" and "Control" rows fell through the mapping and were reported with a null sample role type, silently dropping the value. - Map "Calibration" and "Control" to their ASM sample role types. - Report the raw OpenLab value as "sample type" sample custom information, so types with no ASM equivalent are not dropped. Adds Sirius-control fixture, derived from the existing Sirius fixture, to cover the Control type. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
stephenworlow
approved these changes
Sep 28, 2026
nathan-stender
added a commit
that referenced
this pull request
Sep 28, 2026
### Added - Agilent OpenLab CDS - report sample type (#1273)
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
A customer asked to retrieve sample type from OpenLab CDS result sets. It turns out we already read it — but only mapped two of the four values OpenLab emits, so the rest were silently dropped.
SAMPLE_ROLE_TYPEmapped onlyBlankandSample.CalibrationandControlrows fell through and were reported with a nullsample role type. This is visible in the fixtures already onmain: the Luxo expected JSON has 12 nullsample role typevalues (4Calibrationsamples x 3 signals).This PR:
Calibration->calibration_sample_roleandControl->control_sample_role."sample type"entry in the sample custom information document, so a type with no ASM equivalent is surfaced rather than dropped.The customer's real file contains
Sample,Blank,Calibration, andControl— 4 of its 11 samples were losing their type before this change.Expected JSON changes (intentional)
Per the repo convention, existing expected JSON is not edited to make a change pass — this is a deliberate behavior change, regenerated with
--overwrite:"sample role type"newly populated with"calibration sample role"(previouslynull)"sample type"custom info key: 18xCalibration, 15xBlank, 12xSampletesting/utils.py)No existing non-null value changed.
Test fixture
Sirius-control-2023-09-01 07-52-44-04-00.rslt(33 KB) is derived from the existing anonymizedSiriusfixture, not from a customer export — the only change is flipping one sample's<Type>Calibration</Type>to<Type>Control</Type>in the.acaml. 4 of its 5 members are byte-identical to the source fixture. No new customer data is introduced.Verified it is real coverage: removing the
Controlmapping makes only this fixture fail (sample role type removed from dictionary); restoring it passes.Testing
hatch run lintclean (black 798 files, mypy 795 source files)hatch run test_all.py3.10:pytest tests/— 1522 passed (+2 from the new fixture)🤖 Generated with Claude Code