fix(layout): a catalog flag switched OFF must survive the wire by rbuergi · Pull Request #2687 · Systemorph/MeshWeaver

rbuergi · 2026-08-29T17:36:52Z

The defect

SortConfig.Ascending, SortConfig.ThenByAscending, SectionConfig.ShowCounts and SectionConfig.Collapsible all default to true in C# but to false in the CLR. The hub serializes with WhenWritingDefault, which compares against the CLR default — so true was written, false was dropped, and the reader re-applied the initializer.

None of these four flags could be turned off. An explicit "off" arrived as "on", silently.

How it surfaced

MeshWeaver.Crm's new activity feed asks for Ascending = false (newest first). The rendered payload on the portal:

"sorting":{"$type":"SortConfig","sortByProperty":"date","thenByAscending":true}

No ascending key at all — so the feed rendered oldest first. Compare the sibling view that asks for true, where it survives:

"sorting":{"$type":"SortConfig","sortByProperty":"nextStepDate","ascending":true,...}

Nothing errored; the intent evaporated between the two sides. Same silent shape as the documented "enum member numbered 0 is never stored" trap, one layer up.

The fix

[JsonIgnore(Condition = JsonIgnoreCondition.Never)] on the four properties, so the value is always written whatever it is.

No type or signature change → nothing source- or binary-breaking.
An absent flag still reads as the documented default, so existing payloads are unaffected.

How it was tested

Round-trip through a hub-shaped serializer (DefaultIgnoreCondition = WhenWritingDefault), asserting the false case reaches the client and that the true/default cases are unchanged.

Verified the tests actually catch it rather than assuming:

attributes removed  → Failed: 2, Passed: 1
attributes restored → Passed: 3

Release + -warnaserror clean on MeshWeaver.Layout.

What's New: skipped — a serialization fix with no standalone user-facing story; the visible effect ships with the consumer that needed it.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Wt457V2hiV4YkEmcNFNt4N

SortConfig.Ascending, SortConfig.ThenByAscending, SectionConfig.ShowCounts and SectionConfig.Collapsible all default to `true` in C# but to `false` in the CLR, and the hub serializes with WhenWritingDefault — which compares against the CLR default. So `true` was written and `false` was DROPPED, and the reader then re-applied the initializer: an explicit "off" arrived as "on". None of these four flags could be turned off at all. Measured on MeshWeaver.Crm's new activity feed: it asked for Ascending = false (newest first) and the rendered payload carried no `ascending` key at all, so the portal showed the OLDEST interactions first. Nothing errored — the intent evaporated between the two sides, which is the same silent shape as the already-documented enum-numbered-0 trap, one layer up. Fixed at the root with [JsonIgnore(Condition = JsonIgnoreCondition.Never)] on the four properties, so their value is always written whatever it is. No type or signature changes, so nothing is source- or binary-breaking, and an ABSENT flag still reads as the documented default — existing payloads are unaffected. Tests round-trip through a hub-shaped serializer (WhenWritingDefault) and assert both the false case and that the true/default cases are unchanged. Verified they catch it: removing the attributes turns the suite to 2 failed / 1 passed, and restoring them returns 3/3. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Fixes a serialization round-trip bug where catalog/layout boolean flags with C# initializers defaulting to true could not be switched off because hub serialization uses JsonIgnoreCondition.WhenWritingDefault (CLR-default comparison), causing explicit false values to be omitted on the wire and re-initialized back to true on read.

Changes:

Force four catalog config boolean flags (SortConfig.Ascending, SortConfig.ThenByAscending, SectionConfig.ShowCounts, SectionConfig.Collapsible) to always serialize by applying [JsonIgnore(Condition = JsonIgnoreCondition.Never)].
Add a hub-shaped serializer round-trip test verifying the false case is preserved and that default/true behavior remains unchanged.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
test/MeshWeaver.Layout.Test/CatalogConfigFlagsSurviveTheWireTest.cs	Adds regression tests for hub-like `WhenWritingDefault` serialization round-trips of catalog config flags.
src/MeshWeaver.Layout/Catalog/CatalogTypes.cs	Ensures four `bool` flags are always emitted during JSON serialization so explicit `false` survives the wire.

Suppressed comments (1)

src/MeshWeaver.Layout/Catalog/CatalogTypes.cs:60

This comment block repeats the SortConfig “newest-first … oldest-first” example, which doesn’t relate to Collapsible and can be confusing. Recommend a shorter, flag-agnostic explanation here (or referring to a shared note).

    // 🚨 Serialized ALWAYS, never "when not default". The hub writes with
    // WhenWritingDefault, which compares against the CLR default (false) — not against this
    // property's initializer (true). So `true` went over the wire and `false` was DROPPED, and the
    // reader then re-applied the initializer: an explicit "off" arrived as "on", and these flags
    // could not be turned off at all. Measured 2026-08-29 on MeshWeaver.Crm's activity feed, which

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+        Assert.Contains("\"ascending\":false", json);
+        Assert.Contains("\"thenByAscending\":false", json);
+
+        Assert.False(JsonSerializer.Deserialize<SortConfig>(json, HubLike)!.Ascending);


+    // 🚨 Serialized ALWAYS, never "when not default". The hub writes with
+    // WhenWritingDefault, which compares against the CLR default (false) — not against this
+    // property's initializer (true). So `true` went over the wire and `false` was DROPPED, and the
+    // reader then re-applied the initializer: an explicit "off" arrived as "on", and these flags
+    // could not be turned off at all. Measured 2026-08-29 on MeshWeaver.Crm's activity feed, which
+    // asked for newest-first and silently rendered oldest-first.
+    [JsonIgnore(Condition = JsonIgnoreCondition.Never)]
    public bool ShowCounts { get; init; } = true;


rbuergi · 2026-08-29T17:50:46Z

Two notes for review.

1. The rule was already documented — the framework's own types just didn't follow it. Doc/Architecture/Serialization.md already says it, and already prescribes the exact fix in this PR:

Under WhenWritingDefault a bool property whose value is false is omitted from the JSON entirely, so a property declared public bool Flag { get; init; } = true silently fails to round-trip a true → false change… When a default-valued member must survive the wire, annotate it [JsonIgnore(Condition = JsonIgnoreCondition.Never)].

SortConfig and SectionConfig are four instances of precisely that shape. So this is not a new rule, and no doc change is needed — it is the documented rule being applied where it was missed. Worth noting because it suggests a lint would pay for itself: a public bool … = true on a wire type without the attribute is mechanically detectable.

2. The red "Public surface (binary compatibility)" here is not this change. It is the stale-allow ratchet firing repo-wide after #2678's views carve-out merged — once those types stopped moving in any open PR's diff, the nine type-forwards.allow entries went stale for everyone at once. Fixed by #2688 (removes the block); this PR needs only a fresh run afterwards, no rebase. Diff here is two files, neither near MeshWeaver.Graph.

github-actions · 2026-08-29T17:55:20Z