From 1165bda549fbc3e25c8acb3c12b524150546f831 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20B=C3=BCrgi?= Date: Mon, 29 Jun 2026 16:12:22 +0200 Subject: [PATCH 1/2] fix(sync): JsonElement value-equality in EntityUpdate dedup + MeshNode reduce MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two JsonElement-as-object defects in the sync core: FIX 1 — JsonSynchronizationStream.ToEntityUpdates (both overloads): DistinctBy(x => new { x.Id, x.Collection }) never deduped. x.Id is the result of DecodePointerSegment → JsonSerializer.Deserialize → a boxed JsonElement, whose struct (_parent/_idx cursor) has no value equality, so the anonymous key compared every id as distinct and duplicate per-entity EntityUpdates leaked into the stream (contradicting the documented "one distinct update per affected entity"). Key on je.GetRawText() (stable; ToString() stays stable for string/guid ids). FIX 2 — MeshDataSource.ReduceToMeshNode patch path: `change.Value as MeshNode` returned null when change.Value is a JsonElement (the cross-hub / mirror patch path), pushing a null-content ChangeItem into the mirror. Deserialize the JsonElement to MeshNode the same way the sibling PatchMeshNode does (threaded the hub's JsonSerializerOptions into the reducer). Repros: EntityUpdateDedupTest (count 2 → 1), ReduceToMeshNodeJsonElementTest (null → node). Full Data.Test (254) + Graph.Test (436) green under DOTNET_PROCESSOR_COUNT=2. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../JsonSynchronizationStream.cs | 4 +- src/MeshWeaver.Graph/MeshDataSource.cs | 21 ++++++-- .../EntityUpdateDedupTest.cs | 52 +++++++++++++++++++ .../ReduceToMeshNodeJsonElementTest.cs | 47 +++++++++++++++++ 4 files changed, 118 insertions(+), 6 deletions(-) create mode 100644 test/MeshWeaver.Data.Test/EntityUpdateDedupTest.cs create mode 100644 test/MeshWeaver.Graph.Test/ReduceToMeshNodeJsonElementTest.cs diff --git a/src/MeshWeaver.Data/Serialization/JsonSynchronizationStream.cs b/src/MeshWeaver.Data/Serialization/JsonSynchronizationStream.cs index 936307842d..366f252b0e 100644 --- a/src/MeshWeaver.Data/Serialization/JsonSynchronizationStream.cs +++ b/src/MeshWeaver.Data/Serialization/JsonSynchronizationStream.cs @@ -745,7 +745,7 @@ public static IReadOnlyCollection ToEntityUpdates( ) { OldValue = pointer.Evaluate(current) }; }) - .DistinctBy(x => new { x.Id, x.Collection }) + .DistinctBy(x => new { Id = x.Id is JsonElement je ? je.GetRawText() : x.Id?.ToString(), x.Collection }) .ToArray(); internal static (JsonElement, JsonPatch) UpdateJsonElement(this TChange request, JsonElement? currentJson, JsonSerializerOptions options) where TChange : JsonChange @@ -991,7 +991,7 @@ public static IReadOnlyCollection ToEntityUpdates( ) { OldValue = idSegment == null ? current.Instances : current.Instances.GetValueOrDefault(idSegment) }; }) - .DistinctBy(x => new { x.Id, x.Collection }) + .DistinctBy(x => new { Id = x.Id is JsonElement je ? je.GetRawText() : x.Id?.ToString(), x.Collection }) .ToArray(); internal static (InstanceCollection, JsonPatch) UpdateJsonElement(this DataChangedEvent request, InstanceCollection current, JsonSerializerOptions options) diff --git a/src/MeshWeaver.Graph/MeshDataSource.cs b/src/MeshWeaver.Graph/MeshDataSource.cs index 758eb79d9e..b9ba72e06e 100644 --- a/src/MeshWeaver.Graph/MeshDataSource.cs +++ b/src/MeshWeaver.Graph/MeshDataSource.cs @@ -101,7 +101,9 @@ public static MessageHubConfiguration AddMeshDataSource( return data .Configure(rm => rm .ForReducedStream(reduced => reduced - .AddWorkspaceReference(ReduceToMeshNode)) + .AddWorkspaceReference( + (ci, r, initial) => ReduceToMeshNode( + ci, r, initial, data.Workspace.Hub.JsonSerializerOptions))) .ForReducedStream(reduced => reduced .AddPatchFunction(PatchMeshNode)) .AddWorkspaceReferenceStream( @@ -1125,8 +1127,9 @@ private static IMessageDelivery HandleRunTests( /// stale snapshot) and instances to bind to the wrong assembly. /// /// - private static ChangeItem ReduceToMeshNode( - ChangeItem current, MeshNodeReference reference, bool initial) + internal static ChangeItem ReduceToMeshNode( + ChangeItem current, MeshNodeReference reference, bool initial, + JsonSerializerOptions options) { var instances = current.Value?.Instances.Values.OfType(); var node = !string.IsNullOrEmpty(reference.Path) @@ -1152,7 +1155,17 @@ u.Value is MeshNode m // returning null (which silently drops the emission and blocks live updates). return new(node, current.StreamId, current.Version); } - return new(change.Value as MeshNode, current.ChangedBy, current.StreamId, + // change.Value is a JsonElement when the update was derived from a JSON + // patch (cross-hub / mirror path) — `as MeshNode` would silently null it + // out, dropping a null-content ChangeItem into the mirror. Deserialize it + // the same way the sibling PatchMeshNode does. + var changedNode = change.Value switch + { + MeshNode m => m, + JsonElement je => je.Deserialize(options), + _ => null + }; + return new(changedNode, current.ChangedBy, current.StreamId, ChangeType.Patch, current.Version, [change]); } diff --git a/test/MeshWeaver.Data.Test/EntityUpdateDedupTest.cs b/test/MeshWeaver.Data.Test/EntityUpdateDedupTest.cs new file mode 100644 index 0000000000..5c18f55b39 --- /dev/null +++ b/test/MeshWeaver.Data.Test/EntityUpdateDedupTest.cs @@ -0,0 +1,52 @@ +using System.Linq; +using System.Text.Json; +using Json.Patch; +using MeshWeaver.Data.Serialization; +using Xunit; + +namespace MeshWeaver.Data.Test; + +/// +/// Deterministic repro + spec for . +/// +/// Two patch operations that touch DIFFERENT fields of the SAME entity must collapse into a +/// SINGLE (the method's contract: "one distinct update per affected +/// entity, deduplicated by id and collection"). The id is decoded via +/// which yields a boxed +/// — a struct cursor with NO value equality — so the original +/// DistinctBy(x => new { x.Id, x.Collection }) never deduped and leaked duplicate +/// per-entity updates into the sync stream. The fix keys on the JsonElement's raw text. +/// +/// +public class EntityUpdateDedupTest +{ + [Fact] + public void TwoOps_SameEntity_DedupeToOneUpdate() + { + var options = new JsonSerializerOptions(); + + // Store projection: { "": { "": { entity } } }. + // The id object key is JSON-encoded ("k1" with quote chars) exactly as the + // pointer segments are encoded — mirrors EncodePointerSegment/DecodePointerSegment. + var current = JsonDocument.Parse( + """{ "TestType": { "\"k1\"": { "Name": "old", "Count": 1 } } }""").RootElement; + var updated = JsonDocument.Parse( + """{ "TestType": { "\"k1\"": { "Name": "new", "Count": 5 } } }""").RootElement; + + // TWO operations targeting the SAME (collection=TestType, id="k1") but different leaves. + var patch = JsonSerializer.Deserialize( + """ + [ + { "op": "replace", "path": "/TestType/\"k1\"/Name", "value": "new" }, + { "op": "replace", "path": "/TestType/\"k1\"/Count", "value": 5 } + ] + """, options)!; + + var updates = current.ToEntityUpdates(updated, patch, options); + + // Pre-fix: the boxed-JsonElement id never compares equal → DistinctBy keeps BOTH (count 2). + // Post-fix: keyed on GetRawText() → the two ops collapse to ONE update. + Assert.Single(updates); + Assert.Equal("TestType", updates.First().Collection); + } +} diff --git a/test/MeshWeaver.Graph.Test/ReduceToMeshNodeJsonElementTest.cs b/test/MeshWeaver.Graph.Test/ReduceToMeshNodeJsonElementTest.cs new file mode 100644 index 0000000000..47960a4f10 --- /dev/null +++ b/test/MeshWeaver.Graph.Test/ReduceToMeshNodeJsonElementTest.cs @@ -0,0 +1,47 @@ +using System.Collections.Generic; +using System.Text.Json; +using MeshWeaver.Data; +using MeshWeaver.Mesh; +using Xunit; + +namespace MeshWeaver.Graph.Test; + +/// +/// Deterministic repro + spec for on the +/// patch path. When an update is derived from a JSON patch (the cross-hub / mirror path), the +/// is a boxed , NOT a . +/// The original change.Value as MeshNode silently produced null, pushing a +/// null-content into the mirror (a live update that drops the node). +/// The fix deserializes the JsonElement to MeshNode exactly as the sibling PatchMeshNode does. +/// +public class ReduceToMeshNodeJsonElementTest +{ + [Fact] + public void PatchPath_JsonElementValue_DeserializesToMeshNode() + { + var options = new JsonSerializerOptions(); + var expected = new MeshNode("k1", "TestSpace") { Name = "Hello" }; + + // The cross-hub/mirror shape: the EntityUpdate carries the node as a JsonElement. + var asJson = JsonSerializer.SerializeToElement(expected, options); + var update = new EntityUpdate(nameof(MeshNode), expected.Id, asJson) + { + OldValue = expected + }; + + var change = new ChangeItem( + new InstanceCollection(new Dictionary { [expected.Id] = expected }), + ChangedBy: "tester", + StreamId: "stream-1", + ChangeType: ChangeType.Patch, + Version: 1, + Updates: [update]); + + var result = MeshDataSourceExtensions.ReduceToMeshNode( + change, new MeshNodeReference(), initial: false, options); + + // Pre-fix: `change.Value as MeshNode` → null → a null-content ChangeItem reaches the mirror. + Assert.NotNull(result.Value); + Assert.Equal(expected, result.Value); + } +} From 4ff3ad6f95eeaca2de4f257184988112232b3ebd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20B=C3=BCrgi?= Date: Tue, 30 Jun 2026 07:34:03 +0200 Subject: [PATCH 2/2] test(ai): include /provider-keys in the built-in skill-set assertions #119 added src/MeshWeaver.AI/Data/Skill/provider-keys.md (an 8th built-in skill) but left the two exact-set assertions in SkillNodeTypeTest + SkillHarnessImportSourceTest at the prior 7, turning main red. Add provider-keys (sorts after model) so the assertions match the shipped Data/Skill/*.md set. Co-Authored-By: Claude Opus 4.8 (1M context) --- test/MeshWeaver.AI.Test/SkillHarnessImportSourceTest.cs | 2 +- test/MeshWeaver.AI.Test/SkillNodeTypeTest.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/MeshWeaver.AI.Test/SkillHarnessImportSourceTest.cs b/test/MeshWeaver.AI.Test/SkillHarnessImportSourceTest.cs index 5234c3dff5..dcf8708d9c 100644 --- a/test/MeshWeaver.AI.Test/SkillHarnessImportSourceTest.cs +++ b/test/MeshWeaver.AI.Test/SkillHarnessImportSourceTest.cs @@ -42,7 +42,7 @@ public void SkillStaticRepoSource_Emits_BuiltInSkills_UnderSkillPartition() ((MeshWeaver.Mesh.Security.PartitionAccessPolicy)policy!.Content!).PublicRead.Should().BeTrue(); var skills = nodes.Where(n => n.NodeType == SkillNodeType.NodeType).ToList(); - skills.Select(n => n.Id).OrderBy(x => x).Should().Equal("agent", "code", "create-space", "harness", "layout-area", "maui", "model"); + skills.Select(n => n.Id).OrderBy(x => x).Should().Equal("agent", "code", "create-space", "harness", "layout-area", "maui", "model", "provider-keys"); skills.Should().AllSatisfy(n => { n.Namespace.Should().Be(SkillNodeType.RootNamespace); diff --git a/test/MeshWeaver.AI.Test/SkillNodeTypeTest.cs b/test/MeshWeaver.AI.Test/SkillNodeTypeTest.cs index 67deb6f1c6..ea505e1782 100644 --- a/test/MeshWeaver.AI.Test/SkillNodeTypeTest.cs +++ b/test/MeshWeaver.AI.Test/SkillNodeTypeTest.cs @@ -29,7 +29,7 @@ public void BuiltInSkillProvider_ShipsStandardSkills_AsPickSkillNodes() var skills = nodes.Where(n => n.NodeType == SkillNodeType.NodeType).ToList(); skills.Should().OnlyContain(n => n.Namespace == SkillNodeType.RootNamespace); - skills.Select(n => n.Id).OrderBy(x => x).Should().Equal("agent", "code", "create-space", "harness", "layout-area", "maui", "model"); + skills.Select(n => n.Id).OrderBy(x => x).Should().Equal("agent", "code", "create-space", "harness", "layout-area", "maui", "model", "provider-keys"); var def = (SkillDefinition)skills.Single(n => n.Id == "model").Content!; def.Action!.Kind.Should().Be(SkillActionKind.Pick);