fix(vscode): keep large worktree reviews responsive - #13505
Conversation
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (10 files)
Previous Review Summaries (2 snapshots, latest commit bd4789f)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit bd4789f)Status: No Issues Found | Recommendation: Merge Files Reviewed (5 files)
Previous review (commit 7270835)Status: 2 Issues Found | Recommendation: Address before merge Overview
Fix these issues in Kilo Cloud Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (32 files)
Reviewed by grok-4.6 · Input: 277.9K · Output: 15.6K · Cached: 552.3K Review guidance: REVIEW.md from base branch |
|
a few comments I'd like to address but about that const zeroID = (...args) => args.join('\0');
// so that ...
zeroID(prefix, a, b, c)
// ... produces
`${prefix}\0${a}\0${b}\0${c}`as opposite of us reading or typing that manually. Not for this PR though, just some food for thoughts, this kind of ID is cool and useful, still very awkward to read/follow to me when inlined as single interpolated template string. |
What Problem This Solves
Large Agent Manager reviews can finish opening their file list while the actual diff text is still loading. Fast worktree switching also discarded useful in-flight work or evicted a warmed review, causing repeated Git reads and blank review frames.
This reintroduces the intent of #13449 after its revert in #13456, without restoring the batch-wide webview request/cancellation protocol that made rapid switching slower.
Why This Change Was Made
The optimization needs to preserve independent file lifetimes rather than make an entire review one cancellable operation:
The refactor separates local Git/file I/O, batch decoding, and cache/request scheduling. Inline and full-screen reviews share the visibility hook and file-tree row rendering instead of maintaining parallel implementations.
User Impact
Previously loaded diff text stays available when switching worktrees. Cold reviews prioritize the content that can be seen instead of reading every expanded file. Large file selections no longer wait for scrolling and a second visibility check before they can render.
Cold Git access still has a cost. This is not a claim of zero latency or a hard sub-50 ms guarantee for every repository, disk, or machine. The intent is to remove avoidable queueing, repeated work, and blank-content transitions while keeping resource use bounded.
Evidence
Method
Measurements used headed isolated VS Code instances on macOS, the same local CLI build, the actual Kilo repository worktree directories, and a populated disposable checkout with copied Git/Agent Manager metadata. No working files were edited during the profiling scenarios. The large target had 576 changed files, including
packages/sdk/openapi.jsonat approximately 1.3 MB with 1,139 changed lines.The timer starts at the worktree/file selection. Completion checks inspect non-empty
[data-line]elements inside Pierre's shadow DOM and confirm that they intersect the visible review scroller. File headers, loading placeholders, and mounted row wrappers do not count as rendered diff content. A second metric requires two consecutive frames with all currently visible expanded diff content ready.“Cold” means a fresh extension/webview cache, not a flushed OS filesystem cache. “Warm” means the relevant files were previously visited. The clean-main reference was built from
ab03e87ccd. The post-refactor profiles below were collected before subsequent main integrations, including the prompt-navigator and configured-Git fixes; main is now integrated through5ccf4774c3, with the build and Git-selection/cancellation tests rerun after resolving the integration conflict. Timing ranges include local scheduling and background-activity variation; they are not CI thresholds.Repeated cold large-review measurements
Post-refactor phase breakdown:
Individual file and warm-switch checks after refactoring
The Chromium regression fixtures additionally cover 12 retained reviews across 36 switch frames, 120-file viewport-only loading, exact viewport/prefetch-boundary checks, interrupted-request recovery without a new summary, a virtualized 600-file tree, review comments, context resets, and fast upward scrolling.
Follow-up: warm browser/index-root-guard switching
A follow-up profile used the exact
agent-browser-context-viewandfix-indexing-root-guardworktrees. The first had 125 changed files and the second had 21; the counts, selected session tabs, review width, and warm-up sequence matched in both variants. Three profiles per variant recorded 30 switches in total. Both variants included the requested cache-key/slice cleanups; only the annotation invalidation fix was disabled in the before build.The remaining warm-path work was not Git loading. Reading
props.activewhile building annotation arrays made every activation regenerate unchanged annotations, so Pierre rerendered cached hunks and scheduled new layout measurements. The fix stops tracking activation for that composer-write guard while preserving dependencies on actual comments, edits, and drafts.The strengthened fixture includes a saved review annotation and now checks individual diff-line node identity, not just the containing panel. Existing comment editing, interruption, viewport, and scrolling tests remain green. A final geometry check is performed only before a genuinely new detail request, preventing an older intersection observation from requesting a row that moved outside the viewport; cached and duplicate requests do not perform this layout check.
The separate readability comparison ran the actual committed and suggested cache modules in VS Code's embedded Node v24.18.1, with alternating trials and an in-memory text loader to isolate cache overhead:
Those differences are approximately 6 ns per hit and 48 ns per queued file, within the observed trial spread and not a measurable user-facing delay. The extension declares only the additional
ES2024.Promisetype library for the supported runtime API; its emitted JavaScript target remains ES2022. The Storybook microtasks are documented as asynchronous host-response simulation, not production scheduling.The remaining first-frame time includes transcript/UI layout and frame scheduling, so this is still not a guarantee that every complete worktree switch finishes below 50 ms. Additional focus and cached-layout probes were not retained because they did not show a useful improvement.
Validation
bun run compile/bun run build:check: pass.bun run test:unit: 4,287 tests pass after main integration and the two review fixes.git diff --check: pass.Manual verification
Open Changes in a worktree with many changed files, select a large file, select another file and return, rotate among more than four worktrees, and scroll directly to distant files. Verify actual diff text, preserved scroll position, and no blank-content reload when returning to a warmed review.