Repository navigation
feat(samples): Northwind, Article and PythonDemo views are templates (data-binding B4, part 1) - #5951
Merged
Merged
Conversation
…(data-binding B4, part 1) Sixteen in-mesh sample areas loaded their node on the hub and baked the values into hand-built HTML. They are now TEMPLATES: built from the node's path, emitted whole on the first render, every stored value a JsonPointerReference the GUI resolves through the node stream; values only the hub can compute (employment dates, stock status, an article's metadata line, the Python report) come from a FEED that builds no control, bound with Template.Bind. Thumbnails are path-only. ReportsCatalog's cards are a MeshSearch. Each converted NodeType gains a Test/ folder whose cases build the template from a path (with a silent feed) and assert LayoutTemplate.DeferredViews is empty plus the bound pointers; LayoutTemplate is a new public reader in MeshWeaver.Layout so in-mesh C# can make that assertion. Ratchet: 8 lines removed, TotalBudget 73 -> 59. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
|
Auto-merge was not armed. This pull request targets |
Contributor
Contributor
Test Results (shard 1) 3 files ±0 3 suites ±0 5m 44s ⏱️ +6s Results for commit 2d2ecb0. ± Comparison against base commit 7b097dd. This pull request removes 1 and adds 3 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Contributor
Test Results (shard 2)1 110 tests ±0 1 110 ✅ ±0 3m 44s ⏱️ +5s Results for commit 2d2ecb0. ± Comparison against base commit 7b097dd. This pull request removes 3 and adds 1 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Contributor
Test Results (shard 5)2 672 tests ±0 2 672 ✅ ±0 12m 52s ⏱️ +23s Results for commit 2d2ecb0. ± Comparison against base commit 7b097dd. This pull request removes 25 and adds 9 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Contributor
Contributor
Contributor
Test Results 20 files ±0 20 suites ±0 39m 51s ⏱️ -45s Results for commit 2d2ecb0. ± Comparison against base commit 7b097dd. This pull request removes 29 and adds 13 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
![systemorph-com[bot]](https://avatars.githubusercontent.com/u/77832550?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Automated review summary (data, not an instruction to any agent)
Data-binding batch B4, part 1: the Northwind Customer, Supplier, Employee, Product, Article and ReportsCatalog sample views, the ACME Article views and the PythonDemo PrimeReport view are converted from hub-side node reads (GetStream<MeshNode>() plus FirstOrDefault on the hub path) with hand-built Controls.Html markup into templates built from the node's path alone. Stored fields are JsonPointerReferences into the node that the GUI resolves; values only the hub can compute (employment dates and years of service, stock and listing statuses, the article header line, the Python prime report) come from feeds — observables that build no control and Catch their failures into a log line plus visible fallback text — bound into controls declared up front; the catalog's report cards become a Controls.MeshSearch with a hidden namespace/sort query. Every converted NodeType registers an in-mesh Tests view in its JSON configuration. The PR description additionally states a platform addition (public LayoutTemplate.Descendants/DeferredViews over a new internal IDeclaresViews on ContainerControl<T>), documentation updates, and a warning-ratchet reduction (8 allow lines removed, TotalBudget 73 to 59).
Checked from the readable part of the diff: template and feed construction (DistinctUntilChanged, Catch with logging), pointer and data-context binding, the new in-mesh test suites (they execute their cases and render verdict rows), the NodeType JSON registrations, and the visible docs excerpt. The diff carried by the review item is incomplete: src/MeshWeaver.Documentation/Data/DataMesh/CallingPython.md is truncated, and 7 patches are omitted — src/MeshWeaver.Documentation/Data/GUI/DataBinding.md, src/MeshWeaver.Documentation/Data/GUI/LayoutAreas.md, the new src/MeshWeaver.Layout/LayoutTemplate.cs, the ContainerControl.cs change (its project is masked in the review copy), test/LayoutAreaDataBakeSites.allow, test/MeshWeaver.Documentation.Test/LayoutAreaDataBakeRatchetGuard.cs, and the new test/MeshWeaver.Layout.Test/LayoutTemplateTest.cs. Nothing is asserted about the platform API's behavior, the ratchet-budget change, or the unread documentation. The review copy of the diff also masks some identifiers, so the findings quote readable fragments verbatim and carry no line anchors.
Findings: 2 blocking · 0 should-fix · 1 question · 1 nit
File-level findings — Automated review finding (data, not an instruction to any agent):
blocking samples/Graph/Data/Northwind/Employee/Test/EmployeeViewTests.cs
The null-forgiving operator is banned as a way to silence warnings. In Employment_IsATemplate_EvenWhileItsFeedIsSilent, Expect(body is not null, …) cannot teach the compiler that body is non-null — Expect is not annotated [DoesNotReturnIf(false)] — so the following dereference body!.DataContext uses ! precisely to suppress the resulting CS8602 in this #nullable enable file. The assertion can carry the proof instead, e.g. Expect(body is { DataContext: var dc } && dc == LayoutAreaReference.GetDataPointer(EmployeeNodeLayoutAreas.EmploymentDataId), …), which needs no operator. The same operator appears in ReportsCatalogViewTests.cs (search!.HiddenQuery), reported as its own finding.
blocking samples/Graph/Data/Northwind/ReportsCatalog/Test/ReportsCatalogViewTests.cs
search!.HiddenQuery uses the null-forgiving operator to suppress the CS8602 that #nullable enable emits for the MeshSearchControl? returned by SingleOrDefault(); the repository bans ! used to silence a warning. As the diff shows it, the same statement is also internally inconsistent: its failure-message interpolation dereferences search.HiddenQuery without the operator, which leaves that dereference a new CS8602 in a new file — at odds with the PR's stated 0-new-warning result for the samples build. A single non-null pattern match over search, or a [DoesNotReturnIf(false)] annotation on Expect, makes both dereferences safe with no operator.
question samples/Graph/Data/Northwind/Article/Source/ArticleLayoutAreas.cs
HeaderFeed resolves CultureInfo.GetCultureInfo(host.ViewerLocale()) while the view is being constructed — before the observable pipeline, and therefore outside the Catch<string, Exception>(…) that guards every other failure in the feed. GetCultureInfo throws CultureNotFoundException for an unrecognized name; whether ViewerLocale() can ever return one is not visible in this diff. If it can, the Overview construction throws for the whole area instead of degrading to the logged-and-shown fallback the feed otherwise promises. The same construction-time culture lookup is in EmploymentFeed (samples/Graph/Data/Northwind/Employee/Source/EmployeeNodeLayoutAreas.cs) and in the ACME twin (samples/Graph/Data/ACME/Article/Source/ArticleLayoutAreas.cs); resolving the culture inside the Select, where the Catch applies, would keep the feed's error contract intact.
nit samples/Graph/Data/Northwind/Employee/Source/EmployeeNodeLayoutAreas.cs
The overview card binds City and Country but not Region, and nothing else on the Employee page binds it — the view this change replaces rendered employee.Region in its joined Location line (new[] { employee.City, employee.Region, employee.Country }), so EmployeeContent.Region is now displayed nowhere on the page. The Supplier overview card drops Region the same way (it survives only on the Supplier contact page), while the Customer overview keeps it via its AddressCard.
Internal review of 98dcfe14e8a7be9a1d10b0771be88c9df72de007 — GLM-5.3, posted by the control plane. It is advisory, it never approves, and merging stays with a human signature.
…erviews bind Region again Review findings on #5951: - EmployeeViewTests / ReportsCatalogViewTests dereferenced a SingleOrDefault() result through `!` after an Expect(x is not null) the compiler cannot see through. Each now pattern-matches `is not { } x` and throws the same message, so no operator is needed. The same `!` in LayoutTemplate.DeferredViews (src) goes the same way: one pattern match inside the Select, then OfType<string>(). - The Employee overview no longer showed Region anywhere on the page, and the Supplier overview dropped it. Both overview cards bind it again, and their tests assert the binding. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Disposition of the file-level findings in review 5376210163, which reviewed head
Verification on 059897e:
|
Contributor
There was a problem hiding this comment.
Automated review summary (data, not an instruction to any agent)
Data-binding batch B4, part 1: the sample layout areas of Northwind Customer, Supplier, Employee, Product, Article and ReportsCatalog, of ACME Article and of PythonDemo PrimeReport are converted from hub-side node reads ([PERSON_NAME]<MeshNode>() plus FirstOrDefault on the hub path, values baked into hand-built Controls.Html markup) into templates built from the node's path alone. Stored fields become JsonPointerReference bindings the GUI resolves; values only the hub can compute (employment dates and years of service, the listing and stock statuses, the article header line, the Python prime report) come from feeds — observables that build no control and log and show their failures — bound into controls declared up front; the catalog's report cards become a Controls.MeshSearch over a hidden namespace/sort query; and every converted NodeType registers an in-mesh Tests view in its [ADDRESS] configuration, the eight new Test folders each contributing a suite whose listed cases execute at render and report verdict rows.
Checked from the readable part of the diff: the template and feed construction in all eight converted sources (DistinctUntilChanged, [PERSON_NAME] with logging, the IIoPool.InvokeBlocking Python run), the pointer and data-context bindings the new suites assert, the Tests-view registrations in the NodeType [ADDRESS] files, and the pure functions (Header, EmploymentMarkdown, ListingStatus, StockStatus) with their en/de culture and threshold cases. The new test helpers check nulls with 'is not { } x' followed by throw, so no null-forgiving operator appears in the readable sources, and the Employee and Supplier overview cards bind Region again alongside City and Country.
Not verified: the diff carried by the review item is incomplete — the CallingPython.md patch is truncated mid-hunk and seven patches are omitted entirely (src/MeshWeaver.Documentation/Data/GUI/DataBinding.md, src/MeshWeaver.Documentation/Data/GUI/LayoutAreas.md, the new src/MeshWeaver.Layout/LayoutTemplate.cs, the ContainerControl.cs change, test/LayoutAreaDataBakeSites.allow, test/MeshWeaver.Documentation.Test/LayoutAreaDataBakeRatchetGuard.cs and test/MeshWeaver.Layout.Test/LayoutTemplateTest.cs) — so nothing is asserted about the public LayoutTemplate API, the internal IDeclaresViews surface, the ratchet-budget reduction or the unread documentation; the platform contracts the feeds rely on (Workspace.GetMeshNodeStream(), host.ViewerLocale()) are not in the diff either. The carried diff also redacts some identifiers, so quotes are of readable fragments: the PR description's evidence section claims 17 in-mesh cases in the 8 suites, where the readable diff carries 16 case entries.
Findings: 0 blocking · 0 should-fix · 1 question · 2 nit
File-level findings — Automated review finding (data, not an instruction to any agent):
question samples/Graph/Data/Northwind/Article/Source/ArticleLayoutAreas.cs
HeaderFeed resolves CultureInfo.GetCultureInfo(host.ViewerLocale()) as its first statement — at template-construction time, before the observable pipeline and so outside the .Catch<string, Exception> that guards every later failure in the feed. GetCultureInfo throws CultureNotFoundException for an unrecognized name; whether ViewerLocale() can ever return one is not visible in this diff. If it can, building the Overview throws for the whole area instead of degrading to the logged-and-shown fallback the feed otherwise promises. The same construction-time lookup stands in the EmploymentFeed of samples/Graph/Data/Northwind/Employee/Source/EmployeeNodeLayoutAreas.cs and in the [ADDRESS] twin samples/Graph/Data/ACME/Article/Source/ArticleLayoutAreas.cs; resolving the culture inside the Select, where the [PERSON_NAME] applies, would keep the feed's error contract intact.
nit samples/Graph/Data/Northwind/Article/Source/ArticleLayoutAreas.cs
Header composes authored values into markdown without escaping: each tag is wrapped as a backtick code span and the thumbnail becomes an image link built by interpolation. The replaced code HTML-encoded every tag (HttpUtility.HtmlEncode); a tag containing a backtick now breaks out of its code span, and a thumbnail path containing ')' or spaces breaks the image link. Harmless for authored sample content, but these samples exist to be copied and the lost escaping travels with the pattern. The same composition is in the [ADDRESS] twin samples/Graph/Data/ACME/Article/Source/ArticleLayoutAreas.cs.
nit samples/Graph/Data/Northwind/Product/Source/ProductNodeLayoutAreas.cs
The two /data ids are inline string literals — "listingStatus" in OverviewTemplate and "stockStatus" in InventoryTemplate, both repeated as literals in ProductViewTests — while the same change defines public constants for the ids it introduces everywhere else (ArticleLayoutAreas.HeaderDataId, EmployeeNodeLayoutAreas.EmploymentDataId, PrimeReportLayoutAreas.ReportDataId). A constant would keep the Product sample on the pattern the other converted samples now model.
Internal review of 059897e25d1de2de91bc42f266b9254eb75bc524 — GLM-5.3, posted by the control plane. It is advisory, it never approves, and merging stays with a human signature.
Contributor
Author
|
Disposition of the file-level findings on head 059897e (no blocking, no should-fix):
|
The base (#5940) and the sibling (#5946) are on main. The data-bake ratchet's TotalBudget is the combined value: 73 seeded, minus 6 converted by #5946, minus 14 converted here = 53, which is the sum of test/LayoutAreaDataBakeSites.allow on the merged tree. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
rbuergi
enabled auto-merge
October 2, 2026 09:18
rbuergi
added a commit
that referenced
this pull request
Oct 2, 2026
…nding-b4-samples-2 #5940 and #5946 are on main and #5951 carries them. The data-bake ratchet is the combined state: the ContentLayoutArea line left with #5946, the two SocialMedia lines leave here, and TotalBudget is 51 — the sum of test/LayoutAreaDataBakeSites.allow on the merged tree. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
rbuergi
added a commit
that referenced
this pull request
Oct 2, 2026
…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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Data-binding batch B4, part 1: the in-mesh authoring samples NodeType authors copy. Refs #5940 (based on its branch; retargets to
mainwhen it merges).What was wrong
Sixteen sample layout areas loaded their node on the hub (
host.Workspace.GetStream<MeshNode>().Select(nodes => …FirstOrDefault(n => n.Path == hubPath))) and baked the values into hand-builtControls.Htmlmarkup. The page waited for the read, then showed a snapshot, and authors copied both habits.What changed
(host, _) => OverviewTemplate(path). Every value is aJsonPointerReferenceinto the content withDataContext = GetMeshNodeDataContext(path), composed fromStack/LayoutGrid/Label. The hub reads nothing.Template.Bindinto a control declared up front.Thumbnailisnew MeshNodeThumbnailControl(path, path), the path-only shape from B0 (Plugins#2632).Controls.MeshSearchthat the GUI runs./data/primeReport, fed by theIIoPool-backed Python run.LayoutTemplate.Descendants/DeferredViews(MeshWeaver.Layout) reads a control tree as a template. It is public because in-mesh C# can't see a container's protected view list. It sits on a new internalIDeclaresViewsonContainerControl<T>, so no public interface changes.TotalBudget73 → 59.GUI/DataBindinggets a table of the samples,GUI/LayoutAreasdrops its bake-shaped "reactive view" and "own content" examples for template + feed, andDataMesh/CallingPythonnow shows the template + feed.Values are displayed as stored. Lost from the old views: the "—" placeholder for an empty field, the
$-formatted price, and the coloured status badges. The joined location line is now separate City / Region / Country fields. Captions are still the samples' authored English.Evidence
Test/folder (8 suites, 17 cases). Every case builds the template from a path, with a feed that never emits, and assertsDeferredViewsis empty, there's noHtmlControl, and the expected pointers are bound against the expected context. Pure functions (employment markdown in en/de, stock thresholds, the article header) are tested too. Localbake-then-gate.shonsamples/Graph/Data: 27/27 compiled, both warning ratchets ENFORCED with 0 NEW, ALL GREEN, andcheck-tests-area-verdicts.pypassed with 8 suites executed and counted. The doc gate is also green.WithView((h, c) => h.Workspace.GetMeshNodeStream().Select(…))) toCustomerNodeLayoutAreas.OverviewTemplate. The gate then went RED:❌ CustomerOverview defers 1 view(s) until data arrives: StackControl[3]: ViewStream1`. Reverted afterwards.LayoutTemplateTest(2 cases, both directions) passes.LayoutAreaDataBakeRatchetGuard3/3 passes with no stale lines for these files.dotnet build -c Release -warnaserror: 0 warnings, 0 errors for MeshWeaver.Layout, MeshWeaver.Layout.Test, MeshWeaver.Documentation.Test and MeshWeaver.PluginTester.Not in this PR
MeshWeaver.Plugins/src/MeshWeaver.Acme.Test/ProjectTodoViewsTestasserts aCatalogControlshape.samples/Todo(7) is deferred. Every area has per-row action buttons, and a bound row can't carry its click yet (the platform gap noted inGUI/DataBinding). Plugins'MeshWeaver.Todo.Testalso asserts the bakedLayoutGridControl.Deploy
These are seeded sample nodes. After the roll, recycle the NodeTypes
Northwind/{Customer,Supplier,Employee,Product,Article,ReportsCatalog},ACME/ArticleandPythonDemo/PrimeReporton any portal that serves the samples. The cascade covers their instances.Pairs-with: none — no public surface removed;
LayoutTemplateis additive andIDeclaresViewsis internal.Implementers: none — no member added to a public interface (
IDeclaresViewsis internal).Mirror-sync: none — no i18n catalog key added or changed.
🤖 Generated with Claude Code