Skip to content

Commit

Permalink
Relationship alias coverage fixes (#317)
Browse files Browse the repository at this point in the history 
* Initial incomplete fix for relationship alias coverage

* Added code to clause results helpers to find the relationship expression alias id in annotation structure, replacing old +1 hack

* reworked html generation tests to use input more reflective of actual usage

* Update comments for clarity

* Check for undefined alias id and update iterator variable

* Added UNHIT results to the HTMLBuilder Tests

---------

Co-authored-by: lmd59 <laurend@mitre.org>
  • Loading branch information
@hossenlopp @lmd59
hossenlopp and lmd59 authored Nov 1, 2024
1 parent 10fa89b commit 69dcd10
Show file tree
Hide file tree
Showing 8 changed files with 235 additions and 112 deletions.
6 changes: 3 additions & 3 deletions src/calculation/HTMLBuilder.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,11 @@ Handlebars.registerHelper('highlightCoverage', (localId, context) => {
const libraryName: string = context.data.root.libraryName;
const clauseResults: ClauseResult[] = context.data.root.clauseResults;

const clauseResult = clauseResults.filter(result => result.libraryName === libraryName && result.localId === localId);
const clauseResult = clauseResults.find(result => result.libraryName === libraryName && result.localId === localId);
if (clauseResult) {
if (clauseResult.some(c => c.final === FinalResult.TRUE)) {
if (clauseResult.final === FinalResult.TRUE) {
return objToCSS(cqlLogicClauseCoveredStyle);
} else if (clauseResult.every(c => c.final === FinalResult.FALSE || c.final === FinalResult.UNHIT)) {
} else if (clauseResult.final === FinalResult.FALSE || clauseResult.final === FinalResult.UNHIT) {
return objToCSS(cqlLogicUncoveredClauseStyle);
}
}
Expand Down
89 changes: 83 additions & 6 deletions src/helpers/ClauseResultsHelpers.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ELMProperty } from '../types/ELMTypes';
import { Annotation, AnnotationStatement, ELMProperty } from '../types/ELMTypes';
import { ELMFunctionRef } from '../types/ELMTypes';
import { ELM, ELMBinaryExpression, ELMStatement } from '../types/ELMTypes';

Expand Down Expand Up @@ -33,7 +33,7 @@ export function findLocalIdsInStatementByName(libraryElm: ELM, statementName: st
for (const alias of Array.from(emptyResultClauses)) {
// Only do it if we have a clause for where the result should be fetched from
// and have a localId for the clause that the result should map to
if (localIds[alias.expressionLocalId] != null && alias.aliasLocalId != null) {
if (localIds[alias.expressionLocalId] != null && alias.aliasLocalId) {
localIds[alias.aliasLocalId] = {
localId: alias.aliasLocalId,
sourceLocalId: alias.expressionLocalId
Expand Down Expand Up @@ -132,11 +132,16 @@ export function findAllLocalIdsInStatement(
aliasMap[v] = statement.expression.localId;
// Determine the localId for this alias.
if (statement.localId) {
// Older translator versions require with statements to use the statement.expression.localId + 1 as the alias Id
// even if the statement already has a localId. There is not a clear mapping for alias with statements in the new
// translator, so they will go un highlighted but this will not affect coverage calculation
// There is not a clear mapping for `With` and `Without` relationship aliases with statements in newer
// translator versions. The node in the annotation that contains the alias has a local id that doesn't
// exist in the elm. We have to find this localId by looking for it in the annotation structure.
if (statement.type === 'With' || statement.type === 'Without') {
alId = (parseInt(statement.expression.localId, 10) + 1).toString();
if (annotation) {
const id = findRelationshipAliasAnnotationId(annotation, statement.expression.localId);
if (id) {
alId = id;
}
}
} else {
alId = statement.localId;
}
Expand Down Expand Up @@ -347,6 +352,78 @@ export function findLocalIdsForComparisonOperators(
}
}

/**
* Helper function to kick off the recursive search for the relationship (With, Without) source's localId in the annotation
* structure. If found this returns the localId of the parent node of the given source localId. Null is returned if not
* found.
*/
function findRelationshipAliasAnnotationId(annotation: Annotation[], sourceLocalId: string): string | null {
for (const a of annotation) {
const id = findRelationshipAliasNodeAnnotationId([a.s], sourceLocalId);
if (id) {
return id;
}
}
return null;
}

/**
* Recursively looks through the annotation structure for the source localId and grabs its parent id which also contains
* the alias. This search is valid to be used with relationship (With, Without) clauses and allows us to tag the found
* localId, which only exists in the annotation, as the alias localId for the source so it can be highlighted when the
* source clause has a result.
*
* For example in the below snippet. We are looking for the source localId 355 and want to return 301 which includes
* the alias.
*
* {
* "r": "301",
* "s": [
* {
* "r": "355",
* "s": [
* {
* "s": [
* {
* "value": [
* "\"Bladder Cancer Diagnosis\""
* ]
* }
* ]
* }
* ]
* },
* {
* "value": [
* " ",
* "BladderCancer"
* ]
* }
* ]
* }
*
* @returns id of the node that is parent to the source localId if found
*/
function findRelationshipAliasNodeAnnotationId(
annotation: AnnotationStatement[],
sourceLocalId: string
): string | null {
for (const node of annotation) {
// if this node has a list of more nodes in s, look at the first one and see if it matches the sourceLocalId
if (node.r && node.s && node.s[0]?.r === sourceLocalId) {
// return this localId which is the parent of the sourceLocalId
return node.r;
} else if (node.s) {
// otherwise, keep recursing and return the alias localId if found
const id = findRelationshipAliasNodeAnnotationId(node.s, sourceLocalId);
if (id) {
return id;
}
}
}
return null;
}

/**
* Find the localId of the library reference in the JSON elm annotation. This recursively searches the annotation structure
* for the clause of the library ref. When that is found it knows where to look inside of that for where the library
Expand Down
99 changes: 76 additions & 23 deletions test/unit/HTMLBuilder.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,13 @@ import { getELMFixture, getHTMLFixture, getJSONFixture } from './helpers/testHel

describe('HTMLBuilder', () => {
let elm = <ELM>{};
let simpleExpression: ELMStatement | undefined;
let denominatorExpression: ELMStatement | undefined;
let numeratorExpression: ELMStatement | undefined;
let statementResults: StatementResult[];
let trueClauseResults: ClauseResult[];
let falseClauseResults: ClauseResult[];
const desiredLocalId = '119';
const denominatorLocalId = '119';
const numeratorLocalId = '135';
const trueStyleString = objToCSS(cqlLogicClauseTrueStyle);
const falseStyleString = objToCSS(cqlLogicClauseFalseStyle);
const coverageStyleString = objToCSS(cqlLogicClauseCoveredStyle);
Expand Down Expand Up @@ -114,54 +116,105 @@ describe('HTMLBuilder', () => {

beforeEach(() => {
elm = getELMFixture('elm/CMS723v0.json');
simpleExpression = elm.library.statements.def.find(d => d.localId === desiredLocalId); // Simple expression for Denominator
denominatorExpression = elm.library.statements.def.find(d => d.localId === denominatorLocalId); // Simple expression for Denominator
numeratorExpression = elm.library.statements.def.find(d => d.localId === numeratorLocalId); // Simple expression for Denominator

//
statementResults = [
{
statementName: simpleExpression?.name ?? '',
statementName: denominatorExpression?.name ?? '',
libraryName: elm.library.identifier.id,
final: FinalResult.TRUE,
relevance: Relevance.TRUE,
localId: desiredLocalId
localId: denominatorLocalId
},
{
statementName: numeratorExpression?.name ?? '',
libraryName: elm.library.identifier.id,
final: FinalResult.UNHIT,
relevance: Relevance.FALSE,
localId: numeratorLocalId
}
];

trueClauseResults = [
{
statementName: simpleExpression?.name ?? '',
statementName: denominatorExpression?.name ?? '',
libraryName: elm.library.identifier.id,
localId: desiredLocalId,
localId: denominatorLocalId,
final: FinalResult.TRUE,
raw: true
},
{
statementName: denominatorExpression?.name ?? '',
libraryName: elm.library.identifier.id,
localId: '118',
final: FinalResult.TRUE,
raw: [{ resourceType: 'foo' }]
},
{
statementName: denominatorExpression?.name ?? '',
libraryName: elm.library.identifier.id,
localId: '116',
final: FinalResult.TRUE,
raw: [{ resourceType: 'foo' }]
},
{
statementName: denominatorExpression?.name ?? '',
libraryName: elm.library.identifier.id,
localId: '115',
final: FinalResult.TRUE,
raw: [{ resourceType: 'foo' }]
}
];

falseClauseResults = [
{
statementName: simpleExpression?.name ?? '',
statementName: denominatorExpression?.name ?? '',
libraryName: elm.library.identifier.id,
localId: desiredLocalId,
localId: denominatorLocalId,
final: FinalResult.FALSE,
raw: false
},
{
statementName: denominatorExpression?.name ?? '',
libraryName: elm.library.identifier.id,
localId: '117',
final: FinalResult.FALSE,
raw: []
},
// specifically not including this result to make this clause have no coverage styling.
// This simulates a clause that only exists in the annotation.
// {
// statementName: simpleExpression?.name ?? '',
// libraryName: elm.library.identifier.id,
// localId: '101',
// final: FinalResult.FALSE,
// raw: []
// }
{
statementName: numeratorExpression?.name ?? '',
libraryName: elm.library.identifier.id,
localId: numeratorLocalId,
final: FinalResult.UNHIT,
raw: false
}
];
});

test('simple HTML with generation with true clause', () => {
test('simple HTML with generation with mix of false and true clauses', () => {
// Ignore tabs and new lines
const expectedHTML = getHTMLFixture('simpleTrueAnnotation.html').replace(/\s/g, '');
const res = generateHTML(simpleMeasure, [elm], statementResults, trueClauseResults, 'test');
const expectedHTML = getHTMLFixture('simpleHighlightingAnnotation.html').replace(/\s/g, '');
const res = generateHTML(
simpleMeasure,
[elm],
statementResults,
[...trueClauseResults, ...falseClauseResults],
'test'
);

expect(res.replace(/\s/g, '')).toEqual(expectedHTML);
expect(res.includes(trueStyleString)).toBeTruthy();
});

test('simple HTML with generation with false clause', () => {
// Ignore tabs and new lines
const expectedHTML = getHTMLFixture('simpleFalseAnnotation.html').replace(/\s/g, '');
const res = generateHTML(simpleMeasure, [elm], statementResults, falseClauseResults, 'test');

expect(res.replace(/\s/g, '')).toEqual(expectedHTML);
expect(res.includes(falseStyleString)).toBeTruthy();
});

Expand Down Expand Up @@ -213,7 +266,7 @@ describe('HTMLBuilder', () => {
detailedResults: [
{
statementResults: statementResults,
clauseResults: [trueClauseResults[0], falseClauseResults[0]],
clauseResults: [...trueClauseResults, ...falseClauseResults],
groupId: 'test'
}
]
Expand All @@ -235,12 +288,12 @@ describe('HTMLBuilder', () => {
detailedResults: [
{
statementResults: statementResults,
clauseResults: [trueClauseResults[0], falseClauseResults[0]],
clauseResults: [...trueClauseResults, ...falseClauseResults],
groupId: 'test'
},
{
statementResults: statementResults,
clauseResults: [trueClauseResults[0], falseClauseResults[0]],
clauseResults: [...trueClauseResults, ...falseClauseResults],
groupId: 'test2'
}
]
Expand Down
17 changes: 12 additions & 5 deletions test/unit/fixtures/html/simpleCoverageAnnotation.html
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<div>
<h2>test Clause Coverage: 100%</h2>
<h2>test Clause Coverage: 66.7%</h2>
<pre
style="tab-size: 2"
data-library-name="AnticoagulationTherapyforAtrialFibrillationFlutter"
Expand All @@ -8,13 +8,13 @@ <h2>test Clause Coverage: 100%</h2>
<code>
<span data-ref-id="119" style="background-color:#daeaf5;color:#004e82">
<span>define &quot;Denominator&quot;: </span>
<span data-ref-id="118" style="background-color:white;color:black">
<span data-ref-id="116" style="background-color:white;color:black">
<span data-ref-id="101" style="background-color:white;color:black">
<span data-ref-id="118" style="background-color:#daeaf5;color:#004e82">
<span data-ref-id="116" style="background-color:#daeaf5;color:#004e82">
<span data-ref-id="101" style="">
<span>&quot;Encounter with Atrial Ablation Procedure&quot;</span>
</span>
<span>union</span>
<span data-ref-id="115" style="background-color:white;color:black">
<span data-ref-id="115" style="background-color:#daeaf5;color:#004e82">
<span>&quot;History of Atrial FibrillationFlutter&quot;</span>
</span>
</span>
Expand All @@ -26,4 +26,11 @@ <h2>test Clause Coverage: 100%</h2>
</span>
</code>
</pre>
<pre style="tab-size: 2"
data-library-name="AnticoagulationTherapyforAtrialFibrillationFlutter" data-statement-name="Numerator">
<code>
<span data-ref-id="135" style="background-color:white;color:black"><span>define &quot;Numerator&quot;: </span><span data-ref-id="134" style=""><span><span data-ref-id="125" style=""><span data-ref-id="124" style=""><span><span>&quot;Denominator&quot;</span></span></span><span> NonElectiveEncounter</span></span></span><span>
</span><span data-ref-id="133" style=""><span>with </span><span data-ref-id="127" style=""><span data-ref-id="126" style=""><span><span>&quot;Anticoagulant Therapy at Discharge&quot;</span></span></span><span> Anticoagulant</span></span><span>
such that </span><span data-ref-id="132" style=""><span data-ref-id="129" style=""><span data-ref-id="128" style=""><span>Anticoagulant</span></span><span>.</span><span data-ref-id="129" style=""><span>authorDatetime</span></span></span><span> during </span><span data-ref-id="131" style=""><span data-ref-id="130" style=""><span>NonElectiveEncounter</span></span><span>.</span><span data-ref-id="131" style=""><span>relevantPeriod</span></span></span></span></span></span></span></code>
</pre>
</div>
41 changes: 24 additions & 17 deletions test/unit/fixtures/html/simpleCoverageAnnotation2.html
View file
Original file line number Diff line number Diff line change
@@ -1,29 +1,36 @@
<div>
<h2>test2 Clause Coverage: 100%</h2>
<h2>test2 Clause Coverage: 66.7%</h2>
<pre
style="tab-size: 2"
data-library-name="AnticoagulationTherapyforAtrialFibrillationFlutter"
data-statement-name="Denominator"
>
<code>
<span data-ref-id="119" style="background-color:#daeaf5;color:#004e82">
<span>define &quot;Denominator&quot;: </span>
<span data-ref-id="118" style="background-color:white;color:black">
<span data-ref-id="116" style="background-color:white;color:black">
<span data-ref-id="101" style="background-color:white;color:black">
<span>&quot;Encounter with Atrial Ablation Procedure&quot;</span>
</span>
<span>union</span>
<span data-ref-id="115" style="background-color:white;color:black">
<span>&quot;History of Atrial FibrillationFlutter&quot;</span>
</span>
<code>
<span data-ref-id="119" style="background-color:#daeaf5;color:#004e82">
<span>define &quot;Denominator&quot;: </span>
<span data-ref-id="118" style="background-color:#daeaf5;color:#004e82">
<span data-ref-id="116" style="background-color:#daeaf5;color:#004e82">
<span data-ref-id="101" style="">
<span>&quot;Encounter with Atrial Ablation Procedure&quot;</span>
</span>
<span>union</span>
<span data-ref-id="117" style="background-color:white;color:black">
<span>&quot;Current Diagnosis Atrial FibrillationFlutter&quot;</span>
<span data-ref-id="115" style="background-color:#daeaf5;color:#004e82">
<span>&quot;History of Atrial FibrillationFlutter&quot;</span>
</span>
</span>
<span>union</span>
<span data-ref-id="117" style="background-color:white;color:black">
<span>&quot;Current Diagnosis Atrial FibrillationFlutter&quot;</span>
</span>
</span>
</code>
</pre>
</span>
</code>
</pre>
<pre style="tab-size: 2"
data-library-name="AnticoagulationTherapyforAtrialFibrillationFlutter" data-statement-name="Numerator">
<code>
<span data-ref-id="135" style="background-color:white;color:black"><span>define &quot;Numerator&quot;: </span><span data-ref-id="134" style=""><span><span data-ref-id="125" style=""><span data-ref-id="124" style=""><span><span>&quot;Denominator&quot;</span></span></span><span> NonElectiveEncounter</span></span></span><span>
</span><span data-ref-id="133" style=""><span>with </span><span data-ref-id="127" style=""><span data-ref-id="126" style=""><span><span>&quot;Anticoagulant Therapy at Discharge&quot;</span></span></span><span> Anticoagulant</span></span><span>
such that </span><span data-ref-id="132" style=""><span data-ref-id="129" style=""><span data-ref-id="128" style=""><span>Anticoagulant</span></span><span>.</span><span data-ref-id="129" style=""><span>authorDatetime</span></span></span><span> during </span><span data-ref-id="131" style=""><span data-ref-id="130" style=""><span>NonElectiveEncounter</span></span><span>.</span><span data-ref-id="131" style=""><span>relevantPeriod</span></span></span></span></span></span></span></code>
</pre>
</div>
Loading

0 comments on commit 69dcd10

Please sign in to comment.