Skip to content

Virtualize branch picker list and forward ComboboxList ref - #150

Merged
juliusmarminge merged 2 commits into
mainfrom
feature/web/virtualize-branch-combobox
Mar 3, 2026
Merged

Virtualize branch picker list and forward ComboboxList ref#150
juliusmarminge merged 2 commits into
mainfrom
feature/web/virtualize-branch-combobox

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Mar 3, 2026

Copy link
Copy Markdown
Member
  • Add virtualized rendering and filtered item handling in BranchToolbar
  • Memoize branch-derived data and sync highlight scrolling for large branch sets
  • Convert ComboboxList to forwardRef to support virtualizer scroll targeting

Note

Medium Risk
Moderate UI behavior change in a frequently used branch-selection control; virtualization/filtering and highlighted-index scrolling could introduce selection or keyboard-navigation edge cases, but no auth/data-path changes.

Overview
Improves performance of the BranchToolbar branch picker for large repos by virtualizing the dropdown list via TanStack useVirtualizer and rendering rows with absolute positioning.

Adds case-insensitive filtering via a new filteredItems list passed to Combobox, syncs highlighted-item scrolling (onItemHighlighted), and memoizes branch-derived collections (branches, branchNames, branchByName, picker items) to reduce recomputation. Also removes the unused thread error wiring.

Written by Cursor Bugbot for commit 6105e44. This will update automatically on new commits. Configure here.

Note

Virtualize the BranchToolbar combobox list and forward the ComboboxList ref to support 28px row height with overscan 12 in BranchToolbar.tsx

Add list virtualization and ref forwarding to the branch picker, pre-filter items case-insensitively, and auto-scroll the highlighted index in BranchToolbar.tsx.

📍Where to Start

Start with the combobox rendering and virtualization setup in BranchToolbar within BranchToolbar.tsx.

Macroscope summarized 6105e44.

- Add virtualized rendering and filtered item handling in `BranchToolbar`
- Memoize branch-derived data and sync highlight scrolling for large branch sets
- Convert `ComboboxList` to `forwardRef` to support virtualizer scroll targeting
@coderabbitai

coderabbitai Bot commented Mar 3, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/web/virtualize-branch-combobox

Comment @coderabbitai help to get the list of available commands and usage tips.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Virtualizer scroll element relies on fragile DOM traversal
    • Forwarded the ref from ComboboxList to ScrollAreaPrimitive.Viewport via a new viewportRef prop on ScrollArea, so branchListRef.current directly references the scroll container without relying on .parentElement DOM traversal.

Create PR

Or push these changes by commenting:

@cursor push 36603514fc
Preview (36603514fc)
diff --git a/apps/web/src/components/BranchToolbar.tsx b/apps/web/src/components/BranchToolbar.tsx
--- a/apps/web/src/components/BranchToolbar.tsx
+++ b/apps/web/src/components/BranchToolbar.tsx
@@ -124,7 +124,7 @@
   const branchListVirtualizer = useVirtualizer({
     count: filteredBranchPickerItems.length,
     estimateSize: () => 28,
-    getScrollElement: () => branchListRef.current?.parentElement ?? null,
+    getScrollElement: () => branchListRef.current ?? null,
     overscan: 12,
     enabled: isBranchMenuOpen,
   });

diff --git a/apps/web/src/components/ui/combobox.tsx b/apps/web/src/components/ui/combobox.tsx
--- a/apps/web/src/components/ui/combobox.tsx
+++ b/apps/web/src/components/ui/combobox.tsx
@@ -260,9 +260,13 @@
   return <ComboboxPrimitive.Value data-slot="combobox-value" {...props} />;
 }
 
-function ComboboxList({ className, ...props }: ComboboxPrimitive.List.Props) {
+function ComboboxList({
+  className,
+  ref,
+  ...props
+}: ComboboxPrimitive.List.Props & { ref?: React.Ref<HTMLDivElement> }) {
   return (
-    <ScrollArea scrollbarGutter scrollFade>
+    <ScrollArea scrollbarGutter scrollFade viewportRef={ref}>
       <ComboboxPrimitive.List
         className={cn(
           "not-empty:scroll-py-1 not-empty:px-1 not-empty:py-1 in-data-has-overflow-y:pe-3",

diff --git a/apps/web/src/components/ui/scroll-area.tsx b/apps/web/src/components/ui/scroll-area.tsx
--- a/apps/web/src/components/ui/scroll-area.tsx
+++ b/apps/web/src/components/ui/scroll-area.tsx
@@ -1,6 +1,7 @@
 "use client";
 
 import { ScrollArea as ScrollAreaPrimitive } from "@base-ui/react/scroll-area";
+import * as React from "react";
 
 import { cn } from "~/lib/utils";
 
@@ -9,14 +10,17 @@
   children,
   scrollFade = false,
   scrollbarGutter = false,
+  viewportRef,
   ...props
 }: ScrollAreaPrimitive.Root.Props & {
   scrollFade?: boolean;
   scrollbarGutter?: boolean;
+  viewportRef?: React.Ref<HTMLDivElement> | undefined;
 }) {
   return (
     <ScrollAreaPrimitive.Root className={cn("size-full min-h-0", className)} {...props}>
       <ScrollAreaPrimitive.Viewport
+        ref={viewportRef}
         className={cn(
           "h-full overscroll-contain rounded-[inherit] outline-none transition-shadows focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-1 focus-visible:ring-offset-background data-has-overflow-x:overscroll-x-contain",
           scrollFade &&

getScrollElement: () => branchListRef.current?.parentElement ?? null,
overscan: 12,
enabled: isBranchMenuOpen,
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Virtualizer scroll element relies on fragile DOM traversal

Medium Severity

getScrollElement returns branchListRef.current?.parentElement rather than branchListRef.current itself. Because ComboboxList in combobox.tsx doesn't explicitly forward the ref to the ScrollAreaPrimitive.Viewport, the ref ends up on ComboboxPrimitive.List (the inner list element) via React 19's props-spreading behavior. The code then uses .parentElement to reach the actual scroll container. This only works because base-ui currently places {children} as a direct child of ScrollAreaPrimitive.Viewport with no intermediate wrapper. If base-ui adds a content-wrapper div inside the Viewport (a common scroll-area pattern), .parentElement will return the wrong element, silently breaking all virtualized scrolling.

Fix in Cursor Fix in Web

@juliusmarminge
juliusmarminge merged commit 2f6cac0 into main Mar 3, 2026
5 checks passed
@juliusmarminge
juliusmarminge deleted the feature/web/virtualize-branch-combobox branch March 3, 2026 04:31
aorwall added a commit to aorwall/t3code that referenced this pull request Sep 12, 2026
Merges `upstream/main` at `e81606494` into the fork, from merge base
`02297e3db` — 47 upstream commits.

The theme of this range is scopable settings: upstream made every server
setting addressable at a scope (global / environment / project) with
per-project overrides, which is why 11 of the 15 conflicts are settings
files. The rest is conversation rewind, floating device streams, and a
large batch of message-sync and markdown-streaming perf work.

## Merge stats

- Landed (`HEAD^1..HEAD`): 277 files, 17243+/4783−
- Upstream range (base..`HEAD^2`): 275 files, 17011+/4749−
- Fork delta (`HEAD^2..HEAD`): 756 files, 76559+/2096−

The two file lists reconcile: the 3 extra landed files are
`docs/fork/inventory.json`, `docs/fork/upstream-merge-log.md` and
`docs/fork/gaps.md`; the 1 file in the range that did not land is
`apps/web/src/routes/settings.integrations.tsx`, resolved `ours` per the
`moatless-admin-integrations-route` inventory entry (that route is a
Moatless admin page here, and upstream's embedded-surface settings live
at `/settings/browser`).

All 15 conflicts were resolved by the verdict `preflight.mjs` printed.
No `decide` conflict was left unresolved. Details, including the
owned-concern sweep (no keyword hits) and the unsupported-method
reconciliation (0 ADD, 0 DROP, 2 KEEP, 4 known exceptions), are in the
dated entry in `docs/fork/upstream-merge-log.md`.

Two findings worth naming here:

- **A silent auto-merge failure.** pingdotgg#11285 changed the mini-player target
from a tab id to a source union. Git updated upstream's own assertion in
`PreviewView.test.tsx` and left the fork-only "under the frame
capability" case next to it still asserting the old string. No conflict
marker, no `resolution-check.mjs` finding — only the fork's own test
suite caught it.
- **Stale inventory anchors.** Upstream moved the project Actions
section out of `ProjectSettingsPanel.tsx` into a new
`ProjectActionsSettings.tsx`, which is where `scriptsEditable` is now
derived and where upstream's new writing Reset button is gated. Four
inventory entries were re-pointed in this merge rather than silently
dropping their deltas.

## Usable as-is

Client work the fork can expose with no Moatless backend change:

- Scoped settings UI and the two-select scope picker (pingdotgg#10639, pingdotgg#10636) —
`SettingsScopeContext`, `ScopedSwitch`, `settingKeys`, the `mixed`
state. The reading half works against Moatless today.
- Float device streams over chat, as a source union rather than a tab id
(pingdotgg#11285); recording status on floating previews (pingdotgg#11312); floating
preview using composer margins (pingdotgg#11290).
- PR-page selections into new drafts (pingdotgg#11296);
projects-on-another-machine badge (pingdotgg#11323); Usage opening on Limits
(pingdotgg#11261).
- macOS permission onboarding (pingdotgg#11289); hold-to-quit fix (pingdotgg#11016);
preview keystrokes kept out of the composer (pingdotgg#11354).
- Message-sync and markdown-streaming perf: pingdotgg#11302, pingdotgg#11029, pingdotgg#11211,
pingdotgg#11198, pingdotgg#11196, pingdotgg#11193, pingdotgg#11181, pingdotgg#11206.
- Assorted web/mobile fixes: pingdotgg#11361, pingdotgg#10757, pingdotgg#11357, pingdotgg#10571, pingdotgg#11348,
pingdotgg#11349, pingdotgg#11281, pingdotgg#11188, pingdotgg#11283, pingdotgg#11292, pingdotgg#11187, pingdotgg#11228, pingdotgg#11103, pingdotgg#10612,
pingdotgg#11032, pingdotgg#11233, pingdotgg#11234, pingdotgg#11304, pingdotgg#11240.

## Unsupported in Moatless / needs implementation

- **Conversation rewind** — `thread.conversation.revert` (pingdotgg#11358). A new
member of `DispatchableClientOrchestrationCommand` in
`packages/contracts/src/orchestration.ts`, bringing the fork to 30
command types (28 upstream's, 2 fork-only). Moatless does not dispatch
it, and a client command cannot be refused per-type, so "Edit from here"
on `RevertUserMessageButton` is reachable whenever the turn is idle and
does nothing. Needs backend dispatch.
- **Per-project setting overrides** — the `projectSettingsOverrides`
capability and the 17-key `ProjectSettingsOverrides` record (pingdotgg#11176).
Two pieces are needed: the capability reported by
`/.well-known/t3/environment`, and `server.updateSettings` served at
project scope. Until both land, the capability filter in
`scopedSettings.ts:170` and `ProjectActionsSettings.tsx:72` drops the
write on the client — the control renders, the user toggles it, and
**the write never leaves the browser**. A silent no-op is worse than a
hidden control or an honest refusal; recorded in `docs/fork/gaps.md`.
- **Default thread permissions** — `defaultRuntimeMode` (pingdotgg#11346). Reads
fine, cannot be saved. Same `server.updateSettings` write path as above,
one level deeper, not a separate gap.

## Backend behavior to consider reproducing in Moatless

Upstream server-side work the fork cannot use directly, but that
Moatless would benefit from:

- **Queue messages during context compaction** (pingdotgg#11107,
`ProviderCommandReactor.ts`) — a message sent while compaction is in
flight is currently dropped rather than held.
- **Restore provider history and prompts when rewinding** (pingdotgg#11338,
`CheckpointReactor.ts`) — the counterpart to
`thread.conversation.revert` above; rewinding the thread without
rewinding provider state leaves the two out of sync.
- **Detect file renames in review diffs** (pingdotgg#8086,
`apps/server/src/vcs/GitVcsDriverCore.ts`) — a rename currently reads as
a whole-file delete plus a whole-file add.
- **Preserve qualified Codex model ids** (pingdotgg#9921, `ModelManifest.ts` +
`CodexTextGeneration.ts`).
- **Model defaults** astra-medium / fable-5.1-medium (pingdotgg#11347).

All five are recorded under the runtime-fixes entry in
`docs/fork/gaps.md`.

## Verification

`verify.mjs` (full pass): 7 of 8 checks green — `duplicate-adds`,
`tripwires`, `resolution-check`, `unsupported-methods`, `fmt:check`,
`lint`, `typecheck`.

`test` is red on **`@t3tools/desktop` only**, at
`scripts/browser-secret-native.test.mjs > bundled libsecret helper`:
`Command failed: pkg-config --cflags --libs libsecret-1`. This is the
standing sandbox gap, not a merge regression — the test file's last
commit is `498ab9c39` (pingdotgg#7261, before the merge base), `git diff
--name-only` against both merge parents is empty for it, and `pkg-config
--exists libsecret-1` fails in this environment. It is already an entry
in `docs/fork/gaps.md`. Every other package passes, including
`@t3tools/web` (5079 tests) after the `PreviewView.test.tsx` fix above.

Three typecheck failures the merge introduced were fixed in it:
`SETTINGS_CATEGORY_SCOPES` in `settingsSearch.ts` was missing all 9
fork-only settings paths, and two `filterAvailableSettingsSearchItems`
literals in `settingsSearch.test.ts` were missing the fork's
`forgejoEnabled` field.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---
Moatless task:
https://moatless.soaplabstest.com/tasks/e70b41b3-779d-43b8-8f34-7de516548e7c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant