cqframework / clinical_quality_language

Clinical Quality Language (CQL) is an HL7 specification for the expression of clinical knowledge that can be used within both the Clinical Decision Support (CDS) and Clinical Quality Measurement (CQM) domains. This repository contains complementary tooling in support of that specification.
https://confluence.hl7.org/display/CDS/Clinical+Quality+Language
Apache License 2.0
266 stars 122 forks source link

ToList is not implemented according to the spec #1133

Open EvanMachusak opened 1 year ago

EvanMachusak commented 1 year ago

According to the ELM spec,

https://cql.hl7.org/04-logicalspecification.html#tolist

The operator is effectively shorthand for "if operand is null then { } else { operand }".

However: https://github.com/cqframework/clinical_quality_language/blob/d10c4ef801a8e160510b2a868c044dea5bdb85c1/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/execution/ToListEvaluator.java

When operand is already a list, the ruler simply returns the operand. That logic is on line 12.

This is important as the cql-to-elm translator assumes this behavior. Consider this CQL:


private codesystem "Test": 'https://example.org/fhir/codesystem/test-cs'
private code "Code 1": '1' from "Test"
private code "Code 2": '2' from "Test"
private code "Code 3": '3' from "Test"

define "Test Codes": { "Code 1", "Code 2", "Code 3" }
define "Retrieve with list of codes": [Observation : "Test Codes"]

The translator is generating this ELM for the codes property of the Retrieve:

       "expression" : {
          "type" : "Retrieve",
          "codes" : {
            "type" : "ToList",
            "operand" : {
              "type" : "ExpressionRef",
              "resultTypeSpecifier" : {
                "type" : "ListTypeSpecifier",
                "elementType" : {
                  "type" : "NamedTypeSpecifier",
                  "name" : "{urn:hl7-org:elm-types:r1}Code"
                }
              },
              "locator" : "24:54-24:65",
              "name" : "Test Codes"
            }
          },

Because the ruler ignores the ToList for expressions that are already lists, it codes would end up being expressed as a List<Code>.

If you were to follow the letter of the ELM spec, codes ends up expressed as a List<List<Code>>.

This confounds engines that expect the terminology element of the retrieve statement to be a List<Code> (or a value set, which is implicitly convertible to a List<Code>).

cmoesel commented 1 year ago

IMO, the implementation makes sense -- if a list is passed to ToList, it should return the list as-is rather than increase the dimensions of the list. Which means that my recommendation would be to update the specification. I'd consider this a "technical correction". @brynrhodes - what say you?

EvanMachusak commented 1 year ago

I can see it either way.

I don't mind whether this is an implementation change or a spec change.

We could tack this into the ELM spec:

When applied to a value that is already a List, this expression returns the existing List.