Skip to content

fix(node): give every element of a struct-built array its own id - #202

Open
ehennestad wants to merge 1 commit into
mainfrom
fix-array-constructor-shared-ids
Open

ehennestad wants to merge 1 commit into
mainfrom
fix-array-constructor-shared-ids

Conversation

@ehennestad

Copy link
Copy Markdown
Collaborator

When openminds.Node builds an object array from a struct array it preallocates with obj(1, numel(instance)) = feval(class(obj)). MATLAB fills the elements in between with copies of one default object, so they are distinct handles that all carry the id that one object generated. fromStruct replaces the id only when the struct carries one, and the constructor's final loop only generates ids that are empty, so embedded nodes, which have no @id in a document, keep the shared id.

With four or more embedded values of one type, the parent property's mustBeListOfUniqueItems validator rejects the array and the deserializer drops the parent node with "Value must be an array of unique instances". Three or fewer values leave a single gap element and load; mixed-type arrays take a different path and load. Seen with a DatasetVersion holding seven embedded Contribution values, which openMINDS_MATLAB 0.12.0 dropped on load.

Right after the preallocation, every element now gets its own blank-node id. fromStruct still replaces it when the struct carries an identifier.

Two tests cover it. InstanceTest/testStructArrayWithoutIdsGetsDistinctIds builds four Hash values from id-less structs and checks four distinct ids in struct order. CollectionTest/testEmbeddedArrayOfFourSurvivesRoundTrip saves a File with four embedded Hash values and reloads it with four distinct ids and the digests in order. File and Hash are used because they are the same in v4 and v5, while the DatasetVersion property is otherContribution in v4 and contribution in v5.

🤖 Generated with Claude Code

The Node constructor preallocates the object array for a struct array
with obj(1, n) = feval(class(obj)). MATLAB fills the elements between
the first and the last with copies of one default object, so those
copies carry the same generated blank-node id. fromStruct assigns an id
only from at_id or x_id, which embedded values do not have, and the
final loop regenerates only empty ids. From four embedded values of one
type onwards, two or more therefore shared an id, the property holding
them failed mustBeListOfUniqueItems, and the deserializer dropped the
owning node with "Value must be an array of unique instances".

Generate an id for every element right after preallocation. fromStruct
still replaces it when the struct carries an identifier.

Tests: a File with four embedded Hash values survives a JSON-LD round
trip, and a Hash array built from four id-less structs has four
distinct blank-node ids.

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

Copy link
Copy Markdown
Contributor

Test Results (R2022a)

860 tests   - 260   856 ✅  - 246   5m 16s ⏱️ -25s
 27 suites ±  0     4 💤  -  14 
  1 files   ±  0     0 ❌ ±  0 

Results for commit 6626280. ± Comparison against base commit ba78412.

This pull request removes 262 and adds 2 tests. Note that renamed tests count towards both.
RoundTripTest ‑ testJsonLdRoundTrip(MetadataType=AccessEligibilityType)
RoundTripTest ‑ testJsonLdRoundTrip(MetadataType=AccessForm)
RoundTripTest ‑ testJsonLdRoundTrip(MetadataType=AccessProcessType)
RoundTripTest ‑ testJsonLdRoundTrip(MetadataType=Accessibility)
RoundTripTest ‑ testJsonLdRoundTrip(MetadataType=AccountInformation)
RoundTripTest ‑ testJsonLdRoundTrip(MetadataType=ActionStatusType)
RoundTripTest ‑ testJsonLdRoundTrip(MetadataType=Affiliation)
RoundTripTest ‑ testJsonLdRoundTrip(MetadataType=AgeCategory)
RoundTripTest ‑ testJsonLdRoundTrip(MetadataType=AmountOfChemical)
RoundTripTest ‑ testJsonLdRoundTrip(MetadataType=AnalysisTechnique)
…
CollectionTest ‑ testEmbeddedArrayOfFourSurvivesRoundTrip
InstanceTest ‑ testStructArrayWithoutIdsGetsDistinctIds

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (R2026a)

860 tests   - 260   858 ✅  - 245   5m 43s ⏱️ - 3m 11s
 27 suites ±  0     2 💤  -  14 
  1 files   ±  0     0 ❌ ±  0 

Results for commit 6626280. ± Comparison against base commit ba78412.

This pull request removes 262 and adds 2 tests. Note that renamed tests count towards both.
RoundTripTest ‑ testJsonLdRoundTrip(MetadataType=AccessEligibilityType)
RoundTripTest ‑ testJsonLdRoundTrip(MetadataType=AccessForm)
RoundTripTest ‑ testJsonLdRoundTrip(MetadataType=AccessProcessType)
RoundTripTest ‑ testJsonLdRoundTrip(MetadataType=Accessibility)
RoundTripTest ‑ testJsonLdRoundTrip(MetadataType=AccountInformation)
RoundTripTest ‑ testJsonLdRoundTrip(MetadataType=ActionStatusType)
RoundTripTest ‑ testJsonLdRoundTrip(MetadataType=Affiliation)
RoundTripTest ‑ testJsonLdRoundTrip(MetadataType=AgeCategory)
RoundTripTest ‑ testJsonLdRoundTrip(MetadataType=AmountOfChemical)
RoundTripTest ‑ testJsonLdRoundTrip(MetadataType=AnalysisTechnique)
…
CollectionTest ‑ testEmbeddedArrayOfFourSurvivesRoundTrip
InstanceTest ‑ testStructArrayWithoutIdsGetsDistinctIds

@codecov

codecov Bot commented Oct 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.11%. Comparing base (9030b75) to head (6626280).
⚠️ Report is 12 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #202      +/-   ##
==========================================
- Coverage   80.37%   80.11%   -0.27%     
==========================================
  Files         424      424              
  Lines        4403     4425      +22     
==========================================
+ Hits         3539     3545       +6     
- Misses        864      880      +16     

☔ 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.

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