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
258 stars 120 forks source link

Translator Error When Using Fluent Functions With DisableMethodInvocation #678

Open JSRankins opened 2 years ago

JSRankins commented 2 years ago

Receiving an error when using fluent functions in CQL and DisableMethodInvocation is used.

Translator release: 1.5.4

Library:

library ALibrary version '1.0.0'

using FHIR version '4.0.1'

include FHIRHelpers version '4.0.001' called FHIRHelpers
include MATGlobalCommonFunctionsFHIR4 version '6.1.000' called Global

valueset "Diabetes Mellitus": 'http://cts.nlm.nih.gov/fhir/ValueSet/2.16.840.1.113883.3.3157.1329'

parameter "Measurement Period" Interval<DateTime>

context Patient

define fluent function "confirmed"(Conditions List<Condition>):
  Conditions C where C.verificationStatus ~ Global."confirmed"

define fluent function "active"(Conditions List<Condition>):
  Conditions C where C.clinicalStatus ~ Global."active"
    and C.abatement is null

define fluent function "activeOrRecurring"(Conditions List<Condition>):
  Conditions C
    where C.clinicalStatus ~ Global."active"
      or C.clinicalStatus ~ Global."recurrence"
      or C.clinicalStatus ~ Global."relapse"

define "Diabetes Conditions":
  ["Condition": "Diabetes Mellitus"]

define "Confirmed and Active or Recurring Diabetes Conditions":
  "Diabetes Conditions".confirmed().activeOrRecurring()

Error received: Invalid invocation target: org.hl7.elm.r1.ExprressionRef

Per QMIG STU 2, "This option [DisableMethodInvocation ] should be used with Measures to ensure method-style invocation cannot be used within eCQMs."

cmoesel commented 2 years ago

@JSRankins -- what behavior are you expecting? My understanding is that using fluent functions is "method-style invocation" -- so if you disabled method-style invocation, I would not expect fluent functions to work.

JSRankins commented 2 years ago

Hi @cmoesel , this is in a reference to a specific conversation going on in measure development community. I think you are right, but, if that is the case and we want to support the use of fluent functions in measures (under consideration), then QMIG should probably be updated to provide clarification - either never use DisableMethodInvocation or clarity on how this option works with fluent functions. Authoring tools at that point would have to provide users the ability to toggle that option or be very specific about supporting/not supporting fluent functions. @brynrhodes , thoughts?

brynrhodes commented 2 years ago

Confirmed that this is an issue with the use of that option, and that we would need to turn that off in order for Quality Measures to be able to take advantage of fluent functions. That decision was made quite a while ago, before the introduction of fluent functions in CQL, and was meant to prevent unexpected usage of FHIRPath functions, which were "fluent", but only for the set understood by the compiler (as in, pre-defined in the FHIRPath spec). Now that fluent functions are a feature of the language, this decision should be revisited, because the benefits of fluent functions for readability clear.

I also confirmed that the current Atom and VSCode plugins do not respect the cql-options, so because I'm using Atom to do this development, I didn't see this issue, so thank you for raising it Stan. I'll log a tracker to raise this in QM IG, and there is already an issue to add cql-options support in the Atom plugin: https://github.com/cqframework/atom_cql_support/issues/19

JSRankins commented 2 years ago

Thank you @brynrhodes. Should we make this same update for CQL-based HQMF IG too?

p9g commented 2 years ago

http://build.fhir.org/ig/HL7/cqf-measures/using-cql.html#translation-to-elm

JSRankins commented 2 years ago

Thank you @p9g.

@brynrhodes If we want to take the position that 'should' doesn't set a requirement for tooling, then I think it is a double edged sword. Given the conversation around 'should' as it applies to Disable Method Invocation, do we need to consider the 'should' language around list demotion and list promotion? I'm not comfortable with those staying as 'should' because it now seems that 'should' puts an implementation between a rock and a hard place. If the language remains as 'should', I think the only viable option for an implementation at that point is to surface those options up to the user and make the user decide (would the user even understand what decision he/she was making at that point?), which could create the need to establish a governing policy for certain programs. Thoughts?

brynrhodes commented 2 years ago

@JSRankins , that's a fair point, but the language in the header to the section is specifically about making a recommendation for use with measures. I'm pretty uncomfortable changing all the SHOULDs to SHALLs, though I recognize that it does create a governance/policy issue. I think the de facto policy is that the SHOULDs have been implemented as SHALLs in the measure space, which is a fair implementation decision to avoid the need to surface those as options to the users. I think it makes sense to preserve that interpretation and just change the recommendation in the IG, and I've proposed that as a disposition here: https://jira.hl7.org/browse/FHIR-34197

JSRankins commented 2 years ago

@brynrhodes Thank you for creating the ticket. I appreciate the work that you are doing. My concern with leaving these as 'SHOULD' is that this is the De Facto standard for CMS implementations right now and we may not remember when, how or why a particular requirement was implemented in the future. I don't believe the community has that long term of a memory. I know I usually don't. Short of changing the language, it seems that these nuances may create the need for a CMS implementation guide (not as full-fleshed as the current CMS IGs) that would establish a particular requirement where the underlying IG remains neutral. The purpose of this IG would be to keep the tooling teams safe as well as provide clear documentation for the community.

brynrhodes commented 2 years ago

The error message here could be better, so I'm going to mark this as an enhancement to improve the error message when accessing a fluent function with the disable method invocation option set.