Skip to content

Commit fc045ad

Browse files
JPercivalclaude
andcommitted
docs: document Aggregate skip as legacy bug with pattern guidance
The Aggregate parity test skip is not a regression — our type inference is more correct. Document the pattern (Any-typed aggregate accumulators losing precision in Coalesce resolution) so future agents can recognize similar cases where the legacy inserts unnecessary casts that our pipeline correctly omits. References PR #1710. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 0edb7b0 commit fc045ad

1 file changed

Lines changed: 21 additions & 5 deletions

File tree

  • Src/java/cql-to-elm/src/jvmTest/kotlin/org/cqframework/cql/cql2elm/codegen

Src/java/cql-to-elm/src/jvmTest/kotlin/org/cqframework/cql/cql2elm/codegen/FullParityTest.kt

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -187,11 +187,27 @@ class FullParityTest {
187187
"Error recovery: legacy replaces invalid sort expressions with Null",
188188
// AggregateOperators: PASSING (M19)
189189
// TypeOperators: PASSING (M19)
190-
// Aggregate: union inside aggregate over-wraps QueryLetRef/List operands
191-
// in As(List<Choice<Interval,List<Interval>>>) when types should match
192-
// Aggregate: Factorial needs As(Integer) on Coalesce result (accumulator type
193-
// inference), RolledOutIntervals over-wraps union with ChoiceType
194-
"Aggregate" to "Aggregate accumulator type inference + union over-wrapping",
190+
// Aggregate: OUR TYPE INFERENCE IS MORE CORRECT than legacy (PR #1710).
191+
//
192+
// Factorial: Coalesce(R, 1) where R is an aggregate accumulator starting
193+
// from null. The legacy resolves Coalesce(Any, Integer) via generic overload
194+
// matching which unifies T to Any, giving result type Any. This forces an
195+
// unnecessary As(Integer) cast on the Multiply operand. Our pipeline computes
196+
// the result type as the common type of concrete (non-Any) args = Integer,
197+
// which is correct — Coalesce always returns a non-null value of the unified
198+
// concrete type. No downstream cast needed.
199+
//
200+
// RolledOutIntervals: similar issue — legacy over-wraps union operands in
201+
// As(List<Choice<Interval,List<Interval>>>) because the accumulator type
202+
// propagation loses precision. Our pipeline preserves the correct types.
203+
//
204+
// Pattern to watch for: when the legacy inserts As casts that our pipeline
205+
// doesn't, check whether the legacy's type inference lost precision due to
206+
// Any-typed aggregate accumulators or null starting values. If our inferred
207+
// type is strictly more specific and correct, it's a legacy bug, not a
208+
// parity regression.
209+
"Aggregate" to
210+
"Legacy bug #1710: Coalesce type inference loses precision with Any-typed accumulator",
195211
// CqlIntervalOperators: TestEndsNull — CI constant-folds interval bounds but
196212
// emitter's `hasError` check catches the As-wrapped null bound, emitting bare Null.
197213
// The emission-side interval expansion is still needed as fallback for

0 commit comments

Comments
 (0)