"type": "Equivalent" Should not contain a localId

RohitKandimalla commented 8 months ago

Hello Team,

BonnieMAT team received a help desk ticket where user is unable to achieve 100% test coverage. After investigating we found out that the ELM generated for that particular measure is including a localId property for "type": "Equivalent" when followed by a "NOT" (negation). Ideally localId should not be added as it is causing issues downstream.

Original HelpDesk ticket : BONNIEMAT-1658

Original Measure bundle exported from MAT, it contains the CQL for the measure library as well as all included libraries. CMS137-v12-0-001-QDM-5-6.zip

CQL definition where it is used

define "Treatment Initiation With Non Medication Intervention Dates":
  "Psychosocial Visit" PsychosocialVisit
    let treatmentDate: date from start of Global."NormalizeInterval" ( PsychosocialVisit.relevantDatetime, PsychosocialVisit.relevantPeriod )
    with "First SUD Episode During Measurement Period" FirstSUDEpisode
      such that treatmentDate during Interval[date from start of FirstSUDEpisode.relevantPeriod, date from start of FirstSUDEpisode.relevantPeriod + 14 days )
        and PsychosocialVisit.id !~ FirstSUDEpisode.id
    return all treatmentDate

Corresponding ELM

{
      "localId": "212",
      "locator": "133:13-133:54",
      "type": "Not",
      "operand":
      {
          "localId": "211",
          "locator": "133:13-133:54",
          "type": "Equivalent",
          "operand":
          [
              {
                  "localId": "208",
                  "locator": "133:13-133:32",
                  "path": "id",
                  "scope": "PsychosocialVisit",
                  "type": "Property"
              },
              {
                  "localId": "210",
                  "locator": "133:37-133:54",
                  "path": "id",
                  "scope": "FirstSUDEpisode",
                  "type": "Property"
              }
          ]
      }
  }

As we can see in the ELM, Equivalent operator is followed by a negation ( NOT ), in this case having a localID ("localId": "211") for Equivalent object is causing issues downstream.

Note: When CQL is modified to check equal condition instead of equivalent PsychosocialVisit.id != FirstSUDEpisode.id then the ELM generated doesn't contain a localID and hence the code coverage is not effected.

Please let us know if you need any additional information, Happy to provide !!

JPercival commented 8 months ago

It looks to me like the localId is correctly emitted there. Not and Equivalent are both first-class operators in the CQL spec. Consider the following examples:

Emits localId for equal:

define "Test":
    [Observation] O where O.status.value = 'final'

Emits localId for equivalent:

define "Test":
    [Observation] O where O.status.value ~ 'final'

Does NOT emit localId for equal:

define "Test":
    [Observation] O where O.status.value != 'final'

Does emit localId for equivalent:

define "Test":
    [Observation] O where O.status.value !~ 'final'

So, I actually think the bug is the opposite of what you've posted. For some reason the localId is not being emitted when negating Equals when in fact it should be.

@brynrhodes what are your thoughts?

JSRankins commented 8 months ago

@JPercival , I was wondering that as well when we were looking at this. It appears that cqm-execution and fqm-execution are ironically providing the anticipated behavior for coverage calculation when the localId is missing for "Equivalent" first-class operator. But I think that's because the localId for the "Equivalent" expression is not in the annotations.

Annotation from CMS137-v12-0-001-QMD-5-6.json (from CMS137-v12-0-001-QDM-5-6.zip starting around line 3083): { "r" : "212", "s" : [ { "r" : "208", "s" : [ { "r" : "207", "s" : [ { "value" : [ "PsychosocialVisit" ] } ] }, { "value" : [ "." ] }, { "r" : "208", "s" : [ { "value" : [ "id" ] } ] } ] }

But the localId is present within the related expression block as @RohitKandimalla noted (starting around line 3262). { "localId" : "212", "locator" : "133:13-133:54", "type" : "Not", "operand" : { "localId" : "211", "locator" : "133:13-133:54", "type" : "Equivalent", "operand" : [ {

As a result, there is nothing to tie 211 back to, and coverage is impacted.

brynrhodes commented 7 months ago

I agree that localIds should be being emitted, and I think you're right @JPercival , that the fact that it's not being emitted for Equivalent is actually a bug. However, I think what Stan is pointing out is that the annotations aren't consistent, in that they are referencing a localId that doesn't exist. So I think there are two issues here to be addressed.

brynrhodes commented 7 months ago

I tagged with md impact tracking (because this is affecting coverage calculation) and with engine impact (because it impacts how annotations and localIds are being output which affects coverage calculation).

JPercival commented 7 months ago

Expected root cause fix is: https://github.com/cqframework/clinical_quality_language/issues/697

EvanMachusak commented 7 months ago

I would think implementers of cql-to-elm are free to assign local IDs to all elements. It is a property of the Element type in the schema.

JPercival commented 7 months ago

Also related #1295

birick1 commented 7 months ago

For fqm-execution, we also encountered the coverage calculation behavior noted in the original post in https://github.com/projecttacoma/fqm-execution/issues/287.

In that ticket, the third measure - CMS69 - uses a !~ which affects the coverage calculation. In short, because the ELM for !~ is split into both ELM clauses Not and Equivalent, getting coverage requires two test cases:

the objects aren't equivalent (to satisfy ELM Not)
the objects are equivalent (to satisfy ELM Equivalent)

The need for both test cases is the reason why 100% isn't achieved:

BonnieMAT team received a help desk ticket where user is unable to achieve 100% test coverage. After investigating we found out that the ELM generated for that particular measure is including a localId property for "type": "Equivalent" when followed by a "NOT" (negation). Ideally localId should not be added as it is causing issues downstream.

Having the localId for Equals omitted in the ELM for != looks like it works, but is really just masking the above issue for the coverage calculation when using the != operator. Once the localId is emitted, != and !~ will have the same behavior in the coverage calculation and need two test cases.

To address needing two test cases when using !~ or != (assuming a localId will be emitted in the future), we've drafted PR https://github.com/projecttacoma/fqm-execution/pull/291 to detect the use of !~ and !=. When those cases are detected, only a test case checking the objects are not equal/not equivalent will be needed. We are still testing/reviewing this PR, and would welcome feedback.

Note: fqm-execution is only for FHIR measures. The original issue in this ticket looks like it is for a QDM measure. I haven't verified that QDM measures needed both test cases like the FHIR measures do, but the behavior sounds the same.

cqframework / clinical_quality_language

"type": "Equivalent" Should not contain a localId #1279