Skip to content

refactor: add a JSON-LD deserializer symmetric with the serializer - #110

Open
ehennestad wants to merge 2 commits into
fix-controlled-term-array-constructionfrom
add-jsonld-deserializer
Open

refactor: add a JSON-LD deserializer symmetric with the serializer#110
ehennestad wants to merge 2 commits into
fix-controlled-term-array-constructionfrom
add-jsonld-deserializer

Conversation

@ehennestad

@ehennestad ehennestad commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Serialization had an architecture; deserialization had a function. This adds the missing half.

What loadInstances was doing

A single 144-line function read the files, stripped a hardcoded vocabulary prefix, dispatched each node to a type, built the instances, and then wired the references between them with a hand-rolled recursive traversal. That traversal had no cycle detection, no extension point for another format, and no way to configure any of it.

BaseDeserializer

The counterpart to BaseSerializer. Subclasses implement one method:

rawStructs = parseToStructs(obj, data)

The base class owns what does not depend on the format: dispatching each node to its type, constructing the instances, and wiring the links. JsonLdDeserializer implements the JSON-LD case and accepts several documents in one call, which is what loading a folder of one-file-per-instance needs.

Link wiring moves onto the traversal core

LinkWiringVisitor replaces the recursive resolveLinks local function. Building it on BaseVisitor means it inherits cycle detection and positional replacement rather than reimplementing traversal for the third time in the codebase.

Reporting instead of dropping

A node that could not be turned into an instance was previously handled like this:

catch ME
    warning(ME.message)
end

That loses the error identifier, makes the warning unsuppressable by identifier, garbles the message if it contains a %, and produces one warning per node with no indication of which node was lost. A node with no @type was skipped even more quietly, under a continue % Todo: Why skip?.

Unreadable nodes are now collected and reported once, by identifier, under openMINDS:Deserializer:UnreadableNodes, so it is possible to tell how much of a document did not survive. A caller that cannot work with a partial result can construct the deserializer with UnreadableNodePolicy = "error".

Resolving the type is deliberately left outside that handling. A type in an unknown namespace means the whole document was written for a different model version. Treating that as a per-node problem would hand the caller an empty collection and a warning where a hard failure is more useful, and the fixture test for legacy namespace documents asserts exactly that.

Result

loadInstances is now file reading and deserializer selection, 48 lines instead of 144.

Tests

New DeserializerTest covers reading a collection document, wiring links between nodes, wiring links across separate documents, terminating on a circular document, reporting an untyped node while still returning the readable ones, and the error policy.

Not addressed here

jsonld2struct still strips a hardcoded openminds.ebrains.eu vocabulary prefix, which is wrong for documents written in expanded form under a v4 model. That belongs with the version-tolerant loading work later in this stack.

🤖 Generated with Claude Code

Serialization had an architecture and deserialization had a function.
loadInstances read files, stripped a hardcoded vocabulary prefix,
dispatched types, built instances and wired links between them, with a
hand-rolled traversal that had no cycle detection and no extension point
for another format.

BaseDeserializer is the counterpart to BaseSerializer. Subclasses
implement parseToStructs for one format; the base class handles type
dispatch, instance construction and link wiring. JsonLdDeserializer
implements the JSON-LD case and accepts several documents at once.

Link wiring moves to LinkWiringVisitor, built on the traversal core
introduced earlier in this stack, so it inherits cycle detection and
positional replacement rather than reimplementing traversal.

A node that cannot be turned into an instance was reported with
warning(ME.message), one warning per node, which lost the error
identifier and told the caller nothing about which node was lost or how
much of the document was missing. Such nodes are now collected and
reported once, by identifier, under an identified warning. Callers that
cannot use a partial result can ask for an error instead.

Resolving the type stays outside that handling. A type in an unknown
namespace means the document was written for a different model version,
which is a document-level problem, and reporting it per node would leave
the caller with an empty result and a warning rather than a failure.

loadInstances is now file reading and deserializer selection, and goes
from 144 lines to 48.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Test Results (R2022a)

756 tests  +6   754 ✅ +6   2m 30s ⏱️ -10s
 20 suites +1     1 💤 ±0 
  1 files   ±0     1 ❌ ±0 

For more details on these failures, see this check.

Results for commit d7b0925. ± Comparison against base commit a5ac5dd.

♻️ This comment has been updated with latest results.

@ehennestad
ehennestad force-pushed the add-jsonld-deserializer branch from fa853e0 to d7b0925 Compare August 28, 2026 01:00
@codecov

codecov Bot commented Aug 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 82.05128% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.28%. Comparing base (a5ac5dd) to head (d7b0925).

Files with missing lines Patch % Lines
...openminds/+internal/+serializer/BaseDeserializer.m 80.00% 8 Missing ⚠️
...ternal/+openminds/+internal/+store/loadInstances.m 69.23% 4 Missing ⚠️
...penminds/+internal/+serializer/LinkWiringVisitor.m 88.23% 2 Missing ⚠️
Additional details and impacted files
@@                            Coverage Diff                             @@
##           fix-controlled-term-array-construction     #110      +/-   ##
==========================================================================
+ Coverage                                   79.20%   79.28%   +0.08%     
==========================================================================
  Files                                         419      422       +3     
  Lines                                        4077     4089      +12     
==========================================================================
+ Hits                                         3229     3242      +13     
+ Misses                                        848      847       -1     

☔ 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 force-pushed the add-jsonld-deserializer branch 2 times, most recently from 34f1334 to d7b0925 Compare August 28, 2026 12:59
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