Repository navigation
Do not fail schema compilation on a superfluous slice - #694
Merged
Merged
Conversation
A slice whose discriminator(s) always succeed (or always fail) is superfluous, but not wrong: the SliceValidator handles such a condition correctly. Throwing an IncorrectElementDefinitionException here prevented the whole profile from being compiled. Drop the exception (and the message's missing space). Adds a test with a profile discriminator on an element without profiles. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J2SNRG6mrRVURXPQCPiau5
ewoutkramer
approved these changes
Oct 1, 2026
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The focused behavior change is consistent with SliceValidator semantics and has regression coverage.
Review effort: Balanced
Findings: None
What changed in this PR
Removes schema compilation failure for superfluous slices with fixed discriminator results.
Changes:
- Allows fixed-result slice conditions to compile.
- Adds an R4 regression test for an always-successful discriminator.
| File | Description |
|---|---|
SchemaBuilder.cs |
Stops rejecting fixed-result slice conditions. |
SuperfluousSliceTests.cs |
Verifies successful compilation and validation. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
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.
Requested by Ewout Kramer · Slack thread
Description
SchemaBuilderthrew anIncorrectElementDefinitionExceptionwhen the discriminator condition of a slice always succeeds (or always fails), which stopped the whole profile from being compiled. Such a slice is superfluous but not wrong (see the discussion in the issue), and theSliceValidatorhandles a fixed-result condition correctly as it is, so the exception is removed.The issue asks for a warning instead of silence; the
SchemaBuilderhas no warning mechanism yet (see #430), so for now the slice is simply compiled without complaint.Related issues
Closes #662
Testing
SuperfluousSliceTests(R4 compilation tests): a profile slicingPatient.namewith aprofilediscriminator on$this(no profiles on the element, so the condition always succeeds). Fails before the change with the error from the issue ("...cannot be used as a slicing discriminator"), passes after.Firely.Fhir.Validation.Compilation.R4.Tests: all pass except 3 tests that need the uninitializedFhirTestCasesgit submodule (RoundTripTest,RunFirelySdkTests,RunSingleTest); these fail identically without this change.diagnosticReport-eu-epsprofile from the issue (attachment not reachable from this environment).🤖 Generated with Claude Code
https://claude.ai/code/session_01J2SNRG6mrRVURXPQCPiau5
Generated by Claude Code