Fix versioned extension checks#1630
Open
GinoCanessa wants to merge 9 commits into
Open
Conversation
Add canonicalsAreEqualIgnoringVersion(a, b) to compare two canonical URLs while ignoring a trailing |version on either side. Exact match is tried first so URLs containing a literal '|' still match when both sides carry the same literal; only then is the suspected trailing version (after the last '|') stripped from each side. Covered by a dedicated unit test suite. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e316878c-a7ba-4ef6-84f4-3af562f6765e
findMatchingSlice compared a slice's versioned type[0].profile (e.g. from hl7.fhir.uv.xver-* packages, '...|0.1.0') against the unversioned fished extension url with strict ===, so extensions assigned by unversioned canonical URL never bound to the named slice. validateRequiredChildElements then counted 0 occurrences and emitted a false "minimum cardinality 1 but occurs 0 time(s)" error (31 such errors on the Subscriptions R5 Backport IG) despite the extension being present in the output. Use canonicalsAreEqualIgnoringVersion for the comparison and guard against an empty type array. Add an InstanceExporter regression test covering a version-pinned required extension slice assigned by unversioned URL, plus a second instance to exercise the shared/cached StructureDefinition path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e316878c-a7ba-4ef6-84f4-3af562f6765e
The DuplicateSliceError branch of handleExtensionContainsRule compared the existing slice's type[0].profile against the unversioned extension url with strict ===, so a duplicate ContainsRule item whose type carries a |version produced a versioned profile that never matched, downgrading a harmless no-op into a false "conflicting duplicate" error. Use canonicalsAreEqualIgnoringVersion for the comparison. This also (by design) downgrades a same-slice-name, different-version duplicate from an error to a warning, since SUSHI keeps only the first profile per slice. Both behaviors are pinned by new gating tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e316878c-a7ba-4ef6-84f4-3af562f6765e
When findMatchingSlice binds an extension by unversioned canonical URL only after ignoring a |version pinned on the slice's type.profile, and that pinned version differs from the resolved extension's actual version, emit a debug-level note. This surfaces genuine version drift without warn/error noise (the author usually cannot change the unversioned instance url nor the version pinned in a published package). Silent when the versions agree or either is absent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e316878c-a7ba-4ef6-84f4-3af562f6765e
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e316878c-a7ba-4ef6-84f4-3af562f6765e
The instance "were extensions used correctly along the path?" validation fished the bare slice name to identify a slice's extension. Slice names are arbitrary profile-local identifiers and collide with unrelated published extensions' names (e.g. slice `type` vs. R4 core familymemberhistory-type, whose name is literally "type"), producing false "Non-modifier extension … used on modifierExtension element" errors. Resolve the extension from the matched slice's type.profile (version-insensitively) instead. Fixes both mirror-image checks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3ef4884a-3cd7-4903-a828-f6f73706f42e
Add two exporter-level regression tests covering both mirror directions: a modifierExtension slice named "type" (colliding with R4 core familymemberhistory-type) bound to a real modifier extension, and an extension slice named after a modifier decoy bound to a non-modifier extension. Each asserts correct JSON and no false "used on … element" error. Both fail on the pre-fix code and pass after the type.profile-based resolution fix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3ef4884a-3cd7-4903-a828-f6f73706f42e
Remove the personal inner-loop dev-* skills (dev-request, dev-plan, dev-do, dev-report, dev-review) from version control and revert the local-work scratch/ line from .gitignore, so this branch's shared config matches master. The skills remain on disk and usable locally; personal .git/info/exclude rules keep dev-* and scratch/ ignored. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: bcc74c6b-b553-46ff-b384-c018c971afca
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix false validation errors for versioned extension slices and colliding slice names
Summary
This branch fixes two related, high-impact false-positive validation errors that
cause SUSHI to exit non-zero ("shipwreck") on IGs that use cross-version
extension packages (
hl7.fhir.uv.xver-*), even though the generated FHIR JSON iscorrect. Both are surfaced by the FHIR Subscriptions R5 Backport IG
(
hl7.fhir.uv.subscriptions-backport), which today reports 31 + 12 = 43 falseerrors; with this change it validates cleanly.
Neither fix alters the generated FHIR JSON - they only remove false-positive
diagnostics from the required-element and extension-usage validators.
Motivation
When a profile is supplied by a cross-version extension package, its extension
slices pin a version on
type.profile(e.g.
…/extension-SubscriptionStatus.subscription|0.1.0), while instancesreference the extension by its unversioned canonical URL. SUSHI compared these
with exact string equality in a few places, and separately identified a slice's
extension by fishing the bare slice name. Both assumptions break for
cross-version IGs:
Versioned slice never matches (31 false errors). In
StructureDefinition.findMatchingSlice, a versionedtype.profile(
…|0.1.0) never===the unversioned resolved extension URL, so the assignedextension is never bound to its named slice.
validateRequiredChildElementsthen counts 0 occurrences and emits
Element … has minimum cardinality 1 but occurs 0 time(s).even though theelement is present in the output.
Slice name collides with an unrelated extension (12 false errors).
InstanceExporter's "were extensions used correctly along the path?" check
fished the bare slice name as an extension. A slice named
typeresolves toR4/R4B core
familymemberhistory-type(whosenameis literally"type", andwhich is a non-modifier), so a genuine modifier extension assigned via
modifierExtension[type]is falsely flaggedNon-modifier extension type used on modifierExtension element.(This becomesobservable once fix Find element by path #1 correctly rewrites the path to the profile slice name.)
Both patterns are FHIR-version-agnostic (R4 and R4B are equally affected) and the
IG author cannot fix them from the FSH side, because the offending version/slice
name comes from a published dependency package.
Changes
Version-insensitive canonical comparison
src/fhirtypes/common.ts- newcanonicalsAreEqualIgnoringVersion(a, b)helper. It compares the two values exactly as-presented first (so a URL that
legitimately contains a literal
|still matches when both sides carry the sameliteral), and only falls back to stripping the suspected trailing
|version(everything after the last
|) when the exact compare fails. This isdeliberately pipe-safe and intentionally differs from
FishingUtils, whichsplits on the first
|to peel off a fishing key; that path is left unchanged.Fix 1 - versioned extension-slice matching
src/fhirtypes/StructureDefinition.ts(findMatchingSlice) - match anextension slice by comparing
type.profileagainst the resolved extension URLwith the new helper instead of
===. When a match is achieved only byignoring a pinned version that differs from the resolved extension's actual
version, emit a debug-level note (no warn/error - the author can change
neither the unversioned instance URL nor the version pinned in a published
package). Stays silent when versions agree or either side is unversioned.
src/export/StructureDefinitionExporter.ts- when a duplicate extensionContainsRuleslice is version-pinned, compare with the helper so it is treatedas a harmless duplicate (warning, not error), matching the pre-existing
behavior for unversioned duplicates.
Fix 2 - slice-name / extension-name collision
src/export/InstanceExporter.ts- newresolveSliceExtension(...)resolvesthe Extension bound to a sliced
(modifier)extensionpath part via the matchedslice's
type.profile(fished version-insensitively) rather than by fishingthe bare slice name. Applied to both mirror-image checks (a non-modifier
flagged on a
modifierExtensionslice, and a modifier flagged on anextensionslice), since they share the same root cause. The reliable URL-based first check
is left untouched.
Testing
test/fhirtypes/canonicalsAreEqualIgnoringVersion.test.ts- 10 unit testsfor the helper: unversioned/versioned/mixed equality, differing versions,
differing base URLs, the literal-
|-in-URL exact-match path and its symmetricstrip limitation, and null/undefined handling (no throw).
test/fhirtypes/StructureDefinition.test.ts- a version-pinnedtype.profileslice matched via the unversioned URL, plus the debug drift notefiring on version mismatch and staying silent when versions agree.
test/export/StructureDefinitionExporter.test.ts- a version-pinnedduplicate
ContainsRuleslice is downgraded from error to warning.test/export/InstanceExporter.test.ts- no falseNon-modifier extension …/Modifier extension …errors when a slice namecollides with an unrelated extension's
name(both directions), and a requiredversion-pinned extension slice referenced by unversioned URL is assigned without
a false
minimum cardinality … occurs 0 time(s)error.All affected suites pass locally (
915passing across the four suites;10/10for the helper).
How to reproduce / verify against a real IG
Before:
31 Errors(min-cardinality), then12 Errors(non-modifier) once thefirst fix lands. After: 0 errors from these two causes, with byte-for-byte
identical generated JSON.
Compatibility & risk
removed. New diagnostics are debug-level only.
contain a literal
|are unaffected.findElementByPath, so it does not introduce new external fetches.Related
N/A (have not filed issues for this)
Pre-merge checklist
npm test -- <files>)npm run check(fulltest+lint+prettier) is cleanmasternpm auditshows no new vulnerabilities (no dependency changes in this PR)