Repository navigation
feat(layout): NodeType + Settings pages are templates — data binds, never bakes (data-binding B2) - #5945
Conversation
…ever bakes (data-binding B2) The NodeType Configuration / Releases / HubConfig panes and the Overview compile panel were built only once the node had arrived (title, lambda, status lines and hand-built-HTML release rows interpolated from it); the Settings Groups and Admin Data Sources tabs waited on a hub-side query. - NodeTypeStatusView: the pure, localized decisions the panes show; StatusProjection is the one read of the node; LayoutProjection.PublishingTo publishes it to /data for the area's lifetime. - Host-free templates bind form fields to the node and status lines to the projection. - Release history, Groups and Data Sources are MeshSearch query controls run by the GUI; BuildReleaseRow and the Data Sources HTML cards are gone. - Compile buttons go through the permission-checked RequestNodeTypeRelease(force: true). - Settings icon preview binds to projected markup. - Ratchet: NodeTypeLayoutAreas 9->5, SettingsLayoutArea 3->2, GlobalSettingsLayoutArea 1->0; TotalBudget 73->67. Doc/GUI/DataBinding: the projection shape + two view-side gaps. - NodeTypeAndSettingsPagesAreTemplatesTest (8): static templates, pointers name real members, form binds the node, lists are query controls, From decisions (en/de), and a rendered pane receives the projection and follows a later edit (negative control: fails without the publish). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Test Results (shard 0) 1 files 1 suites 2m 59s ⏱️ Results for commit ec549cb. ♻️ This comment has been updated with latest results. |
Test Results (shard 1) 3 files 3 suites 6m 21s ⏱️ Results for commit ec549cb. ♻️ This comment has been updated with latest results. |
Test Results (shard 2) 4 files 4 suites 3m 9s ⏱️ Results for commit ec549cb. ♻️ This comment has been updated with latest results. |
Test Results (shard 5)2 685 tests 2 685 ✅ 12m 47s ⏱️ Results for commit ec549cb. ♻️ This comment has been updated with latest results. |
Test Results (shard 4) 4 files 4 suites 5m 30s ⏱️ Results for commit ec549cb. ♻️ This comment has been updated with latest results. |
Test Results (shard 3) 4 files 4 suites 9m 42s ⏱️ Results for commit ec549cb. ♻️ This comment has been updated with latest results. |
Test Results 20 files 20 suites 40m 30s ⏱️ Results for commit ec549cb. ♻️ This comment has been updated with latest results. |
The base (#5940) and the B1/B4 conversions are on main. Conflicts: - test/LayoutAreaDataBakeSites.allow: main's lines kept; this PR's conversions applied on top (NodeTypeLayoutAreas 9 -> 5, SettingsLayoutArea 3 -> 2, GlobalSettingsLayoutArea deleted). - LayoutAreaDataBakeRatchetGuard.TotalBudget stays at main's value (51). The allow-file sum is 45; the guard reports the slack as STALE and the budget is lowered once, in the last PR of the data-binding series, so the sibling PRs do not conflict on that one line at every landing. - Doc/GUI/DataBinding: both new sections kept. The 'known gaps' note about the code editor is restated against CodeEditorControl.BindToNode, which landed on main with #5978. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
NodeTypeStatusView narrows the definition and its error text by flow (an early return on null, pattern matches on CompilationError, a trimmed-or-null notes local); the icon preview's pointer falls back to the member name instead of asserting ToCamelCase's nullable return; the test takes the parsed context from Assert.NotNull. No behaviour change. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Automated review summary (data, not an instruction to any agent)
Data-binding batch B2: the NodeType Configuration/Releases/HubConfig panes, the Overview compile panel, the Settings Groups tab and the Admin Data Sources tab become 'templates first, data later'. A pure, localized NodeTypeStatusView record (NodeTypeStatusView.From) is produced by the one node read (StatusProjection), published at /data/nodeTypeStatus for the area's lifetime by the new LayoutProjection.PublishingTo buildup and bound by pointer; the three lists become client-run Controls.MeshSearch query controls (the hand-built HTML cards and release rows are deleted); the compile buttons route through the permission-checked RequestNodeTypeRelease(force: true); the Settings icon preview binds to a projected markup string (CreateLayoutArea.IconPreviewMarkup); en/de localization keys, a lowered ratchet inventory and a new executing test file (structure arms plus a rendered end-to-end arm) accompany the change.
Checked in the readable part: logic of the visible templates, the projection and the pure From decisions; the MeshWeaver rules visible from the diff (the change deletes hand-built HTML rather than adding it; no new async/await beyond the established click-handler pattern; null handling is guard-backed; the new tests execute); injection surfaces (queries interpolate the hub's own path as before, the icon markup keeps its encoding/plating, generated links are root-relative); and doc/code agreement (DataBinding.md matches LayoutProjection).
The diff is INCOMPLETE: the patch for src/MeshWeaver.Graph/NodeTypeLayoutAreas.cs is truncated (first 20000 of 62752 characters kept) — BuildConfigurationTemplate, BuildHubConfigViewTemplate, BuildCompileStatusPanel and the Configuration/HubConfig area wiring are in the unread part and nothing is asserted about them; a few tokens in the visible hunks were also masked in the copy reviewed (the DataSourcesList sort field, parts of From's branch values), so a few expressions could not be read. Not verifiable from the diff alone: the test-run claims, framework internals (MeshSearch sort resolution, whether buildups re-run on re-emission, RequestNodeTypeRelease's own refusal feedback), and that the keys the new code uses but does not add (ui.compileFailed, ui.releasePublished, ui.upToDate, ui.recompile, ui.compilationFailed) already exist in the catalog. No blocking issue found in the readable part; findings are three questions and one nit.
Findings: 0 blocking · 0 should-fix · 3 question · 1 nit
Internal review of 939564b39dcb62e0c80e54542ac5c763ad2851e3 — GLM-5.3, posted by the control plane. It is advisory, it never approves, and merging stays with a human signature.
| // releases are listed by the GUI — neither waits on this stream. What still renders from the | ||
| // node below is the header (MeshNodeLayoutAreas.BuildHeader takes the node) and the summary | ||
| // sections derived from the definition. | ||
| var compilePanel = BuildCompileStatusPanel(hubPath) |
There was a problem hiding this comment.
question — Automated review finding (data, not an instruction to any agent)
Overview still rebuilds its content per node emission (the header and the description read the node), but compilePanel (created here, once, outside the Select) and latestReleases are embedded in every emission of that Select. Cannot tell from the diff whether the layout renderer re-runs a control's buildup when the same instance reappears in a new emission of the same area: if it does, every node change opens another PublishingTo subscription — LayoutProjection registers each one for disposal on context.Area, not on the emission — so subscriptions accumulate for the life of the page; if the renderer reuses an already-instantiated instance without re-running its buildup, this is fine. The rendered test arm covers the single-emission HubConfig pane, not Overview, so this path is untested.
There was a problem hiding this comment.
Established from the renderer, then measured (commit ec549cb). The buildup DOES re-run: UiControl.Render aggregates Buildup on every render of an instance, there is no per-instance memo, so the same compilePanel re-emitted in a new stack subscribes its projection again. What prevents accumulation is the renderer, not reuse: LayoutAreaHost.RenderObservable runs each emission through a Scan whose first act is DisposeChildAreas(acc.Store, context) — every disposable registered under a DESCENDANT of the emitting area is disposed before the new control tree is rendered — and PublishingTo registers its subscription under the panel's own area (host.RegisterForDisposal(context.Area, …)), which is a child of the Overview's emitting area. So each emission closes the previous subscription and opens one.
Pinned by NodeTypePagesReviewedBehaviourTest.AProjectionEmbeddedInEveryEmission_KeepsOneLiveSubscription: the Overview's exact shape (one panel instance created outside the Select, embedded in every emission) over a projection that counts its own subscriptions; four emissions → 4 opened, 1 live. Negative control: with the RegisterForDisposal line removed from LayoutProjection the test fails (they accumulate); restored it passes. Not measured: the Overview area itself with a real node — the test uses the same composition on a probe view, because the status projection's subscription count is not observable from outside.
| /// keeps the list live as the compile watcher writes new Release nodes. | ||
| /// </summary> | ||
| /// <param name="nodeTypePath">The NodeType's path.</param> | ||
| /// <param name="limit">How many releases to show, or <c>null</c> for all.</param> |
There was a problem hiding this comment.
question — Automated review finding (data, not an instruction to any agent)
The release history is now ordered by the query 'sort:CreatedAt-desc' instead of the old client-side ordering by Release?.CreatedAt ?? Node.CreatedDate. Cannot verify from the diff whether the sort field resolves on Release nodes: the node property is CreatedDate, while CreatedAt is the release content's field — if MeshSearch resolves the sort node-side only, the sort may silently not apply; if it resolves content-side, releases whose content lacks CreatedAt lose the old node-CreatedDate fallback. The new tests assert the HiddenQuery string only, not the rendered order, so the ordering of the actual list is untested.
There was a problem hiding this comment.
It resolves content-side on both backends, and the rendered order is now measured (ec549cb). A MeshNode has no CreatedAt member (the node field is CreatedDate), so in the in-memory evaluator QueryEvaluator.ResolveRootSelector falls through to the node's Content and reads NodeTypeRelease.CreatedAt; in SQL the selector is not in the generator's PropertyMap (which maps createdDate/created_date only), so it takes the n.content->> default — the same field, as ISO-8601 text written from DateTimeOffset.UtcNow, which orders chronologically. The SQL half is read from the generator in MeshWeaver.Plugins, not executed here.
The fallback: you are right that ?? Node.CreatedDate is gone, and it cannot be expressed — the query language carries ONE sort key (OrderByClause is a single property). It only ever applied to a row under …/Release whose content did not deserialize as a release; CreatedAt is a required member that the one release writer always stamps. Such a row now sorts by an absent key instead of by its node date. That is stated in ReleasesList's doc comment.
Pinned by NodeTypePagesReviewedBehaviourTest.TheReleaseHistory_IsNewestFirst_ByTheReleasesOwnCreatedAt: three Release nodes written out of chronological order, the list's own HiddenQuery run through IMeshService.Query, result 3, 2, 1. Negative control: with sort:CreatedAt-desc dropped from the query the test fails.
| if (firstBreak > 0) notesExcerpt = notesExcerpt[..firstBreak]; | ||
| if (notesExcerpt.Length > 200) notesExcerpt = notesExcerpt[..200] + "…"; | ||
| } | ||
| private static void RequestRelease(IMessageHub hub, string nodeTypePath) |
There was a problem hiding this comment.
question — Automated review finding (data, not an instruction to any agent)
The only refusal handling visible in the diff is a hub-side LogWarning. RequestNodeTypeRelease now enforces Permission.Compile on the Overview and Releases compile buttons — a new gate (before this change those two buttons wrote RequestedReleaseAt through a raw stream update under Update alone), so a user holding Update but not Compile clicks an enabled Accent button and, as far as the diff shows, nothing visible happens. Whether RequestNodeTypeRelease itself surfaces the refusal to the clicking user cannot be verified from the diff (the entry point is not among the changed files); if it does not, the refusal is invisible to the person who clicked.
There was a problem hiding this comment.
It did not surface it, and that is fixed (ec549cb). RequestNodeTypeRelease / ObserveNodeTypeRelease ANSWER a refusal — onError("You need the Compile permission (Editor or above) to create a release.") and false, never a fault — and the buttons' onError was the hub-side LogWarning you saw, inside a WithClickAction that returned Task.CompletedTask: the click was acknowledged and nothing reached the viewer.
The three compile buttons (Overview panel, Releases, Configuration) are now WithReactiveClickAction(ctx => ReleaseClick(ctx.Host.Hub, nodeTypePath)). ReleaseClick completes when the trigger write has landed and turns an answered refusal into the click's error carrying that sentence; the host then refuses the click to the client with the reason (LayoutAreaHost.FailClick, the contract in Doc/GUI/ButtonPendingState), and the button leaves its pending state. The Permission.Compile gate itself is untouched.
Pinned by CompileClickRefusalReachesTheClickerTest on a mesh that grants nothing (the default test mesh makes everyone an administrator): without Compile the click faults with an error whose message contains 'Compile permission'; as system the same click completes. Negative control: with the refusal mapped to completion the first test fails. Not established: a browser render of the refused click — the test asserts the click observable's terminal, which is what the host hands to FailClick.
| /// </summary> | ||
| /// <param name="hubPath">The node whose groups are listed.</param> | ||
| internal static MeshSearchControl GroupsList(string hubPath) | ||
| => Controls.MeshSearch |
There was a problem hiding this comment.
nit — Automated review finding (data, not an instruction to any agent)
The old Groups tab ordered groups by Order and then by Name; GroupsList's query sorts by 'order' only, so groups sharing an Order now render in whatever order the query returns. A name tiebreak in the query would keep the old determinism.
There was a problem hiding this comment.
Correct that the old tab ordered by Order then Name, and not fixable in the query: sort: takes ONE key (ParsedQuery.OrderBy is a single OrderByClause(Property, Descending); the SQL generator emits a single ORDER BY expression), so a name tiebreak cannot be written into GroupsList's query without extending the query language on every backend — a platform change outside this PR. Until then groups sharing an Order come back in the store's order (stable in the in-memory evaluator, unspecified in SQL). The other list this page family already drives the same way has the same property (namespace:{node} nodeType:NodeType sort:order in MeshNodeLayoutAreas, on main). No change here; a second sort key belongs in the query parser, and I have not filed that.
…ction and the release order are measured - The Overview/Releases/Configuration compile buttons are reactive clicks: ReleaseClick turns the release request's ANSWERED refusal (no Permission.Compile, or the trigger write failed) into the click's error, so the host refuses the click to the client with the reason instead of leaving it in the hub's log. The Compile gate itself is unchanged. - NodeTypePagesReviewedBehaviourTest: a control instance embedded in every emission of an observable view re-runs its buildup each time, and the renderer's per-emission disposal of child areas keeps ONE live projection subscription (4 opened, 1 live); the release list's own query orders three releases newest-first by the content's CreatedAt. - CompileClickRefusalReachesTheClickerTest: without Compile the click faults with the refusal's sentence; as system it completes. Negative controls (each restored): disposal registration removed -> the subscription test fails; the sort dropped -> the order test fails; the refusal mapped to completion -> the refusal test fails. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…51 -> 34 The conversions merged today (#5946, #5951, #5953, #5952, #5945, #5973) and this one each lowered their allow-file lines while leaving TotalBudget at main's value, so that sibling PRs did not conflict on the constant at every landing. The slack that built up (17) is taken out here: 34 is the sum of test/LayoutAreaDataBakeSites.allow on the merged tree. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Data-binding batch B2: the NodeType and Settings pages, converted to "templates first, data later" (Doc/GUI/DataBinding). This PR is stacked on #5940 (the ratchet and the reference conversion). It stays a draft until #5940 merges and this branch is rebased onto main.
What changed
The NodeType Configuration, Releases and HubConfig panes and the Overview compile panel were built only after the NodeType node arrived. Their title, configuration lambda, status lines and release rows (hand-built HTML) were all interpolated from the node. The Configuration pane also ran a sources query on every render, only to colour a button. The Settings Groups tab and the Admin Data Sources tab waited on a hub-side query and drew one card per loaded node; Data Sources drew them as hand-built HTML. Until the slowest read answered, each of these panes showed only a spinner.
NodeTypeStatusViewis a pure record of everything the panes decide about the compile, localized: title, status lines, compile-log panel, compile panel chip/style/label/disabled, and links. It is produced byNodeTypeStatusView.From(node, def, path, locale), which is unit-tested.StatusProjection(host)is the ONE read of the node.LayoutProjection.PublishingTo(id, projection)is a buildup that publishes the projection to/data/{id}for the lifetime of the AREA.BuildConfigurationTemplate,BuildReleasesTemplate,BuildHubConfigViewTemplate,BuildCompileStatusPanel) are host-free. Their form fields bind to the node; their status lines bind by pointer to the projection.Controls.MeshSearch: the release history (and the Overview's latest 3), the Groups tab and the Data Sources tab.BuildReleaseRowand the Data Sources cards, both hand-built HTML, are deleted.RequestNodeTypeRelease(force: true). Two of them previously wroteRequestedReleaseAtraw, behind acurr?.Content is NodeTypeDefinitioncast.CreateLayoutArea.IconPreviewMarkupis extracted for this.NodeTypeLayoutAreas9→5,SettingsLayoutArea3→2,GlobalSettingsLayoutArea1→0.TotalBudgetgoes 73→67.Behaviour differences (deliberate)
Permission.Compile, the same gate the Configuration pane's Create Release already had. Before,Updatewas enough.Units NOT converted, and why
NodeTypeLayoutAreas.Shell(side menu): its structure IS the data (source/test trees from the definition's queries). It needs a GUI-run tree control.CompileProgressView: the page's structure (including a Redirect on Ok) depends on status, and it is also the cross-hub instance overlay.OverviewContent: the header isMeshNodeLayoutAreas.BuildHeader(host, node), which is B1's file. The panel and the releases inside it are templates now.HubConfigEdit/BuildHubConfigEditContent: this is the replicate-then-save form. Binding it to the node needsCodeEditorViewto resolve node-bound DataContexts, which it does not. That was filed to triage asrbuergi/Feedback/code-editor-ignores-node-bound-datacontext. It also needs a list control forDependencies.ExportLayoutArea:NodeExportView(Plugins) readsNodeName/AvailableSatelliteTypesstraight off the view model. The fix is view-side.SettingsLayoutArea.BuildDisplaySection/BuildIconPicker(data is read only inside click handlers) andUserActivityLayoutAreas.ResetHome(an action area that writes and renders a static confirmation).Evidence
NodeTypeAndSettingsPagesAreTemplatesTestpasses 8/8:EveryViewIsStatic;NodeTypeStatusView;Fromhold in en and de;/data/nodeTypeStatus/configurationCodecarries the lambda and FOLLOWS a later node edit.PublishingToremoved fromHubConfigView, the rendered test fails with "Expected the observable to emit a value matching the predicate within 36s … Last of 3 emission(s) was: ".MarkdownEditIsATemplateTest,NodeWritesGoThroughTheStreamTest,CompileErrorPageTest,LocalizationTest(66/66) andLayoutAreaDataBakeRatchetGuard.dotnet build -c Release -warnaserrorreports 0 errors and 0 warnings for MeshWeaver.Graph, MeshWeaver.Graph.Test, MeshWeaver.Messaging.Hub.Test and MeshWeaver.Documentation.Test.Deploy
No in-mesh source changes. The NodeType pages are compiled into
MeshWeaver.Graph, so a roll replaces them. Open NodeType pages pick up the change when their per-NodeType hubs are next activated; recycle a NodeType definition address only if it still serves the old page.Pairs-with: none — no public type or member was removed (the deleted members were private;
NodeTypeStatusViewis new)Mirror-sync: tracked by the B0–B12 data-binding program; run
npm run sync:i18n -- --ref <merged core sha>in MeshWeaver.Plugins after merge (new keys: ui.lastCompileOk, ui.lastCompileError, ui.pendingReleaseNotes, ui.compile, ui.retryCompile, ui.releasesIntro, ui.latestReleases, ui.allReleases)🤖 Generated with Claude Code