fix(vscode): optimize model selector search and auto-jump to active match - #12810
Conversation
…atch Remove the 250 ms search debounce so the model list updates on every keystroke. Add a latest-search-only animation-frame scroll that keeps the active match visible when filtering from a deeply scrolled list, and track pointer coordinates so rows moving beneath a stationary pointer no longer steal selection while real hover still transfers immediately.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| setPreviewKey(next) | ||
| if (!open()) return | ||
| if (scrollFrame !== undefined) cancelAnimationFrame(scrollFrame) | ||
| scrollFrame = requestAnimationFrame(() => { |
There was a problem hiding this comment.
SUGGESTION: The auto-scroll is keyed to filtered(), not to a search change
This effect re-runs on every filtered() recompute, and filtered() depends on visibleModels(), which returns a fresh array whenever models() or connected() changes (and whenever a parent passes a new props.models identity). So a background provider/model refresh while the popover is open will now also scroll the list back to the active/first row, throwing away wherever the user had scrolled to — previously that recompute only reset selection, with no visible jump.
Since the intent is "latest search only", consider keeping the previous search text in a closure variable and scheduling the frame only when search() actually changed.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| } | ||
|
|
||
| function pointerMove(e: MouseEvent, key: string) { | ||
| const moved = pointerX === undefined || pointerY === undefined || e.clientX !== pointerX || e.clientY !== pointerY |
There was a problem hiding this comment.
SUGGESTION: The first mousemove a row sees is always treated as real movement
pointerX === undefined makes moved true, so the first mousemove delivered to any row steals selection even when the pointer never moved. That is exactly the case this guard is meant to protect: if the cursor is resting over the list region but no row has received a mousemove yet (popover opened through the openModelPicker shortcut, or the virtualized rows mount under a stationary cursor), the synthetic mousemove the browser dispatches after the auto-jump scroll passes the check and overrides the first search match.
Recording the baseline coordinates without selecting on the very first event, or tracking coordinates from the list container instead of per row, would close that gap. Real hover always produces follow-up moves, so behavior for genuine pointer movement is unchanged.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (2 files)
Notes
Fix these issues in Kilo Cloud Previous Review Summaries (2 snapshots, latest commit ca14e42)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit ca14e42)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (2 files)
Notes
Fix these issues in Kilo Cloud Previous review (commit 6c31e56)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (3 files)
Notes
Reviewed by kimi-k3 · Input: 43K · Output: 5.4K · Cached: 244.7K Review guidance: REVIEW.md from base branch |
| if (!(target instanceof Element)) return | ||
| const item = target.closest<HTMLElement>('[role="treeitem"][data-key]') | ||
| const key = item?.dataset.key | ||
| if (!key) return |
There was a problem hiding this comment.
SUGGESTION: Pointer coordinates are only recorded when the event lands on a treeitem, so a stationary cursor can still be seen as "moved"
Now that the handler is delegated to the whole list, it receives mousemove for regions that are not tree items (the star button, which is a sibling of the treeitem div, and the row/list padding). Those events hit this early return before pointerX/pointerY are updated, so the last recorded position can be from a different part of the list than where the cursor actually rests.
Concrete case: the cursor sits over a row's star button while pointerX/Y still hold the coordinates of the item cell it passed through. Keyboard navigation (setPointer(false) + scrollToIndex) then shifts the list, Chrome replays a synthetic mousemove at the unchanged cursor position, and if a group label (no star button) is now under that x/y, moved evaluates to true because the stale coordinates differ. That flips pointer() back to true and reassigns selectedKey, which is the exact hijack this change is meant to prevent.
Recording the position unconditionally before the key check keeps the stationary-pointer invariant intact regardless of what the cursor is over:
function pointerMove(e: MouseEvent) {
const moved = pointerX !== undefined && pointerY !== undefined && (e.clientX !== pointerX || e.clientY !== pointerY)
pointerX = e.clientX
pointerY = e.clientY
const target = e.target
if (!moved || !(target instanceof Element)) return
const key = target.closest<HTMLElement>('[role="treeitem"][data-key]')?.dataset.key
if (!key) return
setPointer(true)
setSelectedKey(key)
}Reply with @kilocode-bot fix it to have Kilo Code address this issue.
…atch (Kilo-Org#12810) * fix(vscode): optimize model selector search and auto-jump to active match Remove the 250 ms search debounce so the model list updates on every keystroke. Add a latest-search-only animation-frame scroll that keeps the active match visible when filtering from a deeply scrolled list, and track pointer coordinates so rows moving beneath a stationary pointer no longer steal selection while real hover still transfers immediately. * fix(vscode): stabilize model selector search navigation * chore: update kilo-vscode visual regression baselines * chore: update kilo-vscode visual regression baselines --------- Co-authored-by: kilo-maintainer[bot] <kilo-maintainer[bot]@users.noreply.github.com>
2 participants
Model search now updates instantly on every keystroke and keeps the active match visible while typing.
Problem
The model selector had a 250 ms debounce that made every search feel sluggish. After removing the debounce, a second bug surfaced: when the virtualized list was scrolled far from the active match, filtering would mount the selected row above the viewport without scrolling it into view.
Changes
ModelSelector.tsx. Filtering, grouping, favorites, and keyboard selection now update directly from each keystroke.requestAnimationFramescroll that keeps the active match visible when filtering from a deeply scrolled list.mousemoveso rows moving beneath a stationary pointer (from virtualization or programmatic scroll) no longer steal selection, while real pointer movement still immediately transfers selection to the hovered model.onMouseEnterselection path that caused hover to hijack search results when content moved under a still cursor.Performance
Validation