Tool expanders in agent chat view have no effect and don't collapse tool lists #25

Activity

1. JoshuaRowePhantom

   added

   bugSomething isn't workingSomething isn't working

   on Jun 24, 2026
2. JoshuaRowePhantom commented on Jun 24, 2026

   

   JoshuaRowePhantom

   on Jun 24, 2026

   OwnerAuthor

   More actions

   Investigation Findings

   Root Cause

   Two compounding causes:

   Primary — Mode=TwoWay in Style Setter is silently OneWay in Avalonia 12

   Files:

   • Phantom.Workspaces.Agent.Gui/Controls/AgentChatEditorControl.axaml, line 159
   • Phantom.Workspaces.Agent.Gui/Controls/AgentChatToolsDetailControl.axaml, line 24

   <Setter Property="IsExpanded" Value="{Binding IsExpanded, Mode=TwoWay}" />

   In Avalonia 12, bindings inside Style Setter elements only work OneWay (Source → Target). Mode=TwoWay is silently ignored. When the user clicks an expander, TreeViewItem updates its own IsExpanded locally but the ViewModel's IsExpanded is never written back. The ViewModel retains its original value.

   TreeViewItem exposes dedicated ExpandedEvent/CollapsedEvent routed events for exactly this purpose — they are the correct mechanism and are not used.

   Note: AgentEditorNavigationItemViewModel.ToggleExpandCommand (line 28) would also work but is never bound in any XAML.

   Compounding — BuildEditorTree discards all expansion state on every tool change

   File: Phantom.Workspaces.Agent.Gui/ViewModels/AgentViewModel.cs

   ToolsChanged fires on every MCP server init and every SetToolEnabledAsync call (user checks/unchecks a tool). It calls BuildEditorTree, which calls EditorItems.Clear() and creates brand-new AgentEditorNavigationItemViewModel instances with isExpanded = false for all nodes. Any expansion state set by the user is destroyed.

   Combined effect: clicking an expander does nothing that survives the next tool change (or the initial MCP server discovery).

   ---

   Missing Tests

   1. IsExpanded_RaisesPropertyChanged_WhenSet — setting IsExpanded = true on AgentEditorNavigationItemViewModel fires PropertyChanged
   2. ToggleExpandCommand_TogglesIsExpanded_FalseToTrue and _TrueToFalse
   3. ToggleExpandCommand_CanExecute_OnlyWhenHasChildren
   4. BuildEditorTree_PreservesToolNodeExpansionState_AfterToolsChanged ⚠️ key test — set IsExpanded = true on an MCP server nav item, raise ToolsChanged, assert the node is still expanded
   5. BuildEditorTree_PreservesChatToolsCollapsedState_AfterToolsChanged — collapse "chat-tools", raise ToolsChanged, assert it stays collapsed
   6. TreeViewItemExpanded_UpdatesViewModelIsExpanded (Avalonia UI test) — fire TreeViewItem.ExpandedEvent on a NavigationTree container, assert the DataContext ViewModel's IsExpanded becomes true
   7. TreeViewItemCollapsed_UpdatesViewModelIsExpanded — same for collapsed

   ---

   Recommended Fix

   Fix 1 — Replace broken TwoWay Style Setter with ExpandedEvent/CollapsedEvent handlers

   AgentChatEditorControl.axaml — drop Mode=TwoWay (it was a no-op):

   <Setter Property="IsExpanded" Value="{Binding IsExpanded}" />

   AgentChatEditorControl.axaml.cs — add routed event handlers in constructor:

   public AgentChatEditorControl()
   {
       this.InitializeComponent();
       this.DataContextChanged += this.OnDataContextChanged;
       this.NavigationTree.AddHandler(TreeViewItem.ExpandedEvent, OnTreeViewItemExpanded);
       this.NavigationTree.AddHandler(TreeViewItem.CollapsedEvent, OnTreeViewItemCollapsed);
   }
   
   private static void OnTreeViewItemExpanded(object? sender, RoutedEventArgs e)
   {
       if (e.Source is TreeViewItem { DataContext: AgentEditorNavigationItemViewModel vm })
           vm.IsExpanded = true;
   }
   
   private static void OnTreeViewItemCollapsed(object? sender, RoutedEventArgs e)
   {
       if (e.Source is TreeViewItem { DataContext: AgentEditorNavigationItemViewModel vm })
           vm.IsExpanded = false;
   }

   Apply the same pattern to AgentChatToolsDetailControl.axaml (drop Mode=TwoWay) and AgentChatToolsDetailControl.axaml.cs (add ToolsTree.AddHandler(...) in constructor).

   Fix 2 — Preserve expansion state across BuildEditorTree rebuilds

   AgentViewModel.cs — capture expansion state before clearing, restore it when recreating nodes:

   private void BuildEditorTree()
   {
       var selectedId = this.SelectedEditorItem?.Id;
   
       // Capture expansion state before destroying old tree
       var expandedIds = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
       CollectExpandedIds(this.EditorItems, expandedIds);
       bool isFirstBuild = expandedIds.Count == 0;
   
       this.EditorItems.Clear();
       var toolNavigationItems = this.BuildToolNavigationItems(this.Tools, expandedIds);
       // ... rebuild root with isExpanded: expandedIds.Contains(id) for each node ...
       // "chat-tools" default: isFirstBuild || expandedIds.Contains("chat-tools")
   }
   
   private AgentEditorNavigationItemViewModel BuildToolNavigationItem(
       AgentChatToolViewModel tool, HashSet<string> expandedIds)
       => new(..., isExpanded: expandedIds.Contains(tool.Id));  // was always false
   
   private static void CollectExpandedIds(
       IEnumerable<AgentEditorNavigationItemViewModel> items,
       HashSet<string> expandedIds)
   {
       foreach (var item in items)
       {
           if (item.IsExpanded) expandedIds.Add(item.Id);
           CollectExpandedIds(item.Children, expandedIds);
       }
   }

   ---

   Side Effects

   • No memory leak — AddHandler on the TreeView uses event bubbling; no per-container subscriptions needed.
   • No infinite loop — when the handler sets vm.IsExpanded = true, the OneWay setter re-asserts true on the already-true TreeViewItem; Avalonia detects no change and does not re-fire ExpandedEvent.
   • First-build defaults preserved — the isFirstBuild guard keeps "chat-tools" starting expanded on first build.
   • Tool ID stability required — expansion state preservation relies on tool IDs being stable across rebuilds (acceptable).
3. JoshuaRowePhantom

   added a commit that references this issue on Jun 25, 2026

   Fix agent chat editor expander defaults (#24, #25)

   ...

   f53d7d0
4. JoshuaRowePhantom

   closed this as completedon Jun 25, 2026
5. JoshuaRowePhantom

   added

   verifiedIssue has been verifiedIssue has been verified

   on Jun 30, 2026
6. JoshuaRowePhantom commented on Jun 30, 2026

   

   JoshuaRowePhantom

   on Jun 30, 2026

   OwnerAuthor

   More actions

   Verification passed

   Checked: Commit 53d7d0 (tag: ix-agent-chat-editor-expanders) on the eatures branch, referencing issues #24 and #25.

   Files inspected:

   • Phantom.Workspaces.Gui.Shared/Styles/SharedStyles.axaml — the �ntity-card-tree TreeViewItem template
   • Phantom.Workspaces.Agent.Gui/ViewModels/AgentEditorNavigationItemViewModel.cs — IsExpanded property and ToggleExpandCommand
   • Phantom.Workspaces.Agent.Gui/ViewModels/AgentChatToolsDetailViewModel.cs — CloneNavigationItem expansion defaults
   • Phantom.Workspaces.Agent.Gui.Tests/MainWindowAxamlTests.cs — XAML structure tests
   • Phantom.Workspaces.Agent.Gui.Tests/AgentViewModelNavigationTests.cs — expansion-state tests
   • Phantom.Workspaces.Agent.Gui.Tests/AgentChatToolsDetailViewModelTests.cs — collapse/expand default tests

   Root cause addressed: The �ntity-card-tree TreeViewItem template rendered its child ItemsPresenter unconditionally. Toggling an expander changed the glyph but never hid the children. Fixed by binding ItemsPresenter.IsVisible to IsExpanded.

   Result: Implementation found, all verification criteria satisfied, tests present and green.

   • SharedStyles.axaml now contains inside the �ntity-card-tree TreeViewItem template.
   • MainWindowAxamlTests.cs has two assertions (with a comment referencing issue Tool expanders in agent chat view have no effect and don't collapse tool lists #25) that verify the binding is present in SharedStyles.
   • AgentViewModelNavigationTests.cs and AgentChatToolsDetailViewModelTests.cs cover collapse/expand defaults for top-level and child navigation items.
   • No disabled/quarantined tests, no unresolved TODOs, no timing-dependent tests.