Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not Equal and Not Equivalent clause coverage calculation handling #291

Merged
merged 7 commits into from
Dec 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions src/calculation/ClauseResultsHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,121 +106,134 @@
alId = (parseInt(statement.localId, 10) - 1).toString();
emptyResultClauses.push({ lib: libraryName, aliasLocalId: alId, expressionLocalId: aliasMap[v] });
}
} else if (k === 'asTypeSpecifier') {
// Map the localId of the asTypeSpecifier (Code, Quantity...) to the result of the result it is referencing
// For example, in the CQL code 'Variable.result as Code' the typeSpecifier does not produce a result, therefore
// we will set its result to whatever the result value is for 'Variable.result'
alId = statement.asTypeSpecifier.localId;
if (alId != null) {
const typeClauseId = (parseInt(statement.asTypeSpecifier.localId, 10) - 1).toString();
emptyResultClauses.push({ lib: libraryName, aliasLocalId: alId, expressionLocalId: typeClauseId });
}
} else if (k === 'sort') {
// Sort is a special case that we need to recurse into separately and set the results to the result of the statement the sort clause is in
findAllLocalIdsInSort(v, libraryName, localIds, aliasMap, emptyResultClauses, parentNode);
} else if (k === 'let') {
// let is a special case where it is an array, one for each defined alias. These aliases work slightly different
// and execution engine does return results for them on use. The Initial naming of them needs to be properly pointed
// to what they are set to.
let aLet: any;
for (aLet of Array.from(v)) {
// Add the localId for the definition of this let to it's source.
localIds[aLet.localId] = { localId: aLet.localId, sourceLocalId: aLet.expression.localId };
findAllLocalIdsInStatement(
aLet.expression,
libraryName,
annotation,
localIds,
aliasMap,
emptyResultClauses,
statement
);
}
// handle the `when` pieces of Case expression aka CaseItems. They have a `when` key that should be mapped to get a result from the expression that defines them
} else if (k === 'when' && statement.localId && v.localId) {
localIds[statement.localId] = { localId: statement.localId, sourceLocalId: v.localId };
findAllLocalIdsInStatement(v, libraryName, annotation, localIds, aliasMap, emptyResultClauses, statement);
// If 'First' and 'Last' expressions, the result of source of the clause should be set to the expression
} else if (k === 'type' && (v === 'First' || v === 'Last')) {
if (statement.source && statement.source.localId != null) {
alId = statement.source.localId;
emptyResultClauses.push({ lib: libraryName, aliasLocalId: alId, expressionLocalId: statement.localId });
}
// Continue to recurse into the 'First' or 'Last' expression
findAllLocalIdsInStatement(v, libraryName, annotation, localIds, aliasMap, emptyResultClauses, statement);
// If this is a FunctionRef or ExpressionRef and it references a library, find the clause for the library reference and add it.
} else if (k === 'type' && (v === 'FunctionRef' || v === 'ExpressionRef') && statement.libraryName != null) {
const libraryClauseLocalId = findLocalIdForLibraryRef(annotation, statement.localId, statement.libraryName);
if (libraryClauseLocalId !== null) {
// only add the clause if the localId for it is found
// the sourceLocalId is the FunctionRef itself to match how library statement references work.
localIds[libraryClauseLocalId] = { localId: libraryClauseLocalId, sourceLocalId: statement.localId };
}

// FunctionRefs that use an alias in the scope need to account for the localId of the parent minus 1
if (v === 'FunctionRef') {
(statement as ELMFunctionRef).operand.forEach(o => {
if (o.type === 'Property') {
const propExpr = o as ELMProperty;
if (propExpr.scope != null) {
alId = (parseInt(statement.localId, 10) - 1).toString();
emptyResultClauses.push({
lib: libraryName,
aliasLocalId: alId,
expressionLocalId: aliasMap[propExpr.scope]
});
}
}
});
}
} else if (
k === 'type' &&
(v === 'Equal' ||
v === 'Equivalent' ||
v === 'Greater' ||
v === 'GreaterOrEqual' ||
v === 'Less' ||
v === 'LessOrEqual' ||
v === 'NotEqual')
) {
// Comparison operators are special cases that we need to recurse into and set the localId of a literal type to
// the localId of the whole comparison expression
findLocalIdsForComparisonOperators(statement as ELMBinaryExpression, libraryName, emptyResultClauses);
} else if (k === 'type' && v === 'Property') {
// Handle aliases that are nested within `source` attribute of a Property expression
// This case is similar to the one above when accessing `.scope` directly, but we need to drill into `.source.scope` instead
// This issue came about when working with CQL authored `using QICore` instead of FHIR
if (typeof statement.source === 'object' && statement.source.scope != null && statement.source.localId == null) {
const alias = statement.source.scope;
alId = (parseInt(statement.localId, 10) - 1).toString();
emptyResultClauses.push({ lib: libraryName, aliasLocalId: alId, expressionLocalId: aliasMap[alias] });
}
} else if (
k === 'type' &&
v === 'Query' &&
Array.isArray(statement.source) &&
statement.source.length === 1 &&
statement.source[0].localId == null &&
statement.source[0].expression.scope != null
) {
// Handle aliases that are nested within an expression object in the one object on a `source` array of a Query expression
// This case is similar to the one above, but we need to drill into `.source[0].expression.scope` instead
const alias = statement.source[0].expression.scope;
alId = (parseInt(statement.localId, 10) - 1).toString();
emptyResultClauses.push({ lib: libraryName, aliasLocalId: alId, expressionLocalId: aliasMap[alias] });
} else if (k === 'type' && v === 'Null' && statement.localId) {
// If this is a "Null" expression, mark that it `isFalsyLiteral` so we can interpret final results differently.
localIds[statement.localId] = { localId: statement.localId, isFalsyLiteral: true };
} else if (k === 'type' && v === 'Literal' && statement.localId && statement.value === 'false') {
// If this is a "Literal" expression whose value is false, mark that it `isFalsyLiteral` so we can interpret final results differently
localIds[statement.localId] = { localId: statement.localId, isFalsyLiteral: true };
} else if (k === 'type' && v === 'Not') {
// If this is a "Not" expression, we will want to check if it's operand type is 'Equivalent' or 'Equal'
// If so, we will want to treat this as a 'Not Equivalent' or 'Not Equal' expression which we can do so
// by mapping the 'Equal' or 'Equivalent' clause localId to that of the 'Not' clause
if (statement.operand && statement.localId && statement.operand.localId) {
if (statement.operand.type === 'Equivalent' || statement.operand.type === 'Equal') {
Copy link
Contributor

Choose a reason for hiding this comment

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

This whole set of logic could use a test case. It's one of the few things in here that isn't tested.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call! I added a unit test for the !~...however, I do not think I am able to add a unit test for the != logic at the moment since an update to the translator will be coming soon that puts a localId on the Equal expression (but it doesn't right now). Am I correct in this? @birick1 may have additional insight as well

elsaperelli marked this conversation as resolved.
Show resolved Hide resolved
emptyResultClauses.push({
lib: libraryName,
aliasLocalId: statement.operand.localId,
expressionLocalId: statement.localId
});
}
}
} else if (k === 'localId') {
// else if the key is localId, push the value
localIds[v] = { localId: v };
} else if (Array.isArray(v) || typeof v === 'object') {
// if the value is an array or object, recurse
findAllLocalIdsInStatement(v, libraryName, annotation, localIds, aliasMap, emptyResultClauses, statement);
}

Check warning on line 236 in src/calculation/ClauseResultsHelpers.ts

View workflow job for this annotation

GitHub Actions / Coverage annotations (🧪 jest-coverage-report-action)

🌿 Branch is not covered

Warning! Not covered branch

Check warning on line 236 in src/calculation/ClauseResultsHelpers.ts

View workflow job for this annotation

GitHub Actions / Coverage annotations (🧪 jest-coverage-report-action)

🌿 Branch is not covered

Warning! Not covered branch

Check warning on line 236 in src/calculation/ClauseResultsHelpers.ts

View workflow job for this annotation

GitHub Actions / Coverage annotations (🧪 jest-coverage-report-action)

🌿 Branch is not covered

Warning! Not covered branch

Check warning on line 236 in src/calculation/ClauseResultsHelpers.ts

View workflow job for this annotation

GitHub Actions / Coverage annotations (🧪 jest-coverage-report-action)

🌿 Branch is not covered

Warning! Not covered branch
}

return localIds;
Expand Down
30 changes: 30 additions & 0 deletions test/unit/ClauseResultsHelper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,36 @@ import { getJSONFixture } from './helpers/testHelpers';

describe('ClauseResultsHelpers', () => {
describe('findAllLocalIdsInStatementByName', () => {
test('finds localIds for an Equivalent comparison operator that is wrapped in a Not expression', () => {
// ELM from test/unit/fixtures/cql/NotEquivalent.cql
const libraryElm: ELM = getJSONFixture('elm/NotEquivalent.json');

const statementName = 'Not Equivalent Clause';
const localIds = ClauseResultsHelpers.findAllLocalIdsInStatementByName(libraryElm, statementName);

// For the fixture loaded for this test it is known that the localId for the Equivalent statement
// is 23 and the localId for the Not expression is 24 but we want the Equivalent clause to take
// the result of the Not expression
expect(localIds[23]).toBeDefined();
expect(localIds[23]).toEqual({ localId: '23', sourceLocalId: '24' });
});

test('finds localIds for an Equal comparison operator that is wrapped in a Not expression', () => {
// ELM from test/unit/fixtures/cql/NotEqual.cql, but modified so that Equal has a localId of "100"
// This will soon be the translator functionality, the Equal clause will get a localId (it currently
// does not get one)
const libraryElm: ELM = getJSONFixture('elm/NotEqual.json');

const statementName = 'Not Equal Clause';
const localIds = ClauseResultsHelpers.findAllLocalIdsInStatementByName(libraryElm, statementName);

// For the fixture loaded for this test it is known that the localId for the Equal statement
// is 100 and the localId for the Not expression is 23 but we want the Equal clause to take
// the result of the Not expression
expect(localIds[100]).toBeDefined();
expect(localIds[100]).toEqual({ localId: '100', sourceLocalId: '23' });
});

test('finds localIds for an ELM Binary Expression with a comparison operator with a literal', () => {
// ELM from test/unit/fixtures/cql/comparisonWithLiteral.cql
const libraryElm: ELM = getJSONFixture('elm/ComparisonWithLiteral.json');
Expand Down
24 changes: 24 additions & 0 deletions test/unit/fixtures/cql/NotEqual.cql
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
library Test

using FHIR version '4.0.1'

include FHIRHelpers version '4.0.1'
include MATGlobalCommonFunctions version '5.0.000' called Global

codesystem "EXAMPLE": 'http://example.com'
codesystem "EXAMPLE-2": 'http://example.com/2'
codesystem "ConditionClinicalStatusCodes": 'http://terminology.hl7.org/CodeSystem/condition-clinical'

valueset "test-vs": 'http://example.com/test-vs'

code "Active": 'active' from "ConditionClinicalStatusCodes"
code "Recurrence": 'recurrence' from "ConditionClinicalStatusCodes"
code "Relapse": 'relapse' from "ConditionClinicalStatusCodes"

concept "Condition Active": { "Active", "Recurrence", "Relapse" } display 'Active'

context Patient

define "Not Equal Clause":
[Condition: "test-vs"] C
where C.id != 'notId'
24 changes: 24 additions & 0 deletions test/unit/fixtures/cql/NotEquivalent.cql
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
library Test

using FHIR version '4.0.1'

include FHIRHelpers version '4.0.1'
include MATGlobalCommonFunctions version '5.0.000' called Global

codesystem "EXAMPLE": 'http://example.com'
codesystem "EXAMPLE-2": 'http://example.com/2'
codesystem "ConditionClinicalStatusCodes": 'http://terminology.hl7.org/CodeSystem/condition-clinical'

valueset "test-vs": 'http://example.com/test-vs'

code "Active": 'active' from "ConditionClinicalStatusCodes"
code "Recurrence": 'recurrence' from "ConditionClinicalStatusCodes"
code "Relapse": 'relapse' from "ConditionClinicalStatusCodes"

concept "Condition Active": { "Active", "Recurrence", "Relapse" } display 'Active'

context Patient

define "Not Equivalent Clause":
[Condition: "test-vs"] C
where C.clinicalStatus !~ "Condition Active"
Loading
Loading