fix: decode controlled instance files through the shared JSON-LD path - #119
Open
ehennestad wants to merge 1 commit into
Open
fix: decode controlled instance files through the shared JSON-LD path#119ehennestad wants to merge 1 commit into
ehennestad wants to merge 1 commit into
Conversation
Contributor
ehennestad
force-pushed
the
unify-controlled-instance-reading
branch
from
August 31, 2026 10:40
039ed01 to
a4dfbb9
Compare
Contributor
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## add-external-integration-test #119 +/- ##
================================================================
Coverage ? 80.03%
================================================================
Files ? 422
Lines ? 4067
Branches ? 0
================================================================
Hits ? 3255
Misses ? 812
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
ehennestad
force-pushed
the
unify-controlled-instance-reading
branch
from
September 1, 2026 14:31
a4dfbb9 to
416ce9b
Compare
Three readers load controlled instance files from the local library. One of them decoded through the shared JSON-LD utility; the other two, InstanceResolver.resolveNode and HasControlledInstance.fromName, used raw jsondecode, which mangles @id into x_id. Everything downstream of those two readers therefore leaned on x_id fallback branches rather than the at_-form keywords every other decoded document carries. All three now decode through openminds.internal.utility.json.decode, so no code inside this repository produces x_-form structs any more. The offline reader also doubled every apostrophe inside the JSON text before decoding, a leftover from quoting for some earlier evaluation context. A definition written as 'associative array' came back as ''associative array'', and because the online reader did not do this, the same instance decoded differently depending on where the file came from. The replacement is removed. The x_id fallback branches themselves are deliberately kept. They are not dead compatibility code: openminds-kg-sync converts Knowledge Graph payloads with raw jsondecode and hands x_-form structs into openMINDS types, so the branches are the contract that integration relies on. They can only be removed after kg-sync normalizes its payloads to at_-form, which belongs to the kg-sync migration, not to this stack. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ehennestad
force-pushed
the
unify-controlled-instance-reading
branch
from
September 1, 2026 20:58
416ce9b to
39b42fc
Compare
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
1 participant
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.
The last manual read paths inside the repository, found during the external-integration verification two PRs down.
Two readers decoded with raw jsondecode
Three places load controlled instance files from the local library.
getControlledInstance(used byControlledTermBase) already decoded through the shared JSON-LD utility, butInstanceResolver.resolveNodeandHasControlledInstance.fromNameused rawjsondecode, which mangles"@id"into the field namex_id. Everything downstream of those two readers therefore leaned onx_idfallback branches instead of theat_-form keywords every other decoded document carries.All three readers now decode through
openminds.internal.utility.json.decode. No code inside this repository producesx_-form structs any more.The offline reader corrupted apostrophes
Doubling apostrophes inside JSON text is quoting for an evaluation context that no longer exists; applied before
jsondecodeit simply corrupts values. A definition written in the file as'associative array'came back as''associative array''— visible today onopenminds.controlledterms.DataType("associativeArray"). The online reader did not do this, so the same instance decoded differently depending on whether the file came from disk or from GitHub. The line is removed.Why the x_id fallback branches stay
I planned to remove them with this change, and that plan was wrong. They are not dead compatibility code: openminds-kg-sync's
convertKgNodedecodes Knowledge Graph payloads with rawjsondecodeand handsx_-form structs into openMINDS types — the branches are the contract that integration currently relies on. They can be removed only after kg-sync normalizes its payloads toat_-form (or decodes through the shared utility), which belongs to the kg-sync migration alongside the resolver rename from #108, not to this stack.Tests
Three cases in
ControlledInstanceTest: apostrophes in definitions are not doubled; an instance resolved from an IRI keeps its identifier and carries the file's content; an instance built by name through the mixin gets its identifier from the library file.🤖 Generated with Claude Code