Skip to content

Add test coverage for non-circular references in contained resources#608

Merged
andrzejskowronski merged 2 commits intodevelopfrom
copilot/fix-circular-reference-error
Oct 29, 2025
Merged

Add test coverage for non-circular references in contained resources#608
andrzejskowronski merged 2 commits intodevelopfrom
copilot/fix-circular-reference-error

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Oct 29, 2025 •
edited
Loading

The validator was incorrectly detecting circular references when one contained resource referenced another contained resource, even without an actual cycle. This was resolved in SDK v6.0.1 with reference equality for ScopedNodes.

Changes

  • Added NoCircularInContainedResources test to verify non-circular references between contained resources don't trigger false positives

The test validates this scenario now works correctly:

var pat = new Patient
{
    Id = "pat1",
    Contained =
    [
        new Patient { Id = "pat2a", Link = [new() { Other = new("#pat2b") }] },
        new Patient { Id = "pat2b" }
    ]
};
// Should validate successfully without circular reference error

This complements the existing CircularInContainedResources test which verifies actual cycles are still detected.

Original prompt

This section details on the original issue you should resolve

<issue_title>Validator throws circular reference error for any contained resource that references another contained resource</issue_title>
<issue_description>The Validator throws circular reference error when a contained resource references another contained resource, even when there is no cycle. This is because the internalReferenceNavEvent strips the index from the resolved reference, and so the validation logger cannot distinguish between contained[0] and contained[1]

to replicate:

[TestMethod]
public void NoCircularInContainedResources()
{
    // no circular in contained patients
    var pat = new
    {
        resourceType = "Patient",
        id = "pat1",
        contained = new[]
        {
            new
            {
                resourceType = "Patient",
                id = "pat2a",
                other = new { _type = "Reference", reference = "#pat2b" }
            },
            new
            {
                resourceType = "Patient",
                id = "pat2b",
                other = new { _type = "Reference", reference = "" }
            }
        }
    };

    var result = test(SCHEMA, pat.DictionaryToTypedElement("Patient"));
    result.IsSuccessful.Should().BeTrue();
    result.Evidence.Should().Contain(ass => (ass as IssueAssertion)!.IssueNumber == Issue.CONTENT_REFERENCE_CYCLE_DETECTED.Code); // Passes???
}
```</issue_description>

<agent_instructions>Try adding the unit test and see if it works</agent_instructions>

## Comments on the Issue (you are @copilot in this section)

<comments>
<comment_new><author>@mmsmits</author><body>
We will solve this when we have SDK version 6. 
Since we want to introduce reference equality for ScopedNodes</body></comment_new>
<comment_new><author>@mmsmits</author><body>
We will also need to undo building up the instance path during validation. 
(This is a lot of work).</body></comment_new>
<comment_new><author>@ewoutkramer</author><body>
@Kasdejong - has this been fixed? I think you have worked on this?</body></comment_new>
</comments>

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

@alexzautke
Add test for non-circular references in contained resources
f6c6dea 
Co-authored-by: alexzautke <548617+alexzautke@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix circular reference error in validator Add test coverage for non-circular references in contained resources Oct 29, 2025
Copilot AI requested a review from alexzautke October 29, 2025 12:34
@alexzautke alexzautke marked this pull request as ready for review October 29, 2025 12:40
@andrzejskowronski andrzejskowronski merged commit eb21d43 into develop Oct 29, 2025
2 checks passed
@andrzejskowronski andrzejskowronski deleted the copilot/fix-circular-reference-error branch October 29, 2025 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@andrzejskowronski andrzejskowronski andrzejskowronski approved these changes

@Kasdejong Kasdejong Awaiting requested review from Kasdejong

+1 more reviewer

@alexzautke alexzautke alexzautke approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@alexzautke alexzautke

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Validator throws circular reference error for any contained resource that references another contained resource

3 participants

@alexzautke @andrzejskowronski