chat: add typed scrollbar markers to the Chat View transcript#322312
Closed
matthewcorven wants to merge 52 commits into
Closed
chat: add typed scrollbar markers to the Chat View transcript#322312matthewcorven wants to merge 52 commits into
matthewcorven wants to merge 52 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a typed “overview ruler”-style scrollbar marker overlay to the Chat View transcript so users can visually locate and click-jump to key moments in a conversation (prompts, ask-questions, file changes, compactions, errors), backed by a small read-only geometry API threaded through the base list/tree stack.
Changes:
- Extend list/tree widget APIs to expose overview-ruler layout info and per-element geometry (top/height) for anchoring overlays without DOM querying.
- Introduce a chat-local marker taxonomy + descriptor pipeline and a dedicated controller to render/update markers, resolve overlaps, and handle pointer interaction.
- Add configuration, styling, and unit tests for marker production and controller behavior; scope the feature to the main Chat View transcript (excluding Quick/Inline Chat).
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vs/base/browser/ui/list/listView.ts | Exposes overview-ruler layout info from the list view. |
| src/vs/base/browser/ui/list/listWidget.ts | Threads overview-ruler layout info and element-height access through List. |
| src/vs/base/browser/ui/tree/abstractTree.ts | Threads overview-ruler layout info and element top/height through AbstractTree. |
| src/vs/workbench/contrib/chat/browser/actions/chatPromptNavigationActions.ts | Adds marker taxonomy enums + descriptor computation (lanes, priorities, clustering). |
| src/vs/workbench/contrib/chat/browser/widget/chatScrollbarPromptMarkerController.ts | New controller for marker rendering, overlap handling, and click navigation. |
| src/vs/workbench/contrib/chat/browser/widget/chatListWidget.ts | Wires controller into chat transcript list and refresh triggers; exposes geometry helpers. |
| src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts | Scopes marker enablement to Chat View (not Quick Chat / Inline Chat). |
| src/vs/workbench/contrib/chat/browser/widget/media/chat.css | Adds marker overlay and per-type/per-lane styling; includes some related CSS adjustments. |
| src/vs/workbench/contrib/chat/browser/chat.shared.contribution.ts | Registers chat.scrollbarPromptMarkers.clickBehavior setting. |
| src/vs/workbench/contrib/chat/common/constants.ts | Adds ChatConfiguration key and click-behavior enum. |
| src/vs/workbench/contrib/chat/common/model/chatViewModel.ts | Exposes editedFileEvents on IChatRequestViewModel. |
| src/vs/workbench/contrib/chat/test/browser/actions/chatPromptNavigationActions.test.ts | Unit tests for descriptor production, lane/priority mapping, and click behavior helper. |
| src/vs/workbench/contrib/chat/test/browser/widget/chatScrollbarPromptMarkerController.test.ts | Unit tests for controller layout, geometry, interaction, and lifecycle behavior. |
| .github/prompts/chat-scrollbar-markers-test-hardening.prompt.md | Agent prompt/documentation for test hardening work. |
| .github/prompts/chat-scrollbar-markers-stability-validation.prompt.md | Agent prompt/documentation for stability/perf validation. |
| .github/prompts/chat-scrollbar-markers-scoping.prompt.md | Agent prompt/documentation for feature scoping and implementation plan. |
| .github/prompts/chat-scrollbar-markers-implementation.prompt.md | Agent prompt/documentation for implementation plan and verification checklist. |
Author
|
@microsoft-github-policy-service agree |
…r to editor scrollbar styling, and stabilize clicks on pre-scrolled-virtualized areas
…tion plan - §0: explicitly build both Electron apps (npm run electron + transpile-client) and verify .build/electron paths exist before §1/§2 reference them - §0: add npm install for main worktree (required before transpile-client) - §1: document where --heap-snapshots artifacts land and that §5 consumes them - §1: add independence note clarifying perf:chat and perf:chat-leak don't share state or need port coordination - §3: clarify it launches its own instance and doesn't need the mock server; note its snapshots are consumed by §5 - §4: add mock LLM server setup step before scenarios that need chat responses (C, D, E, I, J, K); specify custom scenario registration for I and J - §4: add heap snapshot capture instructions for §5 consumption - §4 Scenario L: add explicit dependency note that it runs concurrently with Scenario D on the same instance - §5: add prerequisites block enumerating snapshot sources from §1, §3, §4 and what to do if sources are missing - §6: add prerequisites block requiring a fresh Code OSS instance with CDP and mock server (do not rely on §4's instance which may be torn down) - Add execution order summary at top of Methodology listing all section dependencies and produced artifacts
- §1: specify exact output locations for ci-summary.md, run-summary JSON, and heap snapshots (all under .chat-simulation-data/) - §1: document that --ci enables --cleanup-diagnostics which deletes heap snapshots for non-regressed scenarios; add second-pass command without --ci to obtain snapshots for §5 analysis - §2: specify that perf:chat-leak writes to stdout + JSON file (.chat-simulation-data/chat-simulation-leak-results.json); add tee/cp commands to capture both; note JSON overwrite between branch/main runs - §3: fix misconception that chat-session-switch-smoke.mts uses --build flag; it launches via scripts/code.sh from current worktree sources - §4: document mock server standalone startup command and [scenario:<id>] tag-based scenario selection mechanism - §4: document that registerScenario()/ScenarioBuilder cannot produce error responses or errorDetails; provide alternative approaches for Scenario I (kill server mid-stream, dead port) with skip-and-document fallback - §4: document that /compact is extension-handled, not mock-server-handled; provide fallback if compaction marker doesn't appear - §4: add failure handling guidance (max 2 retries, then mark BLOCKED) - §6: correct claim that chat-memory-smoke.mts handles mock server setup; it does not — mock server must be started separately - Scenario L: instruct agent to verify debug log path from source via grep rather than hardcoding /Users/core/out.txt - Deliverables: add missing report sections (session switch from §3, open/close cycle from §6, blocked scenarios list); add blocked status to summary table - Add Cleanup section: remove /tmp/vscode-main-baseline worktree - Add Launch failure handling section: missing electron, port conflicts, CDP timeout, missing sqlite3 - Update execution order summary with §1 two-pass note, §2 output location, and §9 cleanup step - Update constraints recap with worktree cleanup reminder - §0: add reminder to remove baseline worktree (cross-ref Cleanup section)
…arness issue workarounds
…), add close-and-dispose leak check to §6
…s — only editor close disposes ChatWidget, not panel hide
…dispose focus-retry, document a11y
matthewcorven
force-pushed
the
agents/chat-scrollbar-markers-impl
branch
from
d860cb5 to
c73bd37
Compare
Author
Contributor
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: @benibenjMatched files:
|
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Gate marker activation on the primary mouse button so right/middle click no longer navigates or suppresses the scrollbar. Add a pointercancel listener that resets gesture state when the OS interrupts the pointer sequence, preventing a later unrelated pointerup from arming click suppression. Centralize gesture-state cleanup in resetGestureState and call it from setVisible/setEnabled so markerActivated cannot survive a mid-gesture hide or disable. Addresses review findings 1-3 from the edge-case audit.
Add tests for the three edge-case findings: - Non-primary mouse button (right click) does not activate markers or call preventDefault/stopPropagation - pointercancel resets gesture state so a later unrelated pointerup does not arm click suppression - setVisible(false) and setEnabled(false) mid-gesture reset markerActivated so it cannot survive into the next interaction
Keep both our scrollbar prompt marker CSS rules and upstream's .monaco-reduce-motion checkpoint restore rule — they are independent additions that landed at the same location.
- Remove unused onDidChangeFocus public event from ChatListWidget (no consumers) - Wire ChatScrollbarPromptMarkerController.revealItem to the shared applyScrollbarPromptMarkerClickBehavior helper for a single source of truth on click behavior, preserving the deferred focus-retry logic - Add explicit return type annotation to getOverviewRulerLayoutInfo to match the IChatScrollbarPromptMarkerHost interface contract - Add a comment explaining why refresh() (not refreshIfDimensionsChanged()) is required on item-height changes
Closed
…alse) Addresses Copilot review comment: setEnabled(false) previously left the overlay container in the DOM and the capture listeners on the overview- ruler parent attached, so every pointerdown/click/pointerup still went through the controller (early-returning) and the overlay remained until dispose. Now setEnabled(false) calls a new detachOverlay() helper that removes the container from the DOM and clears all parent listeners, making the feature a true no-op when disabled. layout() re-attaches both on re-enable. Also fixes a regression from the earlier revealItem refactor: the applyScrollbarPromptMarkerClickBehavior helper calls focusItem synchronously for RevealAndFocus, but the controller must defer focusItem across animation frames. Now the helper is only used for the Reveal behavior; RevealAndFocus calls reveal directly then defers focusItem via the retry loop. Adds a test verifying setEnabled(false) detaches the container and disposes all parent listeners.
Comment on lines
+544
to
+546
| // Prefer right-lane (prompt) > left-lane > full-lane | ||
| const lanePriority: Record<string, number> = { right: 0, left: 1, full: 2 }; | ||
| candidates.sort((a, b) => (lanePriority[a.lane] ?? 3) - (lanePriority[b.lane] ?? 3)); |
Author
|
Closing this out, will return with a more simplified approach taking inspiration from GitHub Copilot Desktop |
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.
Summary
Adds a typed scrollbar marker overlay to the Chat View transcript, letting users see and jump to significant conversation moments (prompts, questions, file changes, compactions, errors) directly from the vertical scrollbar — analogous to Monaco's overview ruler.
screenshot:
prompt,askQuestion,fileChange,compaction,error) mapped to three lanes (right, left, full), produced from chat view-model signals including a neweditedFileEventsfield onIChatRequestViewModel.getOverviewRulerLayoutInfo,getElementTop,getElementHeight) threaded throughListView→List→AbstractTreeso the chat widget can anchor the overlay without DOM-query hacks.ChatScrollbarPromptMarkerController) that computes pixel-accurate geometry, resolves overlaps deterministically (error > compaction/fileChange > askQuestion > prompt), reuses DOM nodes for smooth transitions, and uses full-width hit-testing so narrow lane markers stay clickable.chat.scrollbarPromptMarkers.enabledsetting (defaultfalse, taggedexperimental) and a companionchat.scrollbarPromptMarkers.clickBehaviorsetting (revealAndFocusdefault,revealalternate). Both settings react live — togglingenabledwhile a chat is open immediately shows/hides the marker overlay without requiring a reload.--vscode-charts-*theme tokens (purple=prompt, yellow=askQuestion/compaction, green=fileChange, red=error), plus focused unit tests covering descriptor production, lane assignment, overlap/click resolution, controller lifecycle, andsetEnabledenable/disable behavior.Lane mapping
The final lane mapping is right, left, and full:
promptaskQuestionfileChangecompactionerrorCenter-lane taxonomy mapping was intentionally excluded — left/right/full mirrors Monaco's overview ruler lane layout and keeps the interaction model simple.
Files changed
src/vs/base/browser/ui/list/listView.ts— exposegetOverviewRulerLayoutInfo()onIListView.src/vs/base/browser/ui/list/listWidget.ts— pass-throughgetOverviewRulerLayoutInfo()andgetElementHeight().src/vs/base/browser/ui/tree/abstractTree.ts— pass-throughgetOverviewRulerLayoutInfo(),getElementTop(),getElementHeight().src/vs/workbench/contrib/chat/browser/actions/chatPromptNavigationActions.ts— typed marker taxonomy, descriptor producers, lane/priority mapping.src/vs/workbench/contrib/chat/browser/widget/chatScrollbarPromptMarkerController.ts— new marker overlay controller withsetEnabledfor live enable/disable.src/vs/workbench/contrib/chat/browser/widget/chatListWidget.ts— wire controller, expose geometry helpers,setScrollbarPromptMarkersEnabledpassthrough.src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts— host scoping flag, gated onchat.scrollbarPromptMarkers.enabled, reacts to live setting changes.src/vs/workbench/contrib/chat/browser/widget/media/chat.css— marker container, lane, and per-type styling.src/vs/workbench/contrib/chat/browser/chat.shared.contribution.ts— registerchat.scrollbarPromptMarkers.enabledandchat.scrollbarPromptMarkers.clickBehaviorsettings.src/vs/workbench/contrib/chat/common/constants.ts—ChatConfigurationkeys +ChatScrollbarPromptMarkerClickBehaviorenum.src/vs/workbench/contrib/chat/common/model/chatViewModel.ts— exposeeditedFileEventsonIChatRequestViewModel.src/vs/workbench/contrib/chat/test/browser/actions/chatPromptNavigationActions.test.ts— descriptor production tests.src/vs/workbench/contrib/chat/test/browser/widget/chatScrollbarPromptMarkerController.test.ts— controller lifecycle/geometry/interaction tests +setEnabledenable/disable tests.Verification
npm run typecheck-clientpasses.npm run valid-layers-checkpasses (list/tree API surface extended).Repro steps for new feature set:
"chat.scrollbarPromptMarkers.enabled": truein settings.chat.scrollbarPromptMarkers.clickBehaviorsetting.chat.scrollbarPromptMarkers.enabledisfalse(default).enabled: true).chat.scrollbarPromptMarkers.enabledtofalsein Settings — markers disappear immediately.true— markers reappear immediately without scrolling or reloading.