Skip to content

Fix RolledOutIntervals starting type - #146

Open
cmoesel wants to merge 1 commit into
mainfrom
fix-RolledOutIntervals-type
Open

cmoesel wants to merge 1 commit into
mainfrom
fix-RolledOutIntervals-type

Conversation

@cmoesel

@cmoesel cmoesel commented Aug 26, 2026

Copy link
Copy Markdown
Member

Fix the starting type to be List<Interval<Date>> so it matches the query source type and the test's expected output type. When the starting type is List<Interval<DateTime>> (as it is on main now), the Dates all get implicitly converted to DateTimes -- and the output is a list of DateTime intervals rather than the expected list of Date intervals.

You can see this if you paste the test expression into CQL Playground and turn on Enable result types:

library Playground
context Unfiltered
define Foo: ({
  Interval[@2012-01-01, @2012-02-28],
  Interval[@2012-02-01, @2012-03-31],
  Interval[@2012-03-01, @2012-04-30]
}) M
  aggregate R starting (null as List<Interval<DateTime>>): R union ({
    M X
      let S: Max({ end of Last(R) + 1 day, start of X }),
        E: S + Quantity{ value: duration in days of X, unit: 'days' }
      return Interval[S, E]
  })

If you change starting (null as List<Interval<DateTime>>) to starting (null as List<Interval<Date>>) (as in this PR), then the result type is as expected.

Fix the starting type to be List<Interval<Date>> so it matches the query source type and the test's expected output type. Otherwise the Dates all get implicitly converted to DateTimes and the expected output is wrong.

@bryantaustin13 bryantaustin13 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved. Needs a change in cql-tests-runner to utilize this:

Every recorded run in cql-tests-results — cqf-java, cqf-javascript, firely, cozeva — reports:

"testStatus": "skip",
"skipMessage": "Skipped by config: CQLtoELM - Could not resolve identifier
MedicationRequestIntervals in the current library"

That skip cites the pre-2024-08 expression. The identifier isn't in the test anymore, but the entry is still in cql-tests-runner SkipList in five configs — conf/localhost.json, development.json, smile-cdr-local.json, cql-execution-local.json, localhost-evaluate.json — and is recorded in docs/test-run-evaluation-2026-08-19.md as one of the 14 skips.

@cmoesel

cmoesel commented Sep 9, 2026

Copy link
Copy Markdown
Member Author

I'm not sure if I can/should approve my own PR, but want to note that MITRE's JS cql-execution team approves of this PR as well (so you do have the approval of two different engine teams).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants