Skip to content

Commit c516b6d

Browse files
lukedegruchyclaude
lukedegruchy
and
claude
authored
Fix multi-component stratifier alignment for list + scalar (#1042)
* Fix multi-component stratifier alignment for list + scalar (CDO-715) A boolean (subject) basis stratifier with two components — one returning a per-patient List and one returning a per-patient scalar — emitted one stratum per component value with a single component, instead of one stratum per unique (list-element, scalar) tuple with both components. Root cause: collectFunctionRowKeys only collected alignment row keys from function (Map) results, so a scalar component sharing a subject with an iterable component fell back to a subject-only row key. The iterable's composite per-element keys and the scalar's subject-only key never intersected, producing disjoint single-component strata. Generalize the collector (renamed collectAlignmentRowKeys) to also gather row keys from iterable results, so scalars expand alongside list elements via the existing expandScalarToAlignmentRowKeys path. Tighten the scalar fallback: when the stratifier has multi-value components but a given subject produced none (empty Map or List), the subject contributes no stratum — matching the existing empty-list semantic. Adds single-subject and multi-subject MeasureStratifierTest cases plus a CohortBooleanMultiComponentListScalarStrat fixture that pairs the existing "Distinct Encounter Statuses" list expression with the "Gender Stratification String" scalar. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Reduce cognitive complexity of collectAlignmentRowKeys Extract per-entry work into a helper method to flatten nesting and remove duplicated qualifiedSubject/computeIfAbsent setup across the Map and Iterable branches. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 6047a9b commit c516b6d

3 files changed

Lines changed: 290 additions & 48 deletions

File tree

‎cqf-fhir-cr/src/main/java/org/opencds/cqf/fhir/cr/measure/common/MeasureMultiSubjectEvaluator.java‎

Lines changed: 80 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -412,10 +412,11 @@ private static List<StratumDef> buildValueOrNonSubjectValueStrata(
412412
* to match function row keys when mixed with function components.</li>
413413
* </ul>
414414
*
415-
* <h3>Mixed Function and Scalar Components</h3>
416-
* <p>When a stratifier has both function components (per-resource) and scalar components (per-subject),
417-
* the scalar values are expanded to match the function row keys. This ensures all components align
418-
* for proper grouping.
415+
* <h3>Mixed Multi-Value and Scalar Components</h3>
416+
* <p>When a stratifier has both multi-value components (function results per-resource OR iterable
417+
* results per-subject) and scalar components (per-subject), the scalar values are expanded to match
418+
* the multi-value row keys. This ensures all components align for proper grouping so that each
419+
* resulting stratum contains a value from every declared component.
419420
*
420421
* <h4>Example: Stratifier with 3 components</h4>
421422
* <ul>
@@ -466,12 +467,14 @@ private static Table<StratifierRowKey, StratumValueWrapper, StratifierComponentD
466467
final Table<StratifierRowKey, StratumValueWrapper, StratifierComponentDef> subjectResultTable =
467468
HashBasedTable.create();
468469

469-
// First pass: Collect all composite row keys (subject|resource) from function components
470-
// These are needed to expand scalar components to match function row keys
471-
final Map<String, Set<StratifierRowKey>> functionRowKeysBySubject = collectFunctionRowKeys(componentDefs);
470+
// First pass: Collect alignment row keys from multi-value components (function Maps and
471+
// iterable Lists). These are needed to expand scalar components so every alignment row
472+
// carries a value from every declared component, producing one stratum per alignment row
473+
// rather than one stratum per (subject, component) pair.
474+
final Map<String, Set<StratifierRowKey>> alignmentRowKeysBySubject = collectAlignmentRowKeys(componentDefs);
472475

473476
for (StratifierComponentDef componentDef : componentDefs) {
474-
for (StratumTableRow stratumTableRow : mapToListOfTableEntries(componentDef, functionRowKeysBySubject)) {
477+
for (StratumTableRow stratumTableRow : mapToListOfTableEntries(componentDef, alignmentRowKeysBySubject)) {
475478
subjectResultTable.put(
476479
stratumTableRow.stratifierRowKey(), stratumTableRow.stratumValueWrapper(), componentDef);
477480
}
@@ -481,54 +484,75 @@ private static Table<StratifierRowKey, StratumValueWrapper, StratifierComponentD
481484
}
482485

483486
/**
484-
* Collects all composite row keys (subject|resource) from function components.
487+
* Collects the row keys produced by multi-value components, keyed by subject. These are used
488+
* to expand scalar components so every alignment row carries a value from every declared
489+
* component.
485490
*
486-
* <p>This is used to expand scalar components to match the function row keys when
487-
* stratifiers mix function and scalar components.
491+
* <p>Two kinds of multi-value components contribute keys:
492+
* <ul>
493+
* <li><b>Function (Map) results</b> — composite {@code (subject | functionInput)} row keys,
494+
* one per input resource.</li>
495+
* <li><b>Iterable (List) results</b> — composite {@code (subject | iterableElement(value, index))}
496+
* row keys, one per list element. Aligning scalars to these keys is what allows a stratifier
497+
* declared with one iterable component and one scalar component to emit strata that contain
498+
* both component values, instead of one stratum per component value.</li>
499+
* </ul>
488500
*
489-
* @return Map from subject (e.g., "Patient/123") to set of composite row keys
501+
* @return Map from subject (e.g., "Patient/123") to set of alignment row keys
490502
*/
491-
private static Map<String, Set<StratifierRowKey>> collectFunctionRowKeys(
503+
private static Map<String, Set<StratifierRowKey>> collectAlignmentRowKeys(
492504
List<StratifierComponentDef> componentDefs) {
493505

494-
final Map<String, Set<StratifierRowKey>> functionRowKeysBySubject = new HashMap<>();
506+
final Map<String, Set<StratifierRowKey>> alignmentRowKeysBySubject = new HashMap<>();
495507

496508
for (StratifierComponentDef componentDef : componentDefs) {
497509
for (var entry : componentDef.getResults().entrySet()) {
498-
String subjectId = entry.getKey();
499-
CriteriaResult result = entry.getValue();
500-
Object rawValue = result == null ? null : result.rawValue();
501-
502-
// Only process function results (Map values)
503-
if (rawValue instanceof Map<?, ?> functionResults) {
504-
String qualifiedSubject = FhirResourceUtils.addPatientQualifier(subjectId);
505-
Set<StratifierRowKey> rowKeys =
506-
functionRowKeysBySubject.computeIfAbsent(qualifiedSubject, k -> new HashSet<>());
507-
508-
for (Object key : functionResults.keySet()) {
509-
rowKeys.add(
510-
StratifierRowKey.withInput(qualifiedSubject, StratifierRowValue.ofFunctionInput(key)));
511-
}
512-
}
510+
collectAlignmentRowKeysForEntry(entry.getKey(), entry.getValue(), alignmentRowKeysBySubject);
513511
}
514512
}
515513

516-
return functionRowKeysBySubject;
514+
return alignmentRowKeysBySubject;
515+
}
516+
517+
private static void collectAlignmentRowKeysForEntry(
518+
String subjectId, CriteriaResult result, Map<String, Set<StratifierRowKey>> alignmentRowKeysBySubject) {
519+
520+
final Object rawValue = result == null ? null : result.rawValue();
521+
if (!(rawValue instanceof Map<?, ?>) && !(rawValue instanceof Iterable<?>)) {
522+
return;
523+
}
524+
525+
final String qualifiedSubject = FhirResourceUtils.addPatientQualifier(subjectId);
526+
final Set<StratifierRowKey> rowKeys =
527+
alignmentRowKeysBySubject.computeIfAbsent(qualifiedSubject, k -> new HashSet<>());
528+
529+
if (rawValue instanceof Map<?, ?> functionResults) {
530+
for (Object key : functionResults.keySet()) {
531+
rowKeys.add(StratifierRowKey.withInput(qualifiedSubject, StratifierRowValue.ofFunctionInput(key)));
532+
}
533+
} else {
534+
int index = 0;
535+
for (Object value : (Iterable<?>) rawValue) {
536+
rowKeys.add(StratifierRowKey.withInput(
537+
qualifiedSubject, StratifierRowValue.ofIterableElement(value, index)));
538+
index++;
539+
}
540+
}
517541
}
518542

519543
private static List<StratumTableRow> mapToListOfTableEntries(
520-
StratifierComponentDef componentDef, Map<String, Set<StratifierRowKey>> functionRowKeysBySubject) {
544+
StratifierComponentDef componentDef, Map<String, Set<StratifierRowKey>> alignmentRowKeysBySubject) {
521545

522546
return componentDef.getResults().entrySet().stream()
523-
.map(entry -> mapToListOfTableEntries(entry.getKey(), entry.getValue(), functionRowKeysBySubject))
547+
.map(entry -> mapToListOfTableEntries(entry.getKey(), entry.getValue(), alignmentRowKeysBySubject))
524548
.flatMap(Collection::stream)
525549
.toList();
526550
}
527551

528552
private record StratumTableRow(StratifierRowKey stratifierRowKey, StratumValueWrapper stratumValueWrapper) {}
529553

530554
private static List<StratumTableRow> mapToListOfTableEntries(
531-
String subjectId, CriteriaResult result, Map<String, Set<StratifierRowKey>> functionRowKeysBySubject) {
555+
String subjectId, CriteriaResult result, Map<String, Set<StratifierRowKey>> alignmentRowKeysBySubject) {
532556

533557
final String qualifiedSubject = FhirResourceUtils.addPatientQualifier(subjectId);
534558
final Object rawValue = result == null ? null : result.rawValue();
@@ -540,33 +564,41 @@ private static List<StratumTableRow> mapToListOfTableEntries(
540564
return addIterableValueRows(qualifiedSubject, iterableValue);
541565
}
542566

543-
// Scalar value: check if we need to expand to match function row keys
544-
Set<StratifierRowKey> functionRowKeys = functionRowKeysBySubject.get(qualifiedSubject);
545-
if (CollectionUtils.isNotEmpty(functionRowKeys)) {
546-
// Expand scalar to match function row keys for this subject
547-
return expandScalarToMatchFunctionRowKeys(functionRowKeys, rawValue);
567+
// Scalar value: expand to match the alignment row keys produced by any multi-value
568+
// components (functions or iterables) on this subject, so the scalar lands in every stratum.
569+
Set<StratifierRowKey> alignmentRowKeys = alignmentRowKeysBySubject.get(qualifiedSubject);
570+
if (CollectionUtils.isNotEmpty(alignmentRowKeys)) {
571+
return expandScalarToAlignmentRowKeys(alignmentRowKeys, rawValue);
548572
}
549573

550-
// No function row keys - use simple subject-only row key
574+
// If the stratifier as a whole has multi-value components but this particular subject
575+
// produced no alignment rows (empty Map / empty List), the subject contributes no
576+
// stratum at all — same semantic as the single-component empty-list case. Otherwise
577+
// (purely scalar stratifier), fall back to a subject-only row key.
578+
if (!alignmentRowKeysBySubject.isEmpty()) {
579+
return List.of();
580+
}
551581
return List.of(addScalarValueRow(qualifiedSubject, rawValue));
552582
}
553583

554584
/**
555-
* Expands a scalar value to match the row keys from function components.
585+
* Expands a scalar value to match the alignment row keys produced by multi-value components
586+
* (function Maps or iterable Lists).
556587
*
557-
* <p>When stratifiers mix function and scalar components, the scalar value applies
558-
* to all resources for that subject. This method creates one row per function row key,
559-
* all with the same scalar value.
588+
* <p>When stratifiers mix multi-value and scalar components, the scalar value applies to every
589+
* alignment row for that subject. Emitting one table row per alignment key, all with the same
590+
* scalar value, lets the downstream group-by-value-set step produce strata where each stratum
591+
* contains a value from every declared component.
560592
*
561-
* @param functionRowKeys the row keys from function components for this subject
593+
* @param alignmentRowKeys the row keys from multi-value components for this subject
562594
* @param scalarValue the scalar value to expand
563-
* @return list of table rows, one per function row key
595+
* @return list of table rows, one per alignment row key
564596
*/
565-
private static List<StratumTableRow> expandScalarToMatchFunctionRowKeys(
566-
Set<StratifierRowKey> functionRowKeys, Object scalarValue) {
597+
private static List<StratumTableRow> expandScalarToAlignmentRowKeys(
598+
Set<StratifierRowKey> alignmentRowKeys, Object scalarValue) {
567599

568600
StratumValueWrapper valueWrapper = new StratumValueWrapper(scalarValue);
569-
return functionRowKeys.stream()
601+
return alignmentRowKeys.stream()
570602
.map(rowKey -> new StratumTableRow(rowKey, valueWrapper))
571603
.toList();
572604
}

‎cqf-fhir-cr/src/test/java/org/opencds/cqf/fhir/cr/measure/r4/MeasureStratifierTest.java‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1505,4 +1505,139 @@ void cohortBooleanValueStratListExpressionInvalid() {
15051505
.hasContainedOperationOutcomeMsg(
15061506
"value stratifier is invalid for expression: [All Encounters] with result types: [Encounter] for measure URL: http://example.com/Measure/CohortBooleanValueStratListExpression. Expected a scalar type");
15071507
}
1508+
1509+
/**
1510+
* boolean (subject) basis stratifier with two components — one returning a
1511+
* per-patient {@code List} ("Distinct Encounter Statuses") and one returning a per-patient
1512+
* scalar ("Gender Stratification String") — must emit one stratum per unique
1513+
* (list-element, scalar) tuple, with each stratum carrying BOTH component values.
1514+
* <p/>
1515+
* Before the fix, scalar components were only expanded to align with function (Map) row keys,
1516+
* never iterable row keys, so the iterable's composite row key and the scalar's subject-only
1517+
* row key landed in disjoint groups — producing one stratum per component value with only that
1518+
* single component, instead of strata with both components.
1519+
* <p/>
1520+
* Patient-9 has two Encounters (statuses {@code finished}, {@code in-progress}) and gender
1521+
* {@code male}, so two strata are expected:
1522+
* <ul>
1523+
* <li>{Encounter Status: finished, Gender: male} — count 1</li>
1524+
* <li>{Encounter Status: in-progress, Gender: male} — count 1</li>
1525+
* </ul>
1526+
*/
1527+
@Test
1528+
void cohortBooleanMultiComponentListScalarStratPatient9() {
1529+
GIVEN_MEASURE_STRATIFIER_TEST
1530+
.when()
1531+
.measureId("CohortBooleanMultiComponentListScalarStrat")
1532+
.subject("Patient/patient-9")
1533+
.evaluate()
1534+
.then()
1535+
.firstGroup()
1536+
.firstStratifier()
1537+
.hasCodeText("Encounter Status and Gender")
1538+
.hasStratumCount(2)
1539+
.stratum(s -> s.getStratum().stream()
1540+
.filter(st -> st.getComponent().stream()
1541+
.anyMatch(c -> c.hasValue()
1542+
&& "finished".equals(c.getValue().getText())))
1543+
.findFirst()
1544+
.orElse(null))
1545+
.hasComponentStratifierCount(2)
1546+
.stratumComponentWithCodeText("Encounter Status")
1547+
.hasValueText("finished")
1548+
.up()
1549+
.stratumComponentWithCodeText("Gender")
1550+
.hasValueText("male")
1551+
.up()
1552+
.firstPopulation()
1553+
.hasCount(1)
1554+
.up()
1555+
.up()
1556+
.stratum(s -> s.getStratum().stream()
1557+
.filter(st -> st.getComponent().stream()
1558+
.anyMatch(c -> c.hasValue()
1559+
&& "in-progress".equals(c.getValue().getText())))
1560+
.findFirst()
1561+
.orElse(null))
1562+
.hasComponentStratifierCount(2)
1563+
.stratumComponentWithCodeText("Encounter Status")
1564+
.hasValueText("in-progress")
1565+
.up()
1566+
.stratumComponentWithCodeText("Gender")
1567+
.hasValueText("male")
1568+
.up()
1569+
.firstPopulation()
1570+
.hasCount(1);
1571+
}
1572+
1573+
/**
1574+
* multi-subject variant of {@link #cohortBooleanMultiComponentListScalarStratPatient9()}.
1575+
* Across all patients (data per {@code cohortBooleanValueStratMultiValueListOverlapping}),
1576+
* the stratifier must produce one stratum per unique (encounter-status, gender) tuple, each
1577+
* with two components. The expected tuples are:
1578+
* <pre>
1579+
* (in-progress, female) → patient-0
1580+
* (in-progress, male) → patient-1, patient-9
1581+
* (finished, female) → patient-0, patient-8
1582+
* (finished, male) → patient-1, patient-9
1583+
* (arrived, female) → patient-2
1584+
* (arrived, male) → patient-3
1585+
* (triaged, female) → patient-4
1586+
* (triaged, male) → patient-5
1587+
* (cancelled, female) → patient-6
1588+
* (cancelled, male) → patient-7
1589+
* </pre>
1590+
*/
1591+
@Test
1592+
void cohortBooleanMultiComponentListScalarStratAllPatients() {
1593+
GIVEN_MEASURE_STRATIFIER_TEST
1594+
.when()
1595+
.measureId("CohortBooleanMultiComponentListScalarStrat")
1596+
.evaluate()
1597+
.then()
1598+
.firstGroup()
1599+
.firstStratifier()
1600+
.hasCodeText("Encounter Status and Gender")
1601+
.hasStratumCount(10)
1602+
.stratum(s -> s.getStratum().stream()
1603+
.filter(st -> st.getComponent().stream()
1604+
.anyMatch(c -> c.hasValue()
1605+
&& "finished".equals(c.getValue().getText())))
1606+
.filter(st -> st.getComponent().stream()
1607+
.anyMatch(c -> c.hasValue()
1608+
&& "male".equals(c.getValue().getText())))
1609+
.findFirst()
1610+
.orElse(null))
1611+
.hasComponentStratifierCount(2)
1612+
.firstPopulation()
1613+
.hasCount(2) // patient-1, patient-9
1614+
.up()
1615+
.up()
1616+
.stratum(s -> s.getStratum().stream()
1617+
.filter(st -> st.getComponent().stream()
1618+
.anyMatch(c -> c.hasValue()
1619+
&& "finished".equals(c.getValue().getText())))
1620+
.filter(st -> st.getComponent().stream()
1621+
.anyMatch(c -> c.hasValue()
1622+
&& "female".equals(c.getValue().getText())))
1623+
.findFirst()
1624+
.orElse(null))
1625+
.hasComponentStratifierCount(2)
1626+
.firstPopulation()
1627+
.hasCount(2) // patient-0, patient-8
1628+
.up()
1629+
.up()
1630+
.stratum(s -> s.getStratum().stream()
1631+
.filter(st -> st.getComponent().stream()
1632+
.anyMatch(c -> c.hasValue()
1633+
&& "cancelled".equals(c.getValue().getText())))
1634+
.filter(st -> st.getComponent().stream()
1635+
.anyMatch(c -> c.hasValue()
1636+
&& "male".equals(c.getValue().getText())))
1637+
.findFirst()
1638+
.orElse(null))
1639+
.hasComponentStratifierCount(2)
1640+
.firstPopulation()
1641+
.hasCount(1); // patient-7
1642+
}
15081643
}

0 commit comments

Comments
 (0)