Repository navigation
fix(tests): correct five DateTime-uncertainty oracle values to match CQF reference-engine assertions - #175
Open
zacharywhitley wants to merge 1 commit into
Conversation
…CQF reference-engine assertions Correct five `<output>` oracle values in tests/cql/CqlTypesTest.xml and tests/cql/CqlDateTimeOperatorsTest.xml whose current values contradict the first-party CQF reference-engine Kotlin unit tests at engine/src/jvmTest/kotlin/org/opencds/cqf/cql/engine/execution/*.kt. Every corrected value is the value the Kotlin engine already asserts by direct unit test; the fixture oracles were originally authored to a stricter-than-spec rule that expanded the same-precision operand to a sub-precision range, and the drift has been latent while the Kotlin tests served as the de-facto oracle and the fixtures as a secondary check. Per-case diff: - tests/cql/CqlTypesTest.xml:128 DateTimeUncertain Interval [ 18, 49 ] -> Interval [ 19, 49 ] Kotlin oracle: CqlTypesTest.kt:163-165 - tests/cql/CqlDateTimeOperatorsTest.xml:880 DateTimeDurationBetweenUncertainInterval2 Interval[ 4, 16 ] -> Interval[ 5, 16 ] Kotlin oracle: CqlDateTimeOperatorsTest.kt:45-48 - tests/cql/CqlDateTimeOperatorsTest.xml:891 DateTimeDurationBetweenUncertainAdd Interval[ 32, 88 ] -> Interval[ 34, 88 ] Kotlin oracle: CqlDateTimeOperatorsTest.kt:52-55 - tests/cql/CqlDateTimeOperatorsTest.xml:902 DateTimeDurationBetweenUncertainSubtract Interval[ 0, 40 ] -> Interval[ 12, 28 ] Kotlin oracle: CqlDateTimeOperatorsTest.kt:57-60 - tests/cql/CqlDateTimeOperatorsTest.xml:913 DateTimeDurationBetweenUncertainMultiply Interval[ 256, 1936 ] -> Interval[ 289, 1936 ] Kotlin oracle: CqlDateTimeOperatorsTest.kt:62-65 Normative basis: CQL 1.5.2 §5.5.2 "Determining Difference and Duration" (strict-less-than admissibility rule, encoded at DateTime.kt:197-205) and §5.5.3 "Uncertainty Propagation" (endpoint-pairwise interval arithmetic, encoded at AddEvaluator.kt:96-102, SubtractEvaluator.kt:140-148, MultiplyEvaluator.kt:85-93).
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
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 PR corrects five
<output>oracle values intests/cql/CqlTypesTest.xmlandtests/cql/CqlDateTimeOperatorsTest.xmlwhose current valuescontradict the first-party CQF reference-engine Kotlin unit tests at
engine/src/jvmTest/kotlin/org/opencds/cqf/cql/engine/execution/.Every corrected value is the value the Kotlin engine already asserts
by direct unit test; the fixture oracles were originally authored to
a stricter-than-spec rule that expanded the same-precision operand
to a sub-precision range, and the drift has been latent while the
Kotlin tests served as the de-facto oracle and the fixtures as a
secondary check.
Normative basis
(https://cql.hl7.org/05-languagesemantics.html#determining-difference-and-duration):
an operand is uncertain at precision
Piff its stored precisionis strictly less specific than
P; a same-precision operand isused as a canonical point. The §5.5.2 worked example is explicit:
for
days between DateTime(2014, 1, 15) and DateTime(2014, 2)theDAY-precision LHS is the fixed point (its max and min coincide at
Feb 15) and only the MONTH-precision RHS expands to
[Feb 1, Feb 28], yielding[17, 44].(https://cql.hl7.org/05-languagesemantics.html#uncertainty): an
arithmetic operator applied to two
Interval<Integer>operandsproduces a new interval whose endpoints are the endpoint-wise
application of the operator (
start = op(A.start, B.start); end = op(A.end, B.end)). This is the behavior implemented in theCQF reference engine's
AddEvaluator.kt,SubtractEvaluator.kt,and
MultiplyEvaluator.kt(file:line citations below). It is NOTfull-interval arithmetic (
[A.low - B.high, A.high - B.low]); thetwo differ on the Subtract case (full-interval would give
[1, 39]; endpoint-pairwise gives[12, 28]).The strict-less-than admissibility rule is encoded verbatim in the
engine at
engine/src/commonMain/kotlin/org/opencds/cqf/cql/engine/runtime/DateTime.kt:197-205(
isUncertain(p)), and the endpoint-pairwise propagation rule in.../elm/executing/AddEvaluator.kt:96-102,.../SubtractEvaluator.kt:140-148, and.../MultiplyEvaluator.kt:85-93.Per-case diff
tests/cql/CqlTypesTest.xml<output>Interval [ 18, 49 ]</output><output>Interval [ 19, 49 ]</output>engine/src/jvmTest/kotlin/org/opencds/cqf/cql/engine/execution/CqlTypesTest.kt:163-165—assertEquals(19.toCqlInteger(), (value as Interval).start, "DateTimeUncertain"); assertEquals(49.toCqlInteger(), value.end, "DateTimeUncertain")tests/cql/CqlDateTimeOperatorsTest.xml<output>Interval[ 4, 16 ]</output><output>Interval[ 5, 16 ]</output>engine/src/jvmTest/kotlin/org/opencds/cqf/cql/engine/execution/CqlDateTimeOperatorsTest.kt:45-48—assertIs<Interval>(value); assertEquals(5.toCqlInteger(), value.start); assertEquals(16.toCqlInteger(), value.end)tests/cql/CqlDateTimeOperatorsTest.xml<output>Interval[ 32, 88 ]</output><output>Interval[ 34, 88 ]</output>engine/src/jvmTest/kotlin/org/opencds/cqf/cql/engine/execution/CqlDateTimeOperatorsTest.kt:52-55—assertIs<Interval>(value); assertEquals(34.toCqlInteger(), value.start); assertEquals(88.toCqlInteger(), value.end)tests/cql/CqlDateTimeOperatorsTest.xml<output>Interval[ 0, 40 ]</output><output>Interval[ 12, 28 ]</output>engine/src/jvmTest/kotlin/org/opencds/cqf/cql/engine/execution/CqlDateTimeOperatorsTest.kt:57-60—assertIs<Interval>(value); assertEquals(12.toCqlInteger(), value.start); assertEquals(28.toCqlInteger(), value.end)tests/cql/CqlDateTimeOperatorsTest.xml<output>Interval[ 256, 1936 ]</output><output>Interval[ 289, 1936 ]</output>engine/src/jvmTest/kotlin/org/opencds/cqf/cql/engine/execution/CqlDateTimeOperatorsTest.kt:62-65—assertIs<Interval>(value); assertEquals(289.toCqlInteger(), value.start); assertEquals(1936.toCqlInteger(), value.end)Case-by-case derivation
Case #1 —
DateTimeUncertain(days between DateTime(2015, 2, 10) and DateTime(2015, 3)). LHS stored DAY, requested DAY → notuncertain; used as fixed point Feb-10. RHS stored MONTH, requested
DAY → uncertain, expanded to
[Mar-01, Mar-31]. Low bound =days_between(Feb-10, Mar-01) = 19; high bound =days_between(Feb-10, Mar-31) = 49. Result[19, 49]. The priorfixture value
[18, 49]requires expanding the DAY-stored LHS toits sub-day millisecond range, which §5.5.2 does not authorize and
DateTime.isUncertaindoes not do.Case #2 —
DateTimeDurationBetweenUncertainInterval2(
months between DateTime(2005) and DateTime(2006, 5)). LHS storedYEAR, requested MONTH → uncertain, expanded to
[2005-01-01, 2005-12-01]. RHS stored MONTH, requested MONTH → notuncertain; used as fixed point
2006-05-01. Low bound =months_between(2005-12, 2006-05) = 5; high bound =months_between(2005-01, 2006-05) = 16. Result[5, 16]. The priorfixture value
[4, 16]requires expanding the MONTH-stored RHS toits sub-month day range, which §5.5.2 does not authorize.
Case #3 —
DateTimeDurationBetweenUncertainAdd(sum of tworow-37 intervals). Inner inputs each evaluate to
[17, 44]per§5.5.2 and
CqlDateTimeOperatorsTest.kt:42-43. Add is endpoint-pairwise per
AddEvaluator.kt:96-102:[17+17, 44+44] = [34, 88]. The prior fixture value[32, 88]derives from a stricter-than-spec inner
[16, 44]doubled; both theinner and the propagation are corrected to the Kotlin-asserted
values.
Case #4 —
DateTimeDurationBetweenUncertainSubtract(row-37interval minus case-#2 interval). Inner inputs: LHS
[17, 44], RHS[5, 16]. Subtract is endpoint-pairwise perSubtractEvaluator.kt:140-148:[17-5, 44-16] = [12, 28]. Theprior fixture value
[0, 40]derives from stricter-than-spec innervalues
[16, 44]and[4, 16]endpoint-pairwise'd; both the innerinputs and the result are corrected. Note that
[12, 28]is NOT thefull-interval-arithmetic result (
[1, 39]); CQF is explicitlyendpoint-pairwise, which is the behavior preserved by this
correction.
Case #5 —
DateTimeDurationBetweenUncertainMultiply(square oftwo row-37 intervals). Multiply is endpoint-pairwise per
MultiplyEvaluator.kt:85-93:[17*17, 44*44] = [289, 1936]. Theprior fixture value
[256, 1936]derives from[16*16, 44*44]withthe same stricter-than-spec inner.
Downstream engine impact
The CQF reference engine's
engine/src/jvmTest/...Kotlin testsalready assert the proposed values, so this PR does NOT require any
reference-engine change. It only aligns the shared fixture corpus
(which downstream engines consume as an acceptance oracle) with the
reference engine's actual behavior.
Downstream consumer coordination
This fixture oracle correction will cause a flip for any downstream
engine that currently matches the stricter-than-spec oracle (and
will continue to flip for any engine that does NOT). One such
downstream —
cql-wasm— is actively implementing the§5.5.2-compliant uncertainty contract and the §5.5.3 endpoint-
pairwise interval arithmetic (M9) under internal docket
CQL-W54-LOWER-TEMPORAL-UNCERTAINTY-CONTRACT-1.cql-wasmwillpublish a parallel downstream implementation PR coordinated with
this upstream fixture correction. No request is made here for the
downstream PR to merge first; this fixture PR is independently
correct against the CQF reference-engine oracle and against the
§5.5.2 / §5.5.3 normative text.
Backward compatibility
Fixture corrections are retroactive — any engine whose result was
previously accepted because it matched the stricter-than-spec oracle
will now be exposed as actually wrong. This is the intent: aligning
the fixture oracle with the first-party Kotlin reference removes a
stale acceptance of spec-divergent behavior. Engines that pass today
by matching
[17, 44]on row-37 (which the pre-existingDateTimeDurationBetweenUncertainIntervalfixture already assertsand
CqlDateTimeOperatorsTest.kt:40-43already verifies) areunaffected.
Checklist
<output>edits applied as per table aboveexpressiontext modified<output>element)engine/src/jvmTest/kotlin/org/opencds/cqf/cql/engine/execution/*.ktpass unchanged (they already assert the proposed values)