Running sub-agents HTML resets internal scroll on every update by recreating the panel #1631

Summary

User report: “Updates to the running subagents html causes scrolling back to the top. The updating of that html should be incremental so it doesn't scroll every time.”

The actual [Running sub-agents] HTML panel in the agent chat WebView destroys and recreates its own scroll container on every activity, completion-state, or child-list notification. Once the panel overflows its height cap, an update discards the element holding its nonzero scrollTop; the newly inserted element starts at the top. The shared browser transport already supports incremental operations: the defect is the panel transformer's remove/full-append policy, not a whole-page reload or the separate Sub-agents browser/navigation list.

Reproduction

Suggested source-derived reproduction; not executed in the live application during this investigation. The user's precise agent count, viewport dimensions, deployment revision, and update cadence were not supplied.

1. Open a parent agent chat containing enough running children/nested activity for [Running sub-agents] to exceed its 25vh cap. Overflow depends on viewport and content, not a particular agent count.
2. Scroll inside that panel, so document.getElementById("subagent-panel-inner").scrollTop > 0. This is distinct from scrolling the main transcript/window.
3. Have a child emit more text/tool activity, change completion state while another child remains running, or add another running child.
4. Observe that the panel's inner element is replaced and its internal scroll position returns to the top.

Expected: update the affected keyed rows/content in place; leave the persistent panel and unaffected rows intact and preserve the user's reading position where content still exists. Main-chat auto-follow must retain its existing semantics.

Root Cause

Source baseline and confirmed event chain

Inspected clean checkout: C:\dev\Phantom.Workspaces-Skills\features, HEAD 1b86a98feb52d32a4bbfef82d624117a57b8fdd4 (2026-10-09, merge PR #1626). git status --short was empty before/after investigation. All paths below are relative to that code repository and all source references are pinned to this HEAD.

1. Live child publications become activity notifications. AgentChat.PartialResponseConflator builds updates off-thread, then awaits foreground UpdateRunningItem publication (Phantom.Workspaces.Llm.Core/AgentChat.cs:2091–2119). AgentRunningItems.SyncItems replaces changed references and individually adds/removes collection entries (Phantom.Workspaces.Llm.Core/AgentRunningItems.cs:115–140). These are notifications from one child's running output, not wholesale persisted-history replacement.

2. The UI adapter already retains activity identity internally, but HTML does not. AgentViewModel.AddSubAgentSlotEager creates RunningSubAgentDisplay and adds it to SubAgentDisplays (Phantom.Workspaces.Agent.Gui/ViewModels/AgentViewModel.cs:919–966). RunningSubAgentDisplay subscribes to running-message collections, updates existing recent activity on Replace, and raises ActivityChanged (Phantom.Workspaces.Agent.Gui/ViewModels/RunningSubAgentDisplay.cs:189–265):

   var key = new ActivityKey(item, itemIndex);
   // Replace updates recentActivity[existingIndex] instead of appending another line.
   if (changed)
       this.ActivityChanged?.Invoke(this, EventArgs.Empty);

   Completion changes are forwarded at :126–130. The remote adapter at :318–365 exposes no activity lines but forwards completion notifications and recreates child display wrappers. DOM keys therefore must use logical agent identity, not wrapper object reference or display name.

3. Every notification unconditionally destroys the scroll owner. ChatOutputHtmlModel constructs RunningSubAgentsHtmlTransformer over the display collection and its generation-bound sink (Phantom.Workspaces.Agent.Gui/ViewModels/DocumentModels/ChatOutputHtmlModels.cs:1969–2009). In Phantom.Workspaces.Agent.Gui/ViewModels/DocumentModels/RunningSubAgentsHtmlTransformer.cs:64–73,109–151:

   private void OnActivityChanged(object? sender, EventArgs e)
       => this.FullRender();
   private void OnCompletionStateChanged(object? sender, EventArgs e)
       => this.FullRender();
   
   // FullRender:
   this.RenderParentPanel();
   this.sink.RemoveContent(ChatOutputHtmlRenderer.SubAgentPanelInnerId);
   // ... if any child is Running:
   this.sink.UpdateContent(
       ChatOutputHtmlRenderer.SubAgentPanelSentinelId,
       ChatOutputUpdateLocation.Append,
       BuildPanelHtml(this.subAgents, this.ancestors));
   this.sink.ScrollToBottom();

   Collection changes also call FullRender. RenderParentPanel uses the same remove/append policy for parent-agent-panel-inner at :142–151, even for an unrelated child activity update. BuildPanelHtml makes a fresh inner wrapper at :164–178. Rows have only a CSS class/optional depth and navigation buttons have data-navigate-agent-id; neither rows nor activity <li> elements have stable DOM IDs/keys (:232–271).

4. The removed inner element is specifically the nested scroll container. Phantom.Workspaces.Agent.Gui/Assets/chat-output-shell.html:491–502,558–563:

   .running-subagents-panel,
   .running-parent-agent-panel {
     /* ... */
     max-height: 25vh;
     overflow-y: auto;
   }

   The sentinel survives, but it does not own that scroll state. Keeping only the sentinel persistent is insufficient.

5. Transport executes those destructive operations faithfully; it does not reload the document for each panel update. AgentChatOutputControl.UpdateContent/RemoveContent serialize commands with ChatOutputBrowserCommands (Phantom.Workspaces.Agent.Gui/Controls/AgentChatOutputControl.axaml.cs:137–165). ControllableWebViewControl queues UI-thread messages, auto-batches them, and dispatches the ordered array through window.hostBridge.receiveMessage (Phantom.Workspaces.Gui.Shared/Controls/ControllableWebViewControl.cs:112–160,193–251). Browser applyCommand does exactly:

   // update / append:
   target.insertAdjacentHTML("beforeend", command.content);
   // remove:
   if (element) { element.remove(); }

   See chat-output-shell.html:1146–1210. The fresh element cannot inherit the removed element's internal scrollTop, focus or selection.

Why existing scroll handling and tests miss it

ScrollToBottom is gated by AgentViewModel.AutoScrollEnabled (AgentChatOutputControl.axaml.cs:158–165), and the browser implementation calls window.scrollTo(0, document.body.scrollHeight), not the panel's scrolling element (chat-output-shell.html:1112–1119). The programmatic-mutation gate and scrollState listener likewise deal with main-window follow behavior (:1129–1144,1224–1249). They do not preserve the removed panel element. No separate intentional inner-panel auto-follow policy is implemented in this path.

The unit test SubAgentPanel_Update_RemovesOldInnerAndAppendsNewInner explicitly requires remove + append + scroll after an activity notification (Phantom.Workspaces.Agent.Gui.Tests/RunningSubAgentsHtmlTransformerTests.cs:452–471). The misleadingly named RunningSubAgentsPanel_UpdatedInPlace_WhenPanelAlreadyPresent (:321–343) checks an append payload, not node identity. Parent activity tests require the same destructive policy (:669–689).

The native-browser test SubAgentPanel_Update_SentinelPresent_InnerReplaced (Phantom.Workspaces.Agent.Gui.WebViewTests/ChatOutputBrowserIntegrationTests.cs:2092–2131) checks sentinel survival, text, and disappearance when complete, but never scrolls inside an overflowing panel or compares old/new DOM objects. Existing AutoScroll_SubAgentPanelAppendedWhileAtBottom_RemainsEnabledAndScrollsToNewBottom and AutoScroll_SubAgentPanelAppendedAfterUserScrolledUp_DoesNotForceScroll (:2253–2347) cover the main page, not panel scrollTop.

Diagnosis scope: source establishes the destructive nested-scroll-container reset. No live/browser reproduction or timing measurement was performed, and there is no evidence that out-of-order async delivery is the cause of this report. High-frequency text/status notifications amplify this policy; this panel currently has no duration/progress timer markup to diagnose separately.

Affected Files

Design / Fix

1. Retain both actual scroll hosts, not just sentinels. Initialize subagent-panel-inner and parent-agent-panel-inner once with their panel classes, hide/empty them when inactive, and show them when appropriate. Never remove/recreate an existing scroll host for routine updates. Empty-state tests should assert no visible/stale panel content, not demand destruction of the host.

2. Stable keyed rows/groups, scoped patches. Build row IDs from panel role plus logical agent/session identity (and ancestry where needed to prevent collisions), using a safe, deterministic ID encoder and normal HTML escaping. Nested child groups and activity containers need stable IDs too. Do not use display names, mutable status, collection positions, or remote wrapper references as identity. Adapt the existing (running item, item index) activity identity into a retained renderer key so replacing streamed text keeps its node and trimming the five-line activity buffer does not re-key survivors. Reconcile children recursively: insert only new nodes, remove only obsolete nodes, and move existing nodes using native DOM operations instead of remove/reinsert markup. Keep running-only filtering and current depth/order semantics.

3. Reuse the shared HTML sink/bridge; do not invent a parallel panel WebView. Existing Append/Before/After/Remove suffice for inserts/removals at stable targets. Existing Replace replaces outerHTML and must not be used on panel hosts or unchanged row/group wrappers. For in-place text/attribute/child patches, introduce a small reusable reconciliation operation/helper through the same IChatOutputHtmlSink.UpdateContent signature and applyCommand. No existing generic keyed browser reconciliation helper was found in this checkout; the proposed helper is new, not an API claimed to exist.

   Illustrative sketch (new identifiers/API are explicitly proposed; not an implementation):

   // Proposed new enum member: ChatOutputUpdateLocation.Reconcile.
   // ToWireLocation maps it to "reconcile"; ChatOutputBrowserCommands.Update
   // already serializes path/location/content/generation.
   private void UpdateChangedRow(
       string rowId,
       string rowHtml)
   {
       if (this.lastRowHtml.TryGetValue(rowId, out var previousHtml)
           && previousHtml == rowHtml)
       {
           return;
       }
   
       this.sink.UpdateContent(
           rowId,
           ChatOutputUpdateLocation.Reconcile,
           rowHtml);
       this.lastRowHtml[rowId] = rowHtml;
   }

   // New branch inside the existing update/location switch,
   // still covered by beginProgrammatic/endProgrammatic:
   case "reconcile":
     reconcileHtmlElement(target, command.content); // proposed shared helper
     break;

   reconcileHtmlElement parses renderer-owned escaped HTML into a detached template and updates the existing target: reconcile keyed children; update changed attributes/text only; leave equal text nodes and browser-owned state untouched. Structural changes use native moves for matching nodes. Avoid whole innerHTML, outerHTML, replaceChildren, or remove() on surviving keyed hosts/groups. Prefer dirty-row updates and cached rendered values to rebuilding/transmitting the whole panel for every token. Add/remove/nested changes reconcile only the relevant keyed group, not the document.

4. Preserve state deliberately. Retained panel/row nodes preserve nested scroll position, focus, selection in unchanged content, and any existing interactive expansion state. For text actively being edited or a removed node, preserve selection/reading anchors where still meaningful and clamp naturally when content disappears; do not promise an impossible unchanged offset after removal of the selected content. When additions/removals above the viewport move content, use a surviving keyed visible-row anchor plus offset as a secondary adjustment if native anchoring is insufficient. Scroll restoration is not the primary fix: element retention and incremental DOM mutation are.

   Preserve the delegated [data-navigate-agent-id] click handling (chat-output-shell.html:1260–1266), latest labels/targets, HTML escaping, and any applicable helper/observer initialization. Do not introduce panel auto-follow merely because the main transcript auto-follows. Continue main-page ScrollToBottom requests through the existing AutoScrollEnabled gate; ensure panel wheel gestures do not accidentally change transcript follow state.

5. Correct lifecycle and update ordering without widening the diagnosis. Subscribe/unsubscribe recursively to descendant collection/activity/completion events so a changed nested child gets a patch without requiring unrelated top-level activity; unregister on removal/disposal. Reconcile remote display-wrapper replacement by logical keys. Preserve generation-bound publication suppression and existing ordered UI-thread bridge delivery. If dirty-row work is deferred/coalesced, apply only the latest state per key/generation, cancel disposed work, and prevent a late snapshot from resurrecting a removed row. Use existing auto-batching/render-completion mechanisms; do not introduce explicit batches that overtake already queued updates. No async reordering has been demonstrated as the current root cause.

Related issue boundaries

Closed #1274 explains the existing capped/self-scrolling panel; closed #1202 covers main-chat auto-follow, which must remain working. #1619 concerns which idle children count as running, not this DOM/scroll reset; keep its lifecycle semantics independent. #1627 concerns transcript/history rendering and typing latency; no shared measured performance trigger is established here, so this issue does not claim to fix it.

Expected Tests

All test classes below already exist. Test names marked new are proposed in Subject_Scenario_ExpectedOutcome form. Existing methods cited below must be preserved/adapted, not mistaken for new tests.

Use the existing native WebView fixture, CreateModel, EvalAsync, and WaitForFrameSyncAsync/pending-message flushing. Unit recordings cannot establish nested scroll preservation. No tests were run or added for this filing-only investigation.

Acceptance Criteria

• Routine activity/status updates retain the actual subagent and parent panel scroll-container DOM objects.
• Surviving keyed rows/groups and unchanged content nodes remain intact; only relevant changes are patched. Add/remove/reorder updates do not recreate the whole panel/document.
• An overflowing panel does not jump to the top after updates while the user's visible content survives; anchor adjustment/clamping handles genuine content removal.
• Correct current running rows, nested depth/order, labels, recent activity, empty-state visibility and navigation are maintained without stale content.
• Main-transcript auto-follow and user opt-out continue working; no new forced inner-panel follow policy.
• Selection, focus, link handlers and applicable expansion state survive updates where their content survives.
• Burst/coalesced updates converge to the latest active generation; removal/disposal cannot be undone by stale queued rendering.
• Destructive-policy tests are replaced with identity/state assertions, and native-browser tests exercise nonzero panel scrollTop.

Investigation Limitations

Static source diagnosis only, pinned to the inspected clean HEAD; deployed revision and exact user conditions are unknown. No screenshots were supplied, attached, or uploaded. No code edits, commits, pushes, dependency installs, or runtime performance measurements were made.