Repository navigation
feat(layout): $Data and the GitHub-sync activity panel are templates (data-binding B3, part 1) - #5944
Conversation
… dimension feeds split from their controls
Data-binding batch B3, part 1 (Doc/GUI/DataBinding → "Templates first, data later").
- DataPathViews ($Data): was data.CombineLatest(showFull, …) => Controls.Markdown(json) — the area
emitted nothing until the referenced data answered, then rebuilt its control per change. Now a
TEMPLATE (BuildTemplate: markdown + "Load all", text/label/visibility bound by pointer to
/data/{viewId}) and a FEED (FeedDataView: builds no control; started in WithBuildup so it lives
with the rendered area; one subscription, loading line first). Strings localized (data.*).
DataViewModel registered in AddLayoutTypes.
- GitHubSyncSettingsTab.BuildActivityPanel: subscribed the activity node twice and rebuilt hand-made
HTML + a Cancel button per tick. Now embeds the activity's own Progress area
(LayoutAreaControl + SpinnerType.Skeleton): the activity hub renders log, status and Cancel.
- EditorExtensions dimension select/label: already returned their control at once, bound to a /data
slot; the collection subscription moved into FeedDimensionOptions / FeedDimensionDisplayName so the
template/feed split is explicit (scanner false positive, no behaviour change).
- Tests: DataReferenceAreaIsATemplateTest (silent data source; template arrives, slot binds, later
change follows; negative control: the pre-conversion area never renders), and
GitSyncActivityPanelIsATemplateTest. LayoutTemplateAssertions (Monolith.TestBase) shares the
reference test's EveryViewIsStatic.
- Ratchet: EditorExtensions, DataPathViews, GitHubSyncSettingsTab lines removed; TotalBudget 73 -> 69.
- StreamView<T> left as is: its contract hands loaded items to the caller's factory; it retires
with its callers (B1 MeshNodeLayoutAreas, Plugins Graph.Views). Documented in DataBinding.md.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Test Results (shard 0) 1 files 1 suites 2m 44s ⏱️ Results for commit ea19152. ♻️ This comment has been updated with latest results. |
Test Results (shard 1) 3 files 3 suites 5m 42s ⏱️ Results for commit ea19152. ♻️ This comment has been updated with latest results. |
Test Results (shard 2) 4 files 4 suites 3m 44s ⏱️ Results for commit ea19152. ♻️ This comment has been updated with latest results. |
Test Results (shard 5) 4 files 4 suites 13m 11s ⏱️ Results for commit ea19152. ♻️ This comment has been updated with latest results. |
Test Results (shard 4)839 tests 646 ✅ 2m 18s ⏱️ Results for commit ea19152. ♻️ This comment has been updated with latest results. |
Test Results (shard 3) 4 files 4 suites 11m 48s ⏱️ Results for commit ea19152. ♻️ This comment has been updated with latest results. |
Test Results 20 files 20 suites 39m 28s ⏱️ Results for commit ea19152. ♻️ This comment has been updated with latest results. |
|
Heads-up from #5968 (fed-stream error arm): the two feeds this PR moves into |
…s LayoutTemplate #5940 and the batches that followed it are on main. Conflicts: - DataBinding.md: both sections kept (this PR's 'Two more shapes' before main's 'authoring samples'). The paragraph that called per-row actions a platform gap now points at Row-scoped actions (#5955 is on main). - LayoutAreaDataBakeSites.allow: main's removals kept, this PR's three lines (GitHubSyncSettingsTab, EditorExtensions, DataPathViews) removed. - LayoutAreaDataBakeRatchetGuard: TotalBudget stays at main's 51. The allow-file sums to 47; the guard reports the slack as STALE and the budget is lowered once, in the last PR of the series, so the sibling PRs do not conflict on that line. Also: the test-base copy LayoutTemplateAssertions (reflection over a protected member, with a null-forgiving operator) is dropped. main has the public MeshWeaver.Layout.LayoutTemplate for exactly this, so GitSyncActivityPanelIsATemplateTest asserts LayoutTemplate.DeferredViews is empty and walks LayoutTemplate.Descendants. Negative control re-run: one deferred view added to BuildActivityPanel fails it with 'StackControl[0]: ViewStream`1'. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…erators (review of #5955) Two findings from the review of #5955, promised on its threads for this PR: - RowScopedClickActionTest held three null-forgiving operators. They are replaced by Assert.NotNull / Assert.IsType, which narrow without one. - The BlurEvent relay in LayoutAreaHost.OnBlur was pinned nowhere while the doc promises the row on a blur. EachRowsFieldBlursWithItsOwnRow is the blur twin of EachRowsButtonActsOnItsOwnRow: a bound list of text fields with a blur action, every row blurred out of render order, each handler sees its own pointer, index, value and node path. Negative control: with the Row relay removed from OnBlur the new test fails (1 of 5); restored, 5 of 5. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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 B3 part 1: converts the $Data area (DataPathViews) from a data-gated observable into a render-once template — a markdown block plus a Load-all button, every value a JsonPointerReference into /data/{viewId} — fed by FeedDataView, a subscription started in WithBuildup that writes the projection into the slot, with the loading line first via StartWith on the same subscription and a new error handler (data.failed) that the old CombineLatest chain lacked. BuildActivityPanel drops its two activity-node subscriptions and its hand-built HTML for an embedded ActivityLayoutAreas.ProgressArea with a skeleton spinner. EditorExtensions gets a structural feed split (FeedDimensionOptions, FeedDimensionDisplayName) with no behaviour change, DataViewModel is registered for serialization, five data.* strings land with en/de parity, three ratchet entries are removed (4 budget units, 73 to 69), and the two #5955 follow-ups land in RowScopedClickActionTest (blur-relay test, null-forgiving operators removed). Checked: the StartWith ordering does put the loading line ahead of the first data value on one subscription; the error path is surfaced instead of swallowed; en and de carry the same five keys; the ratchet arithmetic matches the removed entries; the hard rules hold (hand-built HTML removed, click actions keep the established Task.CompletedTask pattern, no new null-forgiving operators or pragmas, both new tests execute); no prompt injection found in the diff. Could not verify from the diff: several identifiers inside the patches are masked by the control plane (the viewId suffix derivation, the WithBuildup return value, the logging call), and ActivityLayoutAreas.ProgressArea, WithBuildup, LayoutTemplate and the serializer registration list's consumers are outside it; the item carries no CI status.
Findings: 0 blocking · 0 should-fix · 2 question · 0 nit
Internal review of bd85e0e19c43d395b1675c861404273529a24182 — GLM-5.3, posted by the control plane. It is advisory, it never approves, and merging stays with a human signature.
| /// Builds the live progress panel for the running operation at <paramref name="activityPath"/>. | ||
| /// A TEMPLATE (Doc/GUI/DataBinding → "Templates first, data later"): it embeds the activity | ||
| /// node's OWN <see cref="ActivityLayoutAreas.ProgressArea"/> — the platform's progress view, | ||
| /// rendered by the activity's hub: its <see cref="MeshWeaver.Data.LogMessage"/> lines in the |
There was a problem hiding this comment.
question — Automated review finding (data, not an instruction to any agent)
The removed code localized the transcript per #3236 by reading host.ViewerLocale() on the render path — its comment read: the locale is read here, on the render path where LayoutAreaHost has restored the subscriber's AccessContext — never at the write site, which has no viewer. The replacement delegates all rendering to ActivityLayoutAreas.ProgressArea on the activity's hub, and the new doc comment asserts the LogMessage lines arrive in the viewer's language — but ProgressArea's implementation is not in this diff, so it cannot be verified here that it reads the viewer's locale on its own render path rather than at write time or from an ambient culture. If it does not, this embed regresses #3236.
There was a problem hiding this comment.
Verified, and the regression is REAL. The Progress area's renderer is not in this repository: it is ActivityViews.Progress in MeshWeaver.Plugins (src/MeshWeaver.Graph.Views/ActivityViews.cs, on main). It does read the viewer's locale on its own render path — BuildLog(log, locale: host.ViewerLocale(), …) at lines 109 and 214 — but BuildLog uses that locale only for the empty-log line and the 'earlier messages' line. Each transcript row is Controls.Label(msg.Message) (line ~373): the stored English, not msg.Localize(locale). The code this PR removes rendered m.Localize(locale). So embedding the platform view as it stands today turns the GitHub-sync transcript back to English for every non-English viewer — #3236 regressed for this panel.
Consequence: this PR is back to DRAFT (un-armed) and stays there until the Plugins renderer localizes each row (msg.Localize(locale) in BuildLog, with a test rendering a keyed entry in a second language) and that module is published. That fix is correct on its own, independent of this change, and it also repairs every other embed of the activity Progress area. The doc comment here ('LogMessage lines in the viewer's language') is true only after it. I will link the Plugins PR on this thread.
There was a problem hiding this comment.
The renderer fix is Systemorph/MeshWeaver.Plugins#2709: ActivityViews.BuildLog now renders each row with msg.Localize(locale), and BuildProgressIndicator does the same for the latest line beside the spinner. ActivityProgressViewTest pins it (a keyed entry renders German for de, English for en / no locale; an un-keyed entry renders its stored text); with the two lines reverted 2 of 32 fail. This PR stays a draft until that module is published.
There was a problem hiding this comment.
The prerequisite has landed: Systemorph/MeshWeaver.Plugins#2709 merged as ceba0f8eb (2026-10-02 20:43Z), and the MeshWeaver.Graph.Views bundle (batch 2/8) has since built and published green on Plugins main push runs 37117244174, 37117805577 and 37119622234. The embedded Progress area now renders each transcript row with msg.Localize(locale) on its own render path, so this embed no longer regresses #3236. Undrafting.
| .AddLayout(layout => layout); | ||
|
|
||
| protected override MessageHubConfiguration ConfigureClient(MessageHubConfiguration configuration) | ||
| => base.ConfigureClient(configuration).AddLayoutClient(d => d); |
There was a problem hiding this comment.
question — Automated review finding (data, not an instruction to any agent)
This test pins only the self-reference branch (empty path, default data reference held silent). The non-empty-path branch of the rewritten area is not exercised by any test in this diff: OpenData's DataPathReference open, the data.streamUnavailable slot, and the suffix that derives viewId/showFullKey from the path. The suffix line is masked in the patch received, and the pre-conversion code replaced '/' when building the key, so whether the new derivation still sanitizes a path like Orders/10248 cannot be read here; if no other test covers the branch, it is unpinned.
There was a problem hiding this comment.
Two parts. The suffix still sanitizes: var suffix = localPath?.Replace("/", "_") ?? "self"; (DataPathViews.cs:46), so Orders/10248 gives dataView_Orders_10248 / showFull_Orders_10248 exactly as before the conversion — that line is unchanged context, which is why the patch you received masks it.
The rest is right: the non-empty-path branch (the DataPathReference open, the data.streamUnavailable slot, the derived ids) has no test in this diff or elsewhere — DataReferenceAreaIsATemplateTest pins only the self reference. Since this PR is now held as a draft for the localization finding on the neighbouring thread, the missing case goes into its next push rather than into a follow-up: a path-addressed $Data/Orders/10248 whose template arrives with the sanitized ids, then the data, and the unavailable-stream line when the reference cannot be opened.
There was a problem hiding this comment.
Done in the follow-up commits: DataReferenceAreaPathBranchTest (fe13bd8) pins the path-addressed $Data/Orders/10248 branch — template with the sanitized ids first, then the data, and the data.streamUnavailable line when the reference cannot be opened. Green locally on ea19152 together with DataReferenceAreaIsATemplateTest, EditorTest, InlineEditingTest (15/15 executed).
Pull request was converted to draft
…on is reported in the slot (review of #5944) The non-empty-path branch of the $Data template had no test. Three cases now pin it: the template for Orders/10248 arrives before its data, in slots named after the path with its slash replaced (dataView_Orders_10248), and the data and a later change reach the same slot with the control tree unchanged; a path the workspace cannot open is named in the slot; and a mapped collection path is still opened. Writing the second case found that the documented 'reported in the slot, not thrown' did not hold for the commonest unopenable path: a first segment no data source maps throws ArgumentException (Collections X are not mapped to any source) out of the render — as it did before the conversion. OpenData now asks the map the read uses (virtual paths, then DataContext.GetTypeSource), the #5065 rule DomainLayoutAreas.Catalog already follows. Negative controls: without the Replace all three fail; without the guard the unopenable-path case fails on the framework's render-failure control. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Combined state: the string catalogs carry main's todo.* keys and this PR's data.* keys; the data-bake allow-file drops this PR's three lines on top of main's. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…34 -> 29 29 is the sum of test/LayoutAreaDataBakeSites.allow on the merged tree (main's 33 minus this PR's four units: GitHubSyncSettingsTab 1, EditorExtensions 2, DataPathViews 1). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
FeedDimensionOptions (moved out of CreateDimensionSelectControl) forgave three nulls: the collection's stream, the emitted collection and the dimension's type definition. Each threw a NullReferenceException before; each still throws, as an InvalidOperationException naming what is missing. The two $Data area tests address the area by its constant and unwrap controls with BeOfType<T>().Subject. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… reaches an error arm FeedDimensionDisplayName and FeedDimensionOptions subscribed with a bare .Subscribe(onNext), the #5650 crash shape (heads-up from #5968). Both now go through host.FeedData(null, id, stream, onNext): a fault on a later emission is logged at its classified level instead of escaping on the hub. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Re the #5968 heads-up: done in ea19152 — |
Data-binding batch B3, part 1 — framework helpers (Doc/GUI/DataBinding → "Templates first, data later").
What changed
Layout/Views/DataPathViews($Data)data.CombineLatest(showFull, …) ⇒ Controls.Markdown(json)— nothing rendered until the data answered; a new control per changeBuildTemplate: markdown + "Load all", text / label / visibility bound by pointer to/data/{viewId}) + feed (FeedDataView, builds no control; started inWithBuildupso it lives with the rendered area; one subscription, loading line first). Strings localized (data.*).GitSync/GitHubSyncSettingsTab.BuildActivityPanelProgressarea (LayoutAreaControl+SpinnerType.Skeleton) — the activity hub renders log, status, CancelLayout/EditorExtensionsdimension select + read-only label/dataslot (scanner false positive)FeedDimensionOptions/FeedDimensionDisplayName— explicit template/feed split, no behaviour changeLayout/Domain/LayoutHelperExtensions.StreamView<T>MeshNodeLayoutAreas.Thumbnail/Metadata(batch B1) andGroupMembershipLayoutAreas,MeshDataSourceLayoutAreas×2,PinViewsin MeshWeaver.Plugins (batch B5). Swept withgit grep -F .StreamViewover core, Plugins, Education, Reinsurance, SocialMedia, Manufacturing, Crm, FundReporting, Memex, PartnerRe.Memex.Ratchet: three lines removed from
test/LayoutAreaDataBakeSites.allow,TotalBudget73 → 69.Root cause it fixes
Each of these areas waited on the hub for data before it emitted, then baked values into controls — the page showed a spinner until the slowest read answered, and the result was a snapshot. One finding worth keeping (now in DataBinding.md): registering a feed with
host.RegisterForDisposal(ctx.Area, …)from the area function does NOT work — the area's disposables are cleared when its first control renders, so the feed died before the data came (measured: the slot stayed on the loading line). The feed has to start inWithBuildup, asTemplate.Binddoes.Evidence
DataReferenceAreaIsATemplateTest— the default data reference is a subject held silent: the$Datastack arrives anyway with pointer-bound children, the slot shows the loading line, then the data, then a later change, with the control tree unchanged. Negative control (the pre-conversionDataPathViews): fails on the first wait — no control while the source is silent.GitSyncActivityPanelIsATemplateTest— the panel embeds the activity'sProgressarea with a skeleton and holds no deferred view. Negative control: adding one deferred view failsEveryViewIsStatic.EditorTest,InlineEditingTest,LayoutTest,LocalizationTest,LayoutAreaDataBakeRatchetGuardgreen locally; Release-warnaserrorclean for Layout, GitSync, Messaging.Hub, Documentation, Monolith.TestBase, Layout.Test, Documentation.Test.Deploy
Nothing to recycle beyond a normal roll: these are compiled framework areas, not in-mesh source.
Pairs-with: none — no public type or member removed (
BuildActivityPanelwas private;StreamViewkept).Implementers: none — no interface member added.
Mirror-sync: run
npm run sync:i18n -- --ref <merged core sha>in MeshWeaver.Plugins after this merges (addsdata.noDefaultReference,data.streamUnavailable,data.none,data.loadAll,data.failed).🤖 Generated with Claude Code