Skip to content

feat: make serialization options reach the serializer, and add PropertyFilter - #147

Open
ehennestad wants to merge 1 commit into
unhide-isreferencefrom
serializer-property-filter
Open

feat: make serialization options reach the serializer, and add PropertyFilter#147
ehennestad wants to merge 1 commit into
unhide-isreferencefrom
serializer-property-filter

Conversation

@ehennestad

Copy link
Copy Markdown
Collaborator

Why

kg-sync subclasses the JSON-LD serializer to do one thing: keep only a named set of properties in each emitted document, for targeted patch operations. That is a general serialization capability rather than a Knowledge Graph concept, so it belongs in the configuration where any consumer can reach it, and the subclass can go.

What

Adds PropertyFilter as a serialization option. Filtering runs before property names are rewritten, while the fields are still plain property names, and the identifier and type keys survive it, so a filtered document still identifies the node it describes and a linked node still serializes as a valid reference.

Then lets Node.serialize build its serializer from forwarded options, which is what the method's own comment and Todo already said it should do.

Bug fixed on the way

serialize accepted RecursionDepth and IncludeIdentifier and passed neither to the serializer it called, so both were silently ignored. Nothing in the toolbox or its tests relied on them.

Passing a Serializer and passing options are now mutually exclusive and say so, since a serializer already carries its own configuration and silently ignoring one or the other is how the previous bug read.

🤖 Generated with Claude Code

@ehennestad
ehennestad force-pushed the serializer-property-filter branch 2 times, most recently from 8747285 to 8d27665 Compare September 4, 2026 21:01
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Test Results (R2022a)

803 tests  ±0   802 ✅ ±0   2m 36s ⏱️ -34s
 22 suites ±0     1 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit 4ea3ba6. ± Comparison against base commit 5bb6452.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Test Results (R2026a)

803 tests   802 ✅  3m 10s ⏱️
 22 suites    1 💤
  1 files      0 ❌

Results for commit 4ea3ba6.

♻️ This comment has been updated with latest results.

@codecov

codecov Bot commented Sep 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 56.52174% with 10 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (unhide-isreference@5bb6452). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...openminds/+internal/+serializer/JsonLdSerializer.m 44.44% 5 Missing ⚠️
code/+openminds/Node.m 64.28% 5 Missing ⚠️
Additional details and impacted files
@@                  Coverage Diff                  @@
##             unhide-isreference     #147   +/-   ##
=====================================================
  Coverage                      ?   80.94%           
=====================================================
  Files                         ?      422           
  Lines                         ?     4224           
  Branches                      ?        0           
=====================================================
  Hits                          ?     3419           
  Misses                        ?      805           
  Partials                      ?        0           

☔ 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 serializer-property-filter branch from 8d27665 to 321c3d0 Compare September 4, 2026 21:10
…tyFilter

kg-sync subclasses the JSON-LD serializer to do one thing: keep only a named
set of properties in each emitted document, for targeted patch operations.
That is a general serialization capability rather than a Knowledge Graph
concept, so it belongs in the configuration where any consumer can reach it,
and the subclass can go.

Add PropertyFilter as a serialization option. Filtering runs before property
names are rewritten, while the fields are still plain property names, and the
identifier and type keys survive it, so a filtered document still identifies
the node it describes and a linked node still serializes as a valid reference.

Then let Node.serialize build its serializer from forwarded options, which is
what the method's own comment and Todo already said it should do. This fixes
a bug rather than only finishing a design: serialize accepted RecursionDepth
and IncludeIdentifier and passed neither to the serializer it called, so both
were silently ignored. Nothing in the toolbox or its tests relied on them.

Passing a Serializer and passing options are now mutually exclusive and say
so, since a serializer already carries its own configuration and silently
ignoring one or the other is how the previous bug read.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@ehennestad
ehennestad force-pushed the serializer-property-filter branch from 321c3d0 to 4ea3ba6 Compare September 5, 2026 05:00
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