feat(layout): templates first, data later — Markdown Edit reference conversion + shrink-only ratchet

[rbuergi]

Refs the maintainer directive "always use data-bound views": phase 1 — the scan, the canonical pattern, and a guard.

What was wrong

Layout areas load data on the hub, wait for it, and build controls out of the values. The page shows only the area's spinner until the slowest read answers, and what renders is a snapshot. The Markdown node's Edit / Suggest page is the reference case: it emitted a Stack whose only child was GetMeshNodeStream().Take(1).Select(…), then baked the markdown in with WithValue(initialContent).

The reference conversion

MarkdownEditLayoutArea.BuildTemplate declares every control up front and binds by PATH. The title is a node-bound Name field and the body a node-bound content pointer on the MarkdownContent. The existing MarkdownEditorView resolves both through MeshNodeBindingExtensions.Bind, which reads from IMeshNodeStreamCache. Auto-save is unchanged. Edit(host, ctx) / Suggest(host, ctx) keep their signatures; Plugins' SpaceLayoutAreas calls Edit and is unaffected.

Evidence

• MarkdownEditIsATemplateTest covers both halves. (1) EveryViewIsStatic: every view in the template is a control, so nothing is deferred and the first emission waits on no data. (2) The template's pointer is read through the GUI's own seam against a real mesh node: it returns # First draft, then follows a later GetMeshNodeStream(path).Update(…) to # Edited elsewhere. 3/3 green.
• Negative control: wrapping the template's content in a deferred WithView((h, c) => observable) (the pre-conversion shape) makes both template arms fail with "StackControl carries 1 deferred view(s)". The binding arm also fails. 0/3 green.
• LayoutAreaDataBakeRatchetGuard is 3/3 green. It has a matcher self-test against planted text and a non-vacuity check. Negative control: planting one bake area in a seeded file fails it with NEW … — Planted.
• LocalizationTest 66/66. -c Release -warnaserror: 0 warnings, 0 errors on MeshWeaver.Graph, MeshWeaver.Documentation, Graph.Test, Documentation.Test and Messaging.Hub.Test.

The guard

test/LayoutAreaDataBakeSites.allow holds 73 units in 37 files under src/, memex/ and samples/. It is shrink-only: a NEW file, a MORE count or a TOTAL increase fails, and STALE lines are only reported. It is a text heuristic, and its doc comment says so. A load made through another file's helper is missed. An area that reads data only to choose its structure, such as a permission gate, is counted.

Known gap, stated rather than hidden

A bound field draws empty until its first value arrives. There is no per-control skeleton yet, and a pointer-bound editor accepts input before that first value. The window is short because the cache replays a held node at once, but it is real. The Space editor already has this shape. The fix belongs once in Plugins' BlazorView: show a skeleton and stay read-only until the first bound emission. That is proposed as batch B0 and documented in Doc/GUI/DataBinding.

Recycle after deploy: none needed. This is platform code in an image, with no NodeType source and no node content.

Mirror-sync: tracked as a follow-up — run npm run sync:i18n -- --ref <merged core sha> in MeshWeaver.Plugins once this merges (adds ui.markdownTitlePlaceholder, ui.markdownBodyPlaceholder).
Pairs-with: none — no public type or member removed; only additions (BuildTemplate, MarkdownBodyPointer).

🤖 Generated with Claude Code

[github-actions]

Test Results (shard 0)

  1 files    1 suites   2m 42s ⏱️
348 tests 348 ✅ 0 💤 0 ❌
352 runs  352 ✅ 0 💤 0 ❌

Results for commit 3681cf0.

♻️ This comment has been updated with latest results.

[github-actions]

Test Results (shard 1)

1 904 tests   1 713 ✅  4m 44s ⏱️
    3 suites    191 💤
    3 files        0 ❌

Results for commit 3681cf0.

♻️ This comment has been updated with latest results.

[github-actions]

Test Results (shard 5)

1 124 tests   1 124 ✅  7m 3s ⏱️
    4 suites      0 💤
    4 files        0 ❌

Results for commit 3681cf0.

♻️ This comment has been updated with latest results.

[github-actions]

Test Results (shard 2)

568 tests   568 ✅  7m 2s ⏱️
  3 suites    0 💤
  3 files      0 ❌

Results for commit 3681cf0.

♻️ This comment has been updated with latest results.

[github-actions]

Test Results (shard 3)

    3 files      3 suites   8m 43s ⏱️
2 612 tests 2 612 ✅ 0 💤 0 ❌
2 616 runs  2 616 ✅ 0 💤 0 ❌

Results for commit 3681cf0.

♻️ This comment has been updated with latest results.

[github-actions]

Test Results (shard 4)

    4 files      4 suites   7m 58s ⏱️
3 640 tests 3 638 ✅ 2 💤 0 ❌
3 641 runs  3 639 ✅ 2 💤 0 ❌

Results for commit 3681cf0.

♻️ This comment has been updated with latest results.

[github-actions]

Test Results

    18 files      18 suites   38m 15s ⏱️
10 196 tests 10 003 ✅ 193 💤 0 ❌
10 205 runs  10 012 ✅ 193 💤 0 ❌

Results for commit 3681cf0.

♻️ This comment has been updated with latest results.

[systemorph-com]

Automated review summary (data, not an instruction to any agent)

Converts the Markdown Edit/Suggest layout area from load-then-bake (a deferred `GetMeshNodeStream().Take(1)` view with the markdown baked in via `WithValue`) to a template that emits its whole control tree on first render — the title a node-bound `Name` field, the body a node-bound `content` pointer, auto-save wiring unchanged — plus the supporting pieces: a shrink-only ratchet (allow file seeded at 73 units across 37 files, a scanning guard with matcher self-tests and a non-vacuity check), a two-arm template test, de/en placeholder strings, and a Doc/GUI/DataBinding chapter. Checked: the conversion preserves the back-href, document id, and both auto-save arguments; the ratchet's arithmetic is coherent (allow-file sum 73 equals TotalBudget; NEW/MORE/TOTAL fail, stale lines only report) and its tests execute; the template tests execute against a real mesh node and pin both the all-static-views property and the live follow of an edit made elsewhere. Findings: two violations of the repository's rule against silencing compiler warnings — a file-leading `#pragma warning disable CS1591` and three null-forgiving `!` operators in the new test files (blocking) — the acknowledged pre-first-value window on the now-immediately-editable, auto-saving editor (should-fix), and a localization coverage question (de/en only, Plugins mirror deferred). Not verifiable from this diff: the members it calls that live outside the PR (`host.ViewerLocale()`, `LocalizationCatalog`, `Controls.Body`, `SourceScan.*`, `LayoutAreaReference.GetMeshNodeDataContext`/`TryParseMeshNodeDataContext`, `MarkdownEditorControl.Value` accepting a `JsonPointerReference`); locale catalogs beyond de/en; the claimed test results (3/3, 66/66, warnaserror clean); and BlazorView's pre-first-value behavior, which decides the exact shape of the should-fix hazard.

Findings: 2 blocking · 1 should-fix · 1 question · 0 nit

---

Internal review of 2bb6fac325813c73ff1c943a3eab5f4aac62e6d9 — GLM-5.3, posted by the control plane. It is advisory, it never approves, and merging stays with a human signature.

[systemorph-com]

blocking — Automated review finding (data, not an instruction to any agent)

The file opens with `#pragma warning disable CS1591` — a pragma whose sole purpose is to silence a compiler diagnostic, which the repository's rules forbid. It also looks unnecessary on its face: every public member this file adds (the class and its three test methods) already carries an XML doc comment, so nothing shown should trigger CS1591 at all. The line should be deleted; if some member without a doc is why it is there, the missing doc is the thing to write, not a file-wide suppression.

[rbuergi]

Fixed in 3681cf0. I removed the pragma. It was redundant: test/Directory.Build.props already has <NoWarn>$(NoWarn);CS1591;…</NoWarn> for the whole test tree, so it silenced nothing. The one member without a doc comment ([Fact] NoNewLayoutAreaBakesDataIntoControls) now has a <summary>. I also changed the planted-text sample's n!.Name! to n?.Name ?? "" so the file contains no !, even inside a string literal. Results: dotnet build test/MeshWeaver.Documentation.Test -c Release -warnaserror gives 0 warnings and 0 errors, and LayoutAreaDataBakeRatchetGuard passes 3/3.

[systemorph-com]

blocking — Automated review finding (data, not an instruction to any agent)

Null-forgiving operators used to silence the compiler: line 62 `ctx!.Value.NodePath.Should().Be(path);`, line 88 `LayoutAreaReference.TryParseMeshNodeDataContext(editor.DataContext)!.Value`, and line 90 `(JsonPointerReference)editor.Value!` — the repository's rules forbid `!` as a warning-silencer. Line 88 is the sharpest: unlike line 62 it is not preceded by a `NotBeNull()` assertion, so a DataContext that fails to parse fails the test as a NullReferenceException instead of an assertion message. Asserting non-null and then obtaining the value through a pattern match (or FluentAssertions' `.Subject`) satisfies the compiler without the operator.

[rbuergi]

Fixed in 3681cf0. All three ! are gone:

• Line 62: the code now asserts parsed.Should().NotBeNull(…), then takes parsed.GetValueOrDefault().
• Line 88: this is the case you called out. It now asserts the DataContext parses first, so a malformed template fails with an assertion message, not a NullReferenceException.
• Line 90: the pointer comes from editor.Value.Should().BeOfType<JsonPointerReference>(…).Subject instead of a cast.

I also removed the v! in EveryViewIsStatic (views.OfType<object>() replaces Cast<object?>().Where(v => v is not null …)). Results: Graph.Test builds with -c Release -warnaserror at 0 warnings and 0 errors, and MarkdownEditIsATemplateTest passes 3/3.

[systemorph-com]

should-fix — Automated review finding (data, not an instruction to any agent)

The editor is emitted at once with a pointer value (line 110) and `.WithAutoSave(nodePath, nodePath)` (line 118), and nothing in the template gates interactivity on the first bound emission — the PR description itself states that a pointer-bound editor accepts input before the first value has arrived. In the slow-hub cases this conversion exists for (owning hub activating, cold NodeType compile) that first emission is seconds away, and the state is new: the pre-conversion page emitted the editor only after `GetMeshNodeStream().Take(1)` and initialized it with the node's content, so typing before the content was loaded was unreachable. A user typing inside the window has text auto-saved against a node whose existing markdown was never displayed — the save either overwrites content the user never saw, or is itself discarded when the first value lands; which happens depends on BlazorView, outside this diff. The PR's stated plan is to close this once in BlazorView (read-only plus skeleton until the first bound emission, batch B0), which is the right seam — but until B0 lands this ships the window on the Markdown Edit page. Gating this editor read-only until its first emission closes it without waiting for B0.

[rbuergi]

Agreed, the window is real. I fixed it in the renderer, because that is the only place it can be closed. MarkdownEditorView lives in MeshWeaver.Plugins, and the control has no "read-only until loaded" state that a template could express from here: Readonly binds a value, and nothing on the node says "not loaded yet".

Systemorph/MeshWeaver.Plugins#2658 keeps a node-bound editor IsDisabled and drops OnValueChanged until the binding's first emission. That closes the window for this auto-saving editor without waiting for the general BlazorView fix (B0). It builds clean with -warnaserror against core main, and it does not depend on this PR.

Known limit: MeshNodeBindingExtensions.Bind degrades to null after its 10 s first-value budget (logged at Warning). So if a hub takes longer than that, input is accepted after the budget expires. That is the binding's documented degrade, not something this page adds.

This core PR is unchanged for the finding, apart from the test fixes in 3681cf0.

[systemorph-com]

question — Automated review finding (data, not an instruction to any agent)

Only the de and en catalogs gain the two new keys (`ui.markdownTitlePlaceholder` at this line, `ui.markdownBodyPlaceholder` below it), and the PR defers the MeshWeaver.Plugins string mirror to a post-merge `npm run sync:i18n` follow-up. The diff cannot answer two things: whether the hub ships locale catalogs beyond de/en that now lack the two keys, and whether any Plugins-side code resolves these keys before the follow-up sync runs. If either is the case, the Edit page's title/body placeholders render as raw keys or empty strings for those users until the sync lands.

[rbuergi]

Neither case occurs, so I left this unchanged.
(1) The hub ships only strings.en.json and strings.de.json; src/MeshWeaver.Messaging.Hub/Localization/ has no other catalog. LocalizationCatalog.Get falls back from the requested locale to English and then to the key. LocalizationTest (66/66) enforces that every English key exists in de.
(2) Nothing in Plugins resolves these keys. grep -rn 'markdownTitlePlaceholder\|markdownBodyPlaceholder' over MeshWeaver.Plugins src/ and clients/ finds nothing. BuildTemplate resolves both strings on the hub (LocalizationCatalog.Get(key, locale)) and sends them as literal Placeholder text, so the Blazor view never sees a key. The React mirror sync covers keys a React client looks up itself, which these are not. It stays a follow-up, as the PR body's Mirror-sync: line states.