Skip to content

Concept codes comparison adjusted - #115

Merged
bryantaustin13 merged 6 commits into
mainfrom
fixConceptCodes
Aug 18, 2026
Merged

bryantaustin13 merged 6 commits into
mainfrom
fixConceptCodes

Conversation

@bryantaustin13

Copy link
Copy Markdown
Contributor

Server returning correct code, but test is failing due to format and test evaluation.

To duplicate the test that fails, run the following

<?xml version="1.0" encoding="utf-8"?>
<tests xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="http://hl7.org/fhirpath/tests" xsi:schemaLocation="http://hl7.org/fhirpath/tests ../../testSchema/testSchema.xsd"
       name="CqlTypeOperatorsTest">
	<group name="SingleTest" version="1.0">
		<test name="CodeToConcept1" version="1.0">
			<capability code="type-operators" />
			<expression>ToConcept(Code { code: '8480-6' })</expression>
			<output>Concept { codes: { Code { code: '8480-6' } } }</output>
		</test>
	</group>
</tests>

Moved from #100, which was raised from a fork before I had committer access on this repository. Same work, same branch name, now hosted here so CI and reviews run against cqframework directly.

This branch has moved on since #100: main is merged in, and it carries an additional fix. normalizeForComparison rebuilds objects via Object.entries, which does not see the non-enumerable Symbol property #110 uses to record a numeric interval's point type and quantity-precision. Rebuilding stripped that metadata, so intervalsEqual fell back to the decimal step and 11 comparison tests failed once #110 landed. The metadata is now carried across, with a regression test that fails without it. 185 passing, tsc --noEmit clean.

Two conflicts, both fallout from #109 landing on main.

src/shared/results-utils.ts: the conflict presents as a docblock, but
main's side carries the whole longEquals function (#106, Long returned
as valueString) and the auto-merged body below already calls it.
Taking either side alone breaks the build. Kept longEquals together
with this branch's expanded resultsEqual docblock. Concept comparison
normalization is unchanged.

src/test-results/cql-test-results.ts: #109 added a module-level
formatActualValue that duplicated CQLTestResults.formatActualValue and
its seven private statics. Resolved to one implementation: keep main's
(recursive lists, quantities, interval boundary recursion), fold in
this branch's Code and Concept rendering, and drop the duplicate
statics. Arrays of Codes now render as CQL rather than falling back to
JSON. Carried over the try/catch so a nested Long or a circular value
no longer throws, and exported the function so displayFixes converges
on the same one.
Both branches independently reworded this docblock and the
equalizeValueTypes comment, which made them conflict with each other for
no functional reason. Use wording that is accurate on either branch (no
enumeration of the specific shapes each one handles) so whichever lands
on main second merges cleanly.
normalizeForComparison rebuilds objects to collapse singleton Concept.codes
and drop undefined-valued keys, but it walks Object.entries, which does not
see the non-enumerable Symbol property the extractors use to record a
numeric interval's point type and quantity-precision (#110/FHIR-56226).
Rebuilding therefore stripped that metadata, intervalsEqual fell back to the
decimal step for every interval, and 11 comparison tests failed once main
was merged in.

Carry the metadata onto the rebuilt object. This forwards only what the wire
already declared, so it cannot create a false match. Covered by a regression
test that fails without the carry-forward, since the failure mode is
otherwise silent.

@raleigh-g-thompson raleigh-g-thompson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No comments. Looks good.

#113 landed the try/catch and export in formatActualValue, which this branch
also carries. The only real difference is this branch's Concept and Code
rendering, which main has no counterpart for, so git flagged the insertion
purely because it sits adjacent to the shared try/catch.

Resolved by keeping this branch's version of cql-test-results.ts, which is a
strict superset of main's: identical docblock, identical bigint/circular
guard, plus the isConceptShaped/isCodeShaped branches and their helpers.
Nothing from main is dropped.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants