Repository navigation
chat: Avoid transient sticky row rebuilds during resize - #340147
Open
Benjamin Christopher Simmonds (benibenj) wants to merge 1 commit into
Open
Benjamin Christopher Simmonds (benibenj) wants to merge 1 commit into
Benjamin Christopher Simmonds (benibenj) wants to merge 1 commit into
Conversation
Measure invalidated sticky source ranges before the existing tree refresh sees estimated geometry. Keep the original tree lifecycle and content invalidation behavior, with regression coverage for resize identity, measurement, focus, and streaming/hidden updates.\n\nPart of #339863. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Benjamin Christopher Simmonds (benibenj)
enabled auto-merge (squash)
October 6, 2026 21:29
Copilot started reviewing on behalf of
Benjamin Christopher Simmonds (benibenj)
October 6, 2026 21:29
View session
Contributor
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The focused fix is consistent with the layout lifecycle and is covered by comprehensive regression tests.
Review effort: Balanced
Findings: None
What changed in this PR
Synchronizes sticky-row geometry measurement during chat width changes to prevent transient rebuilds while preserving focus and editor identity.
Changes:
- Refreshes invalidated sticky source ranges before the tree refresh.
- Adds regression coverage for resizing, streaming, visibility, focus, scrolling, and editor retention.
| File | Description |
|---|---|
chatListRenderer.ts |
Measures sticky source geometry synchronously on width changes. |
chatListWidget.test.ts |
Adds real-editor sticky-content retention tests. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
roblourens
approved these changes
Oct 6, 2026
This branch has not been deployed
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.
Part of #339863; this does not resolve the whole multi-pane resizing issue.
Summary
Width layout clears the cached sticky source geometry, but previously deferred its measurement until the next animation frame. The existing tree refresh could therefore see estimated geometry, rebuild the sticky row, and then rebuild it again once the measured geometry arrived.
Run the existing source-range measurement synchronously before that tree refresh. This is one production method-call change: no generic-tree changes, new caching/lifetime machinery, debounce, setting, or skipped measurement. The existing width guard and content/source invalidation paths remain in place.
Add regression coverage for unchanged/height-only layout, width measurement and sticky/editor identity, focus/scroll anchors, and streaming/hidden updates using real code-block editors.
Validation
npm run transpile-client- passed../scripts/test.sh --run src/vs/base/test/browser/ui/tree/objectTree.test.ts --run src/vs/workbench/contrib/chat/test/browser/widget/chatListWidget.test.ts --run src/vs/workbench/contrib/chat/test/browser/widget/chatListRenderer.test.ts- 832 passing, one pre-existing pending../scripts/test.sh --run src/vs/workbench/contrib/chat/test/browser/widget/chatListWidget.test.ts --grep "width changes keep sticky templates and code editors while updating measured geometry"- fails on the original baseline as expected; passes with this fix.npm run typecheck-client- passed.npm run eslint -- src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts src/vs/workbench/contrib/chat/test/browser/widget/chatListWidget.test.ts- passed.git --no-pager diff --check- passed.Real current-source Code OSS Agents windows, inspected with
@hediet/dbgjs, also passed streaming while user-scrolled, hidden updates, follow-tail, sticky transitions, sash dragging, font/zoom changes, code/diff model identity, drafts, focus, and removal/disposal checks.Native measurements
Compared base
564de3cb8366c7a23c2a1235ff3e8597566b4075, an earlier broader optimization, and this smaller fix. Three fresh native lifecycles per variant in balanced order; 1/3/6 populated panes, eight Markdown/TypeScript-code-block turns per pane, and combined/height-only/width-only resizing: 81 scenarios. Resizes use realBrowserWindow.setBounds. Complete lifecycles were serialized; scored timings use no profiler, trace, or operation-counting wrappers.Six-pane medians:
The fixed combined p95 ranged from 66.7 to 66.9 ms across the three runs. Separate dbgjs counting found zero sticky row/template creations or disposals in the fixed six-pane combined resize run, versus about 1.29 per chat layout on the baseline.
These are local development-build observations, not statistical guarantees. Height-only work is essentially unchanged, and significant style/layout cost remains. No paid provider orchestration was involved. All temporary app/debugger processes and profiles were cleaned up.