Repository navigation
fix(blazor): cure the render-storm circuit wedge (re-bind guard) + surface bind errors - #151
Merged
Merged
Conversation
…-binds + surface bind errors BlazorView.OnParametersSet re-disposed and re-subscribed ALL data bindings on EVERY parent render. The sync stream is ReplaySubject-backed, so each re-subscribe replays → StateHasChanged → the parent re-renders → OnParametersSet → … a self-sustaining client-side render loop that pegs the Blazor circuit (the recurring "chat freeze"/ wedge; CPU to multiple cores, no data write in flight). Guard it: skip the re-bind when no binding-relevant parameter actually changed (ViewModel / Stream / Model / Context by reference — immutable records/handles; Area / DataContext by value). Identical inputs ⇒ identical bindings, so skipping is correct and cannot stale a binding; it just stops the spurious re-subscribe→replay that sustained the loop. This is client-side only — it does NOT touch the server RenderArea pipeline (unlike the earlier generator-dedup attempt that broke submit). Also surface the swallowed bind fault: the DataBind catch only logged, freezing the area silently. It now calls SurfaceError (modal + inline) so a wedge shows what broke.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a recurring Blazor circuit “render-storm” wedge by preventing redundant data-binding teardown/rebuild cycles on no-op parent renders, and by surfacing previously-swallowed binding faults to the UI so failures don’t present as a silent “freeze”.
Changes:
- Add a parameter snapshot + guard in
BlazorView.OnParametersSetto skip re-binding when binding-relevant inputs are unchanged. - Surface unexpected
DataBindfailures viaSurfaceError(modal + inline) instead of only logging them.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Test Results (shard 1)1 391 tests +116 1 390 ✅ +116 6m 35s ⏱️ + 1m 33s Results for commit 5b17d11. ± Comparison against base commit 583764c. This pull request removes 6 and adds 122 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Contributor
Contributor
Test Results (shard 0)1 478 tests - 122 1 478 ✅ - 122 3m 43s ⏱️ - 1m 55s Results for commit 5b17d11. ± Comparison against base commit 583764c. This pull request removes 122 tests.♻️ This comment has been updated with latest results. |
Contributor
Test Results (shard 2) 15 files + 1 15 suites +1 7m 12s ⏱️ +17s Results for commit 5b17d11. ± Comparison against base commit 583764c. This pull request removes 27 and adds 46 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Contributor
Test Results 54 files + 1 54 suites +1 20m 1s ⏱️ -5s Results for commit 5b17d11. ± Comparison against base commit 583764c. This pull request removes 27 and adds 44 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
…u + thread lists) Traced the recurring circuit wedge to two reactive subscriptions that fire StateHasChanged on structurally-identical re-emissions — the re-bind guard treated the symptom (child re-binds); these are the roots. 1) Whole-page cascade. The client menu read (MenuStreamExtensions.GetMenu) had NO dedup, and NodeMenuItemDefinition compared its Children list by REFERENCE — so the layout-area stream's per-render Full re-emission delivered a "changed" menu every time and PortalLayoutBase re-rendered the WHOLE page (every area, incl. search-*). Fix: NodeMenuItemDefinition value equality compares Children by sequence; GetMenu gets DistinctUntilChanged(MenuItemsSequenceComparer). Only a real menu change now re-renders the layout. 2) Per-token chat churn. ThreadChatView's my-threads / child-threads / resume Hub.GetQuery subscriptions rebuilt the list + StateHasChanged UNCONDITIONALLY, and a streaming round re-emits the whole snapshot many times/second. The nav menu shows only path+name, so dedup on that visible shape (VisibleThreadListChanged) — a content-only churn no longer re-renders. Plus cache GetHeaderCell by threadPath so the header area gets a stable ViewModel reference (lets the re-bind guard skip it). Both fixes sit at the read/subscription layer — NOT the LayoutAreaHost control-generator input (that earlier attempt broke submit by skipping RenderArea's data push).
…de panel Selecting a thread previewed it in the (cramped) side panel via SetContentPath. Per user preference, navigate the main pane to the thread (full width) and collapse the side panel instead. (Separable from the render-storm fix; bundled for one local roll.)
… correctly
- Navigate to a valid landing + open the chat side panel via the header toggle
(title="Chat"); the composer is a Monaco editor (was: guessed selector on an
invalid /User/{id} URL, so it never chatted).
- Reconnect assertion checks VISIBILITY, not presence — #components-reconnect-modal
is always in the DOM (hidden), so Count>0 was a permanent false positive.
- Container check polls a few seconds (tolerates the momentary re-mount on a rapid
mid-stream submit; only a PERMANENT teardown = the storm).
Verified against the fix image: passes; 1× re-bind per area (was 37-59×), CPU 300m
peak (was 1.2+ cores).
…per-token re-render - BuildHarnessCommandCompletions now sources the active CLI harness's OWN slash-commands AND skills (probed from the CLI init via IHarnessRuntimeInfo → harnessRuntimes), so typing / under Claude Code / Copilot lists ALL its commands + skills; each forwards 1:1 to the CLI (existing dispatch). - HeaderView: dedup the aggregated change-set on its visible summary (path/op/version) before BuildHeader — CollectUpdatedNodes re-emitted per streamed token, re-rendering the Header layout area (~128x in the e2e capture). A residual render-storm loop.
Selecting a thread navigated to /{threadPath} + collapsed the side panel, but a Thread
node has no main-pane renderer — the chat vanished ('entering chat makes it disappear').
Restore the working side-panel open (SetContentPath). A real full-width thread view needs
a dedicated main-pane chat route, not a raw path nav.
Status chips (harness / model / effort / agent) are now <button>s that open the SAME combobox the matching slash-command uses — reusing HandleSlashCommandAsync → the skill's Pick action → OpenPicker (no hand-rolled dropdown): - harness → runtime switcher (all harnesses) - mesh agent / model → the node-pick combobox - ClaudeCode/Copilot model + effort → forwarded 1:1 to the CLI (their own, not mesh nodes) Emoji glyphs replaced with crisp inline SVG (currentColor) per type via ChipSvg(kind). Button chrome reset in CSS; .is-clickable adds cursor + hover.
…-code The '/' trigger fired the command completion at line-start OR after any whitespace, so a '/' inside code — paths (cd /etc), // comments, a / b, http:// — popped the command list. Harmless when that list was 3 items; now that it lists EVERY CLI command + skill it churned Monaco → 'stuck when entering code'. Gate '/' to line 1, only optional whitespace before it (a real command is the first thing typed). '@' references are unchanged.
Resolve the Monaco completion-provider conflict by combining both fixes: the '/' trigger keeps this branch's composer-start-only rule (line 1, whitespace-prefix only — no command popup mid-code), the '@' trigger keeps main's newer lastIndexOf word-boundary implementation (punctuation-safe, email- and in-path-@ excluded). Verified: node --check parses; MeshWeaver.Blazor + Blazor.Portal build Release -warnaserror clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CS8604 under -warnaserror after merging main (main carries the older test iteration that interpolated BaseUrl). Available (asserted via SkipUnless at the top) guarantees BaseUrl is non-null. Co-Authored-By: Claude Fable 5 <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
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.
Problem
The recurring "chat freeze"/wedge (#100): the Blazor circuit pegs to multiple cores with no data write in flight — a pure client-side render storm.
Root cause
BlazorView.OnParametersSetre-disposed + re-subscribed all data bindings on every parent render. The sync stream is ReplaySubject-backed, so each re-subscribe replays →StateHasChanged→ the parent re-renders →OnParametersSet→ … a self-sustaining loop.Fix (client-side only — does NOT touch the server RenderArea pipeline, unlike the reverted generator-dedup attempt that broke submit)
ViewModel/Stream/Model/Contextby reference (immutable records/stable handles),Area/DataContextby value. Identical inputs ⇒ identical bindings, so skipping is correct and cannot stale a binding; it just stops the spurious re-subscribe→replay that sustained the loop.DataBindcatch only logged, freezing the area silently. It now callsSurfaceError(modal + inline) so a wedge shows what broke.Test
Repro:
ChatRepeatedlyNoVanishTest(Playwright, ≥5 chats incl. mid-stream). Deployed to memex.localhost for live CPU verification.🤖 Generated with Claude Code