StructureDefinitionCorrectionsResolver update by Rob5045 · Pull Request #632 · FirelyTeam/firely-validator-api

Rob5045 · 2026-02-06T13:38:08Z

Description

Updated StructureDefinitionCorrectionsResolver to include fix for R5 ImagingSelection and correctIdElement now also updates fhir type extension if it is present. Added separate projects for Firely.Fhir.Validation.Compilation for all fhir versions. Updated FirelySdkVersion to 5.13.2. Updated Newtonsoft.Json to version 13.0.4.

Testing

Added unit test StructureDefinitionCorrectionsResolverTests for all fhir versions. Probably need to update CompareToCorrectSchemaSnaps instead of this test, but I don't know how.

…ImagingSelection and correctIdElement now also updates fhir type extension if it is present. Added unit tests. Added separate projects for Firely.Fhir.Validation.Compilation for all fhir versions. Updated FirelySdkVersion to 5.13.2. Updated Newtonsoft.Json to version 13.0.4.

Copilot

Pull request overview

Updates StructureDefinitionCorrectionsResolver to apply additional R5 corrections (ImagingSelection) and to update the structuredefinition-fhir-type extension when correcting Resource.id, while restructuring Compilation into per-FHIR-version projects and bumping dependency versions.

Changes:

Extend StructureDefinitionCorrectionsResolver to correct R5 ImagingSelection.instance.sopClass and update structuredefinition-fhir-type extension values when fixing id.
Split Firely.Fhir.Validation.Compilation into version-specific projects (R4/R4B/R5) and update project/solution references accordingly.
Add resolver unit tests per FHIR version and bump Firely SDK / Newtonsoft.Json versions.

Reviewed changes

Copilot reviewed 26 out of 28 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/Firely.Fhir.Validation.Compilation.Shared/StructureDefinitionCorrectionsResolver.cs	Adds new corrections (R5 ImagingSelection) and updates `fhir-type` extension when fixing `id`; replaces array `.Contains` with a `HashSet`.
test/Firely.Fhir.Validation.Compilation.Tests.Shared/Support/ResourceIdCorrectionTestContext.cs	Adds shared test helper for validating `Resource.id` corrections across versions.
test/**/StructureDefinitionCorrectionsResolverTests.cs	Adds new tests for id correction (and ImagingSelection in R5).
src/Firely.Fhir.Validation/Firely.Fhir.Validation.csproj	Updates Newtonsoft.Json version and InternalsVisibleTo entries for new compilation projects.
src/Firely.Fhir.Validation.R4/Firely.Fhir.Validation.R4.csproj	Updates references to version-specific compilation projects.
src/Firely.Fhir.Validation.Compilation.R4/Firely.Fhir.Validation.Compilation.R4.csproj	Introduces new version-specific compilation projects and metadata.
src/Firely.Fhir.Validation.Compilation.R4/PublicAPI..txt	Adds PublicAPI analyzer baselines for new projects.
Firely.Validator.API.sln	Removes old compilation project and adds new version-specific compilation projects.
firely-validator-api*.props	Bumps Firely SDK version and updates copyright year.
test/.../fhirpkg.lock.json	Updates lock timestamp.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-06T13:48:08Z

src/Firely.Fhir.Validation.Compilation.Shared/StructureDefinitionCorrectionsResolver.cs

-                if (idElements.Count() == 1 && idElements.Single().Type.Count == 1)
+
+                var idElements = elements.Element.Where(e => Regex.IsMatch(e.Path!, @"^[a-zA-Z]+\.id$"));
+                if (idElements.SingleOrDefault()?.Type is { Count: 1 } singleTypeRef && singleTypeRef[0].Code != "id")


SingleOrDefault() will throw if there is more than one matching *.id element. The previous implementation only corrected when exactly one match existed; this change can crash at runtime for inputs with multiple matches. Consider materializing the matches and explicitly checking Count == 1 before accessing the element (e.g., var matches = idElements.Take(2).ToList(); if (matches.Count == 1 && matches[0].Type is { Count: 1 } ...) ...).

Suggested change

if (idElements.SingleOrDefault()?.Type is { Count: 1 } singleTypeRef && singleTypeRef[0].Code != "id")

var matches = idElements.Take(2).ToList();

if (matches.Count == 1 &&

matches[0].Type is { Count: 1 } singleTypeRef &&

singleTypeRef[0].Code != "id")

Copilot · 2026-02-06T13:48:09Z

test/Firely.Fhir.Validation.Compilation.Tests.Shared/Support/ResourceIdCorrectionTestContext.cs

+                    extension.Value.Should().BeOfType<FhirUrl>();
+
+                    var fhirUrl = (FhirUrl) extension.Value;
+
+                    fhirUrl.Value.Should().Be(expectedFhirTypeValue);


The structuredefinition-fhir-type extension’s value[x] is typically a uri in FHIR, which is commonly represented as FhirUri (not FhirUrl). These assertions/casts can fail even when the extension is correct. Prefer asserting against IValue<string> (or PrimitiveType) and comparing .Value, or accept both FhirUri and FhirUrl if needed by version-specific models.

Copilot · 2026-02-06T13:48:09Z

test/Firely.Fhir.Validation.Compilation.Tests.Shared/Support/ResourceIdCorrectionTestContext.cs

+            const string packageServer = "https://packages.simplifier.net";
+
+            PackageResolver = new FhirPackageSource(ModelInfo.ModelInspector, packageServer, packageNames);


These tests depend on a live external package server, which can make CI flaky (network outages, rate limits, server changes) and slow. Consider switching to locally restored packages/testdata (checked-in or cached via fhirpkg.lock.json + a local cache) or make packageServer injectable/configurable so CI can point to a stable internal mirror/cache.

@Rob5045 - is there a way to avoid this? Or do we really need a specific package from simplifier?

@ewoutkramer I don't know. I just copied this pattern from other unit tests.

Copilot · 2026-02-06T13:48:09Z

Firely.Validator.API.sln

+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Firely.Fhir.Validation.Compilation.R4", "src\Firely.Fhir.Validation.Compilation.R4\Firely.Fhir.Validation.Compilation.R4.csproj", "{A6269A54-25B4-1737-179A-F284C026E816}"
+EndProject
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Firely.Fhir.Validation.Compilation.R4B", "src\Firely.Fhir.Validation.Compilation.R4B\Firely.Fhir.Validation.Compilation.R4B.csproj", "{234EBC96-BE5E-A912-1C3D-4B28E4704227}"
+EndProject
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Firely.Fhir.Validation.Compilation.R5", "src\Firely.Fhir.Validation.Compilation.R5\Firely.Fhir.Validation.Compilation.R5.csproj", "{912D0907-75AD-4D17-02DF-22494D04C547}"
+EndProject


These new projects are SDK-style (<Project Sdk="Microsoft.NET.Sdk">) but use the legacy C# project type GUID ({FAE04EC0-...}). Most SDK-style projects in the solution use {9A19103F-...}; mixing GUIDs can cause inconsistent behavior in some tooling. Consider updating these entries to the SDK-style project type GUID for consistency and compatibility.

Copilot · 2026-02-06T13:48:10Z

src/Firely.Fhir.Validation.Compilation.R5/PublicAPI.Shipped.txt

@@ -0,0 +1 @@
+


PublicAPI.Shipped.txt contains only a BOM/empty content. PublicApiAnalyzers typically expects consistent headers (often #nullable enable) across shipped/unshipped files, and an empty shipped file can cause baseline maintenance issues. Consider adding the standard header (and keeping the file intentionally empty otherwise) to reduce analyzer/baseline friction.

Suggested change

#nullable enable

// Public API baseline for Firely.Fhir.Validation.Compilation.R5 (intentionally empty).

ewoutkramer · 2026-02-06T14:21:29Z

I don't really want a compiler per version of FHIR, it was by design a "R4+" compiler, so we don't need to manage multiple codebases again. StructureDefinition (that we are using here) is the same for R4+, so we didn't really need it so far. Is it really necessary to add this complexity?

Rob5045 · 2026-02-08T17:54:44Z

I don't really want a compiler per version of FHIR, it was by design a "R4+" compiler, so we don't need to manage multiple codebases again. StructureDefinition (that we are using here) is the same for R4+, so we didn't really need it so far. Is it really necessary to add this complexity?

@ewoutkramer OK that is clear. I'm happy to revert my changes but can you then tell me what to do with the specific code sections for R4B and R5 (opd-3 and bdl-8) in StructureDefinitionCorrectionsResolver? I'm guessing you don't want to add a ModelInspector parameter to the ctor to have runtime FhirRelease checks.

Rob5045 · 2026-02-09T11:44:42Z

Also correctConstraints checks multiple resource types which seems inefficient to me. Why not have a correction per resource type?

…meter to StructureDefinitionCorrectionsResolver and StructureDefinitionToElementSchemaResolver.

ewoutkramer · 2026-02-11T14:57:33Z

@Rob5045 and I discussed this at the office - new updates to this PR will come.

…emaResolver and StructureDefinitionCorrectionsResolver ctor. Use sd.FhirVersion instead. Added ImagingSelection.json schema snap.

ewoutkramer · 2026-02-16T10:52:26Z

@Rob5045 - PR is still draft?

Rob5045 requested a review from Kasdejong February 6, 2026 13:38

alexzautke requested review from andrzejskowronski, Copilot and ewoutkramer February 6, 2026 13:46

Copilot AI reviewed Feb 6, 2026

View reviewed changes

Restoed Firely.Fhir.Validation.Compilation. Added ModelInspector para…

91b49f1

…meter to StructureDefinitionCorrectionsResolver and StructureDefinitionToElementSchemaResolver.

ewoutkramer marked this pull request as draft February 11, 2026 14:57

Removed ModelInspector parameter from StructureDefinitionToElementSch…

25994fb

…emaResolver and StructureDefinitionCorrectionsResolver ctor. Use sd.FhirVersion instead. Added ImagingSelection.json schema snap.

ewoutkramer approved these changes Feb 16, 2026

View reviewed changes

-                if (idElements.SingleOrDefault()?.Type is { Count: 1 } singleTypeRef && singleTypeRef[0].Code != "id")
+                var matches = idElements.Take(2).ToList();
+                if (matches.Count == 1 &&
+                    matches[0].Type is { Count: 1 } singleTypeRef &&
+                    singleTypeRef[0].Code != "id")

		const string packageServer = "https://packages.simplifier.net";

		PackageResolver = new FhirPackageSource(ModelInfo.ModelInspector, packageServer, packageNames);


	#nullable enable
	// Public API baseline for Firely.Fhir.Validation.Compilation.R5 (intentionally empty).

		@@ -0,0 +1 @@
		No newline at end of file

Conversation

Rob5045 commented Feb 6, 2026

Description

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

ewoutkramer Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Rob5045 Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

ewoutkramer commented Feb 6, 2026

Uh oh!

Rob5045 commented Feb 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Rob5045 commented Feb 9, 2026

Uh oh!

ewoutkramer commented Feb 11, 2026

Uh oh!

ewoutkramer commented Feb 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Rob5045 commented Feb 8, 2026 •

edited

Loading