Preserve Debug Steps tree state across filter sessions - #3919
Merged
Conversation
The filter drove every row's IsExpanded from the single IsFiltering flag via a TreeViewItem style setter. Expansion lived nowhere else: user gestures (expander arrow, double-tap, arrow keys) all write the property with SetCurrentValue, which the style binding overwrites on every flip of the flag. Starting a filter therefore destroyed the expansion state the user had built up, and clearing it collapsed the whole tree, burying the still-selected row under collapsed groups. Row state (visibility + expansion) now lives on a StepNodeViewModel wrapper per step, two-way bound from the style, so gestures persist in the view-model. A filter session snapshots expansion on entry, hides non-matches and opens only the paths to matches while typing, and on exit restores the snapshot and re-expands the selected step's ancestors so the selection stays visible. Wrapping is skipped for reference-equal step lists because step replays re-report the same run and a rebuild would wipe the state mid-navigation. Assisted-by: Claude:claude-fable-5:Claude Code
Contributor
There was a problem hiding this comment.
Pull request overview
Improves the Debug Steps tool pane (debug-only Avalonia UI) so filtering the step tree no longer destroys user-driven expansion state, and ensures the currently selected step is revealed/centered after filter changes.
Changes:
- Moves per-row UI state (visibility + expansion) into a new
StepNodeViewModelwrapper so filtering and user gestures update stable view-model state rather than transientTreeViewItemcontainers. - Reworks
DebugStepsPaneModelfiltering to snapshot/restore expansion across filter sessions, expand paths to matches, and request selection centering after filter updates. - Updates the view and tests accordingly, removing the old XAML multi-binding converter and adding headless tests that pin the new behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| ILSpy/Views/DebugSteps.axaml.cs | Adds view-side attach/detach wiring and logic to center the selected tree row after filter-driven expansion changes. |
| ILSpy/Views/DebugSteps.axaml | Switches TreeView row bindings to view-model-owned IsVisible / two-way IsExpanded, removing the prior converter-based filtering. |
| ILSpy/Views/DebugStepFilterConverter.cs | Removes the old IMultiValueConverter-based filtering implementation. |
| ILSpy/ViewModels/StepNodeViewModel.cs | Introduces a wrapper view-model that carries per-row UI state and parent/child relationships. |
| ILSpy/ViewModels/DebugStepsPaneModel.cs | Implements filter-session snapshot/restore and match-path expansion; adds a view notification for selection centering. |
| ILSpy.Tests/Views/DebugStepsTests.cs | Updates existing tests to use the wrapper tree and pane-side filtering behavior. |
| ILSpy.Tests/Views/DebugStepsFilterStateTests.cs | Adds new headless tests covering filter round-trips, match visibility/expansion, selection preservation, and centering. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Filtering and un-filtering the Debug Steps tree re-arranges rows around the selection: matches surface under newly expanded groups and the restore step re-opens the selected step's ancestors. Without scrolling, the selected row regularly ended up outside the viewport even though it was expanded into view. The ViewModel now raises a reveal request after each filter pass while a visible step is selected; the View answers it by centering the row, because scrolling needs containers and the ScrollViewer. The measurement anchors on the row header since an expanded group's container spans its whole subtree, and it runs at Loaded dispatcher priority so freshly expanded containers have valid geometry. Assisted-by: Claude:claude-fable-5:Claude Code
siegfriedpammer
force-pushed
the
debugsteps-filter-tree-state
branch
from
July 27, 2026 16:45
2b40970 to
e5cb1b6
Compare
The explicit 15-second budget timed out on a loaded Windows Release CI runner (a different test of this fixture on each attempt, while the sibling workflow run passed the same head). The waits are completion-bounded - the predicate observes the end of an async re-decompile - so the 60-second Waiters default costs nothing when healthy and only converts runner slowness from a red run into a longer green one. Assisted-by: Claude:claude-fable-5:Claude Code
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.
The Debug Steps pane's filter box destroyed the tree state it filtered: typing the first character expanded every row (wiping the expansion the user had built up), and clearing the filter collapsed the whole tree, leaving the still-selected step buried and off screen.
Root cause
The XAML drove every row's
IsExpandedfrom the singleIsFilteringflag through aTreeViewItemstyle setter. All user expansion gestures (expander arrow, header double-tap, arrow keys) write the property viaSetCurrentValue, and a current value is discarded whenever the active style binding produces a new value - so each flip ofIsFilteringoverwrote every row's manual state. Expansion lived nowhere else:Stepper.Nodehas no UI state, and the containers were the only carrier.Fix
StepNodeViewModelcarryingIsVisibleand a two-way-boundIsExpanded, making row state authoritative in the view-model instead of dying with the containers.DebugStepFilterConverteris gone; the matching logic moved into the pane model where it is unit-testable.Tests
Four new headless view-level tests in
DebugStepsFilterStateTestspin the behavior (expansion round-trip, match revealing, selection reveal, centering); they simulate gestures withSetCurrentValue, matching what realTreeViewItemgestures do. Full ILSpy.Tests suite: 1073 tests, 0 failures.This PR was authored by an AI agent (Claude Code) operated by @siegfriedpammer.
🤖 Generated with Claude Code