Non-subject value stratifier — scalar and list expression support (#909)#1030
Merged
lukedegruchy merged 12 commits intoMay 25, 2026
Merged
Conversation
…basis CV measure When a CONTINUOUSVARIABLE Encounter-basis measure uses a Patient-context scalar expression as its stratifier component criteria (e.g. `define "Age": AgeInYearsAt(end of "Measurement Period")`) instead of a CQL function taking the population resource, the engine routes through the scalar-fallback NON_SUBJECT_VALUE path. Group-level aggregation is correct, but stratum measureScore is never computed and the stratum-level measure-observation population is missing both the cqfm-aggregateMethod and the extension-aggregationMethodResult extensions. Adds a self-contained dataset (10 patients, 12 encounters - identical to the ContinuousVariableObservationEncounterBasis fixtures) and a failing test mirroring continuousVariableResourceMeasureObservationEncounterBasisAvg, where the only meaningful difference is the stratifier expression form. No production fix in this commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Formatting check succeeded! |
MeasureMultiSubjectEvaluator.getResourcesForSubjects previously ran normalizePopulationKey on MEASUREOBSERVATION subjectResources, but those entries are Set<Map<input, output>> — yielding String.valueOf(map) garbage instead of real resource IDs. Scalar non-subject stratifiers on continuous-variable measures ended up with bogus resourceIdsForSubjectList, no intersection with the Map keys in downstream stratum scoring, and a null measureScore per stratum. The new MEASUREOBSERVATION branch flattens Map.keySet() through normalizePopulationKey, mirroring getPopulationResourceKeySet:929-938. The regression test added in b0155b4 now passes. Also drops a stray logger.info("1234: ...") debug statement from the same method. Test coverage for the four #909 scenarios: - Case 1 (non-subject + function): already covered. - Case 2 (non-subject + scalar): CV regression test now green; hardens MeasureStratifierTest.ratioResourceValueStratAge with per-stratum score assertions (was stratum-count-only). - Case 3 (subject + function = error): new MeasureStratifierTest.cohortBooleanValueStratFunctionStratifierInvalid plus a CohortBooleanValueStratFunctionStratifier.json fixture pin the InvalidRequestException from FunctionEvaluationHandler.validateStratifierExpressionTypes. - Case 4 (subject + scalar): already covered. Adds direct unit coverage of all four cases against MeasureMultiSubjectEvaluator via a new nested Issue909 class in MeasureMultiSubjectEvaluatorTest. The case-2 unit test exercises a MEASUREOBSERVATION population so the fix is pinned at unit level. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
lukedegruchy
changed the title
…ct-stratifier-expression-scalar-failing-test
Extend the #909 quadrants with a fifth case the issue didn't name: a non-subject value stratifier whose component expression evaluates to a List instead of a scalar. Expected semantics: the subject appears in each stratum derived from a list element, with all of that subject's resources counted in each such stratum -- the same scalar fallback used for #909 case 2, fanned out across N strata per subject. Without this fix, MeasureMultiSubjectEvaluator.getResourceIdsForValueStratifier ran the iterable-expansion row-key inputParams (per-element synthetic keys like "value_0_finished") through the intersection-against-population path, where they never match real resource IDs. Every stratum was produced correctly but with an empty resourceIdsForSubjectList, so each stratum's population count was 0. Fix: distinguish "intersectable" inputParams (resource IDs / stringified primitives, from function-input row keys) from "synthetic" inputParams (per-element keys from iterable expansion of non-resource elements) and route the latter to the existing subject-level resource-attribution path. Capture the inputParam slot of StratifierRowKey as a typed value: * Replace Optional<String> inputParamId with Optional<StratifierRowValue> inputParam, where StratifierRowValue is a sealed interface with two records: - Resource(resourceId) -- intersectable; covers function-input row keys (collectFunctionRowKeys, addFunctionResultRows) and iterable-of-resource elements - Scalar(index, value) -- non-intersectable; covers per-element rows from iterable expansion of non-resource elements (including null), and owns the "value_<i>_<v>" / "null_<i>" string format that was previously spread across normalizeValueKey and an ad-hoc prefix check * Add factories ofFunctionInput(Object), ofIterableElement(Object, int), ofResourceId(String) -- these subsume the deleted normalizeResourceKey and normalizeValueKey helpers. * getResourceIdsForValueStratifier now filters by StratifierRowValue::isIntersectable instead of a string-prefix check, so the encode and recognize sides cannot drift apart. Remove dead code on StratifierRowKey exposed by the refactor: fromLegacyString, toLegacyString, and hasInputParam have no remaining call sites. New fixtures and tests: * LibrarySimple.cql: "Distinct Encounter Statuses" patient-context list expression -- realistic stratifier rooted in actual test Encounter.status values. * CohortResourceValueStratListScalar.json -- cohort measure using the list expression as a non-subject value stratifier. * MeasureStratifierTest.cohortResourceValueStratListScalar -- pins the end-to-end behaviour on patient-9 (2 Encounters, finished + in-progress): each stratum counts 2 Encounters. * SubjectResourceKeyTest -- updated to the new StratifierRowKey.withInput(subject, StratifierRowValue.ofResourceId(...)) API. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
lukedegruchy
changed the title
JPercival
approved these changes
May 20, 2026
…n-scalar-failing-test
…ct-stratifier-expression-scalar-failing-test
Sonar flagged the PR at 75.8% new-code coverage. Add 18 tests across four new nested classes to cover the StratifierRowValue refactor and the list-returning non-subject value stratifier path: * ListReturningNonSubjectValueStratifier -- exercises addIterableValueRows and the case-5 scalar-fallback path (Scalar inputParam -> isIntersectable false -> subject-level resource attribution): non-resource list elements, null element, and list-of-resources (intersectable). * MixedComponents -- exercises expandScalarToMatchFunctionRowKeys when a stratifier mixes per-resource function results with a per-subject scalar. * CriteriaStratifiers -- covers buildCriteriaStrata, calculateCriteriaStratifierIntersection, the Map-keyset branch of criteriaResultAsIntersectionSet, and the null-result branch. * StratifierRowValueFactories -- direct unit coverage of every factory and branch in StratifierRowValue: ofFunctionInput (resource w/ ID, no ID, non-resource), ofIterableElement (null, scalar, resource), ofResourceId (happy + null), Resource(null) validation, and Scalar accessors for both null and non-null value. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
lukedegruchy
deleted the
ld-20260511-non-subject-stratifier-expression-scalar-failing-test
branch
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.
This branch closes the remaining gap for issue #909: a stratifier with a non-subject (non-boolean) population basis and a scalar (non-function) component expression must evaluate the expression in subject context and apply that single value to every resource for the subject. The previously failing regression test for the continuous-variable + scalar-stratifier combination now passes, and test coverage is extended so all four (basis × expression-type) quadrants of #909 are exercised at multiple levels.
MeasureMultiSubjectEvaluator.getResourcesForSubjectsforMEASUREOBSERVATIONpopulations: previously the helper rannormalizePopulationKeyon the population'sSet<Map<inputResource, outputValue>>entries, producingString.valueOf(map)strings instead of real resource IDs, which left every continuous-variable stratum with a nullmeasureScorewhen the stratifier was scalar.ContinuousVariableResourceMeasureObservationTest.continuousVariableObservationEncounterBasisNonFunctionStratifierexercising the failing path end-to-end on a continuous-variable Encounter-basis measure.MeasureMultiSubjectEvaluatorvia a newIssue909nested class, including aMEASUREOBSERVATIONpopulation in case 2 so the fix is pinned at the unit level (the test fails without the fix).cohortBooleanValueStratFunctionStratifierInvalid) plus fixture that exercise the existing upstream validator path for case 3 (subject basis + CQL function = error), which had no test before.MeasureStratifierTest.ratioResourceValueStratAgewith per-stratum score assertions (it previously only asserted stratum count, and would have passed even before the fix).