Repository navigation
Equate open and closed Time interval boundaries - #148
Open
bryantaustin13 wants to merge 1 commit into
Open
bryantaustin13 wants to merge 1 commit into
bryantaustin13 wants to merge 1 commit into
Conversation
#110 equated open and closed numeric interval boundaries, but Time boundaries kept the structural comparison, where the closed flags must match. An engine answering Interval[@T05:59:59.999, @t08:59:59.999) for an expected Interval[@T05:59:59.999, @t08:59:59.998] failed, though both are the same interval: the predecessor of a Time is one unit of its lowest specified precision (CQL Predecessor/Successor). When exactly one of two Time-literal boundaries is open, it now moves one unit of its own precision inwards (hour, minute, second or millisecond) before the values are compared. Boundaries at different precisions, and moves that would leave the day, are not equated. Date and DateTime boundaries keep the structural comparison. On cql-execution this fixes ExceptTimeInterval and ExceptTime2 and changes nothing else; results on the Java engine are unchanged. Part of #85. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
Part of #85.
Problem
#110 made the comparator treat open and closed numeric interval boundaries as equivalent:
[1.0, 4.0)equals[1.0, 3.99999999]. Time boundaries were left on the structural comparison, where the closed flags must match exactly. So an engine that answers with an open Time boundary fails against an expected closed one, even though both describe the same interval.The two tests that
conf/cql-execution-local.jsonstill skips for #85 are this case. With their skips removed, cql-execution returns:ExceptTimeIntervalInterval[@T05:59:59.999, @T08:59:59.998]Interval[@T05:59:59.999, @T08:59:59.999)ExceptTime2Interval[@T11:00:00.000, @T11:59:59.999]Interval(@T10:59:59.999, @T11:59:59.999]CQL 1.5.3 Predecessor (Successor is the same, with adding):
and End:
So the end of
[@T05:59:59.999, @T08:59:59.999)is@T08:59:59.998, and the two forms are the same interval.Change
In
boundariesEqual(src/shared/results-utils.ts), when exactly one of two Time-literal boundaries is open, the open one moves one unit of its own precision inwards before the values are compared:@T09@T09:00@T09:00:00@T09:00:00.000One fractional digit is still millisecond precision, so
.1is 100 ms.Deliberately not equated:
@T08:59:59is not treated as the closed form of@T09:00).@T00:00:00.000or after@T23:59:59.999.When both flags match, nothing changes.
Verification
test/results-utils.test.ts. They cover both reported cases, the step at each precision, wrong step sizes, mixed precisions, the day edges, unchanged same-flag behaviour, and Date boundaries staying structural.f591d9e,conf/cql-execution-local.jsonwithout its runner-issue skips): exactlyExceptTimeIntervalandExceptTime2go from fail to pass. No other test changes.9e92193,conf/localhost.json): 1641 pass / 182 fail / 19 skip / 0 error, the same asmain, with no status or actual-value changes.boundariesEqualas Compare DateTimes at the offset the evaluation actually ran at #145. Applied on top of Compare DateTimes at the offset the evaluation actually ran at #145, the one conflicting hunk resolves by keeping this block and passing Compare DateTimes at the offset the evaluation actually ran at #145'soptionsto the finalresultsEqual. 277 unit tests then pass.Follow-up
Once this merges, the two
ExceptTime2/ExceptTimeIntervalskips can be removed fromconf/cql-execution-local.json. They are the config's last skips for #85. Of the other tests the issue was raised for,IntegerIntervalExcept1to3andExcept12already pass onmain.DecimalIntervalExcept1to3is now skipped there for a different reason (FHIR-59229, CQL 2.0 Decimal semantics).🤖 Generated with Claude Code