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.

Sign up for GitHub

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

Relationship alias coverage fixes #317

Merged
merged 6 commits into from
Nov 1, 2024
Merged

Relationship alias coverage fixes #317

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f0a9961
Initial incomplete fix for relationship alias coverage
hossenlopp Oct 22, 2024
a871fc0
Added code to clause results helpers to find the relationship express…
hossenlopp Oct 28, 2024
335baf9
reworked html generation tests to use input more reflective of actual…
hossenlopp Oct 31, 2024
c052ea3
Update comments for clarity
lmd59 Oct 31, 2024
cf8fbda
Check for undefined alias id and update iterator variable
lmd59 Oct 31, 2024
5769ba7
Added UNHIT results to the HTMLBuilder Tests
hossenlopp Oct 31, 2024
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
6 changes: 3 additions & 3 deletions src/calculation/HTMLBuilder.ts
View file
Open in desktop
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
Open in desktop
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 @@
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 @@
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) {
lmd59 marked this conversation as resolved.
Show resolved Hide resolved
const id = findRelationshipAliasAnnotationId(annotation, statement.expression.localId);
if (id) {
alId = id;
}
}
} else {
alId = statement.localId;
}
Expand Down Expand Up @@ -347,6 +352,78 @@
}
}

/**
* 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) {

Check warning on line 413 in src/helpers/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 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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading