Skip to content

fix: take an identifier-only mixed-type link from the instance library - #196

Open
ehennestad wants to merge 1 commit into
mainfrom
fix-serialize-mixed-type-reference-as-link
Open

ehennestad wants to merge 1 commit into
mainfrom
fix-serialize-mixed-type-reference-as-link

Conversation

@ehennestad

Copy link
Copy Markdown
Collaborator

Fixes the R2026a test failure in run 34695830401: RoundTripTest/testJsonLdRoundTrip(MetadataType=AnatomicalAtlasVersion) errored with OPENMINDS_MATLAB:MetaTypeRegistry:InvalidKey because the serializer was handed an openminds.internal.MixedTypeReference as if it were a node.

Cause

A property that holds several types stored a bare {"@id": ...} as a MixedTypeReference, whatever the identifier named. When it named a controlled instance, Collection.addNode treated it as a controlled instance and made it a node, and saving failed because a reference of unknown type is not a metadata type. The regenerated toolbox from openMetadataInitiative/openMINDS_MATLAB_pipeline#193 exposed this for AnatomicalAtlasVersion: its linked types gained HasControlledInstance, so the round-trip synthesizer now fills them from real library documents, one of which links usageCondition to a licence by @id. The same defect was already recorded in knownRoundTripGap for ParcellationTerminologyVersion and QuantitativeRelationAssessment.

Controlled terms built from a bare identifier, and IRI strings assigned to mixed-type properties, are already taken from the instance library at construction. A struct carrying only a library IRI was the one input that was not.

Change

  • MixedTypeSet: an identifier-only link whose identifier is a controlled instance IRI is taken from the library with openminds.instanceFromIRI and checked against the allowed types. Any other identifier stays a MixedTypeReference. So does a library lookup that fails, with a warning carrying the cause; the library does not always hold what an IRI points to, for example an IRI spelling MNIColin27_1998 where the file is MNI-Colin27_1998.
  • Serializer.enqueueDocument: a reference is never queued as a document of its own, at any recursion depth. This also fixes Node.serialize() on a node holding a mixed-type reference, which failed the same way at the default recursion depth.
  • knownRoundTripGap: ParcellationTerminologyVersion and QuantitativeRelationAssessment are removed, since both now round-trip.
  • Tests: library lookup from an identifier-only link, the fallback with its warning, and a reference getting no document of its own at recursion depth 2.

Not changed

  • Typed references to library instances still become stub nodes of a collection and are written as {"@type", "@id"} documents. That is round-trip stable and out of scope here, but in the atlas case it is 1339 near-empty documents out of 1365 and deserves its own issue.
  • The R2022a job cannot exercise these paths because the synthesizer caches names in a dictionary, which that release lacks; the call fails inside a try/catch and the affected properties are silently skipped. Worth a separate fix so the R2022a run stops passing vacuously.

🤖 Generated with Claude Code

A property that holds several types held a bare @id as a MixedTypeReference,
whatever the identifier named. When it named a controlled instance, the
collection added it as a node, and saving failed because a reference of
unknown type is not a metadata type. Controlled terms and IRI strings in the
same properties are already taken from the library at construction; a struct
carrying only a library IRI now is too, and a lookup the library cannot serve
keeps the link as a reference and says why.

The serializer no longer queues a reference as a document of its own, at any
recursion depth. That closes the same failure for Node.serialize.

ParcellationTerminologyVersion and QuantitativeRelationAssessment leave the
known round-trip gaps, since the reason they were listed is gone.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Test Results (R2022a)

861 tests   857 ✅  5m 3s ⏱️
 27 suites    4 💤
  1 files      0 ❌

Results for commit b5bc40f.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (R2026a)

861 tests   859 ✅  4m 30s ⏱️
 27 suites    2 💤
  1 files      0 ❌

Results for commit b5bc40f.

@codecov

codecov Bot commented Sep 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 80.25%. Comparing base (9030b75) to head (b5bc40f).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
code/+openminds/+base/MixedTypeSet.m 92.30% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #196      +/-   ##
==========================================
- Coverage   80.37%   80.25%   -0.13%     
==========================================
  Files         424      424              
  Lines        4403     4436      +33     
==========================================
+ Hits         3539     3560      +21     
- Misses        864      876      +12     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ehennestad
ehennestad added this pull request to stack #201 September 19, 2026 16:29
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.

1 participant