Skip to content

refactor(sidebar): move the pure workspace-list builder into src/shared/sidebar-list (no visible change) - #29906

Merged
Jinwoo-H merged 5 commits into
mainfrom
jinwoo/sidebar-list-shared-builder
Oct 17, 2026
Merged

Jinwoo-H merged 5 commits into
mainfrom
jinwoo/sidebar-list-shared-builder

Conversation

@Jinwoo-H

@Jinwoo-H Jinwoo-H commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

ELI5

The code that decides what the desktop's workspace list shows (which rows, in what order, under which headers, with which counts) moves into a shared folder that has no ties to the desktop window. Nothing on screen changes. Later the phone can run this exact same code, so both lists match by construction instead of by two copies of the rules being kept in sync.

What Changed

Before and after for the user: identical. Same rows, same order, same headers, counts, collapse behaviour, highlights and Cmd+1–9 targets.

Mechanism:

  • Commit 1, test(sidebar): captures golden files of the sidebar's row output from the unmodified code. The test drives the window's real hooks (filter → sort → grouping → pins → lineage → host sections → selection) over a fixture store. It records:

    • every row, with keys, collapse keys, counts, per-host counts and ids, nesting/lineage and occurrence placement;
    • occurrences that are planned but hidden;
    • the preferred copy of duplicated pins;
    • Cmd+1–9 targets, both with the list mounted and with it unmounted.

    It covers every Group by × Sort by × Project order × "show pinned in groups" combination, each expanded and collapsed. It also has host-filter, single-host, repo-filter, hide-sleeping, hide-default-branch and new-external-inbox scenarios, plus "rows missing from the frozen order go last". The fixture includes local, SSH and paired-server rows, a project on two hosts, the same workspace id on two hosts (one copy pinned), nested project groups, local and SSH folder workspaces, GitHub PR states, a GitLab MR workspace, cross-project lineage, a pending creation, an imported-worktrees card, an inbox row and an empty-project placeholder. These files are meant to stay as the correctness reference.

  • Commits 2–5 (the move): the pure list code moves from src/renderer/src/components/sidebar/** to src/shared/sidebar-list/. That covers:

    • the row plan (planSidebarRows);
    • grouping, pinned rows and project-group sections;
    • empty-project placeholders;
    • host sections, host collapse and host order;
    • the visible-row filter (selectVisibleSidebarWorktrees);
    • lineage projection and host scoping of project groups and folders.

    The builder takes only plain data plus two readers the window supplies:

    • facts: is a row sleeping, created elsewhere, and so on. This was already a shared interface.
    • prGroupOf: which PR lane a worktree belongs to. The window builds it from its PR cache, so the cache itself and the PR cache-key code stay in the window.

    It has no clock, store, React, i18n or Electron imports.

    • Header icons: a group header carries an icon key, for example { kind: 'pr-status', prGroup: 'done' } or { kind: 'workspace-status', status }. The window turns the key into the same glyph and colour when it draws the header (rows/header-visuals.ts).
    • Header labels: Pinned, All and the PR-lane headers get their localized name from that same lookup at draw time. Headers named by user data (project, project group, status) keep their label from the builder.
    • Window callers: the three that build rows (the row hook, reveal, and the Cmd+1–9 replay) call planSidebarRows directly. compute-visible-worktrees.ts is a thin adapter over the shared filter. The old positional buildSidebarRowPlan/buildRows remain only as a test fixture, so the existing tests run unchanged.
    • Not moved: the parts that hold state over time (the Smart sort latch, the settled sort epoch, sleeping-sweep retention timers, pending creations), folder path-status routing, and the rendered-row key index.
    • Outside the sidebar: six files change, one import line each.

The golden files from commit 1 are byte-for-byte unchanged by the move. The golden harness changed in three ways only: import paths, renames to the .test-fixture.ts convention, and reading the header's colour, glyph and fallback label through getSidebarHeaderVisual. That is the same function the header uses when it draws, so the goldens also prove the header visuals and labels didn't change.

Size: production code is 87 files, +852/−783, and almost all of that is renames and import paths. Including tests and goldens the total is 155 files, +8,566/−899. The goldens are about 870 KB of text.

Why

The phone currently has its own copy of the list rules (mobile/src/worktree/workspace-list-sections.ts and friends), and the two drift. The agreed direction is one builder that the desktop window and the phone page both run over the same inputs. This PR is only the first, invisible step: it isolates the builder and pins today's output so later steps can't silently change it.

Alternatives considered:

  • Reshaping more at once (moving the timers, moving the tests next to the shared code, migrating every test off the positional helper). Each of those adds risk to a no-visible-change PR, so they are left to later PRs.
  • Passing the PR cache into the builder. Rejected: a reader keeps PR caches and their key format out of the shared code, and the phone can supply lanes from the per-row facts the host will publish.

Linked Issue

N/A (maintainer refactor; part of the phone and desktop list parity work)

Visual Proof

N/A. No visual or interaction change. The goldens prove the rendered rows, header tone/icon and shortcut targets are identical.

Testing

  • Automated tests added/updated, or explained why not below
  • Goldens pass before (commit 1) and after (commits 2–3) with no golden file change. A deliberately edited golden fails the test.
  • Sidebar, shared, workspace-view and store suites: 802 files / 7,424 tests pass (--maxWorkers=2). The wider renderer run had only 3 failures (rich-markdown-escape-fidelity, terminal-shutdown-layout-capture), and they fail the same way on the base commit.
  • pnpm tc and changed-code quality gate pass. oxlint and max-lines pass with no new disables. The focus-read ratchet passes (pruned one entry because the reading file moved to shared). Localization coverage and extraction pass; no copy changed.
  • Timing at ~415 worktrees / 420–458 rows, base vs this branch, run alternately on the same machine.
    • Builder and filter alone: equal or faster in every mode. For example, a full build in repo grouping takes 0.83–1.06 ms on base and 0.74–0.94 ms on this branch. PR-status grouping roughly halves (1.9–2.4 ms to 0.86–1.0 ms) because lane labels are no longer looked up for every row.
    • Through the real hooks: a full rebuild is within run-to-run noise (about ±0.4 ms on a 1.5–3.9 ms rebuild), except PR-status, which is about 1 ms faster. The Cmd+1–9 lookup with the list closed is the same.
  • Not run: the Electron app itself (the goldens drive the window's real hooks instead).

Review

An independent review checked correctness and design. It found the moved files identical apart from import paths, the icon key and label inputs. It confirmed the shared folder has no renderer, React, i18n or clock imports and that no other reader of header tone/icon exists. A second design review asked for the narrower inputs (a PR-lane reader instead of the cache, no settings, no labels in the shared code, and the status icon key carrying only the status id), and those are applied. A follow-up review confirmed each window caller passes exactly the values the old positional defaults gave.

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

  • Cross-platform, SSH and remote servers: the logic is unchanged and the goldens include SSH and paired-server rows.
  • Mobile: nothing on the phone uses these files yet.
  • Compatibility: no wire or format change.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

Golden output of the unmodified window pipeline (filter, sort, grouping,
pins, lineage, host sections, Cmd+1-9 targets) across every groupBy x sortBy
x projectOrderBy x pinned policy, plus host-filter, repo-filter, hide-sleeping
and hide-default-branch scenarios, each expanded and collapsed. Kept as the
correctness reference for moving the row builder.
…list

The sidebar's list rules (visible filter, grouping, pinned rows, project
group sections, placeholders, host sections, collapse keys) move to
src/shared/sidebar-list as pure functions. Headers carry an icon key instead
of a React component; the window resolves glyph and tone when it paints, and
passes localized header labels in. The window keeps calling through thin
adapters (build-rows, compute-visible-worktrees); sort latch, settled epoch,
sleeping retention and pending creations stay where they are.

github-cache-key moves to src/shared/github because PR-status grouping needs
it; the renderer path stays as a re-export so its callers are untouched.
Other edits outside the sidebar are import paths only. The golden fixtures
from the previous commit pass unchanged.
Header labels are read on every re-emit again, as before the move. Settings
and labels types move next to the other row types; header visuals no longer
export what nobody imports. Adds a golden for the new-external-worktrees inbox
row, captured on the pre-move pipeline.
@coderabbitai

coderabbitai Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 0c25433e-8190-49c4-9019-e038d7ad71ba
📥 Commits

Reviewing files that changed from the base of the PR and between 18865f2 and 46f8315.

📒 Files selected for processing: 56
  • src/renderer/src/components/sidebar/WorktreeCard.host-badge-toggle.test.tsx
  • src/renderer/src/components/sidebar/host-section-collapse-folders.test.ts
  • src/renderer/src/components/sidebar/host-section-collapse.test.ts
  • src/renderer/src/components/sidebar/host-section-folder-workspace-counts.test.ts
  • src/renderer/src/components/sidebar/host-section-rows.test.ts
  • src/renderer/src/components/sidebar/pinned-section-worktrees.test.ts
  • src/renderer/src/components/sidebar/project-group-header-drop.test.ts
  • src/renderer/src/components/sidebar/rendered-sidebar-worktree-order.ts
  • src/renderer/src/components/sidebar/sidebar-lineage-ancestors.test.ts
  • src/renderer/src/components/sidebar/sidebar-list-golden-serialize.test-fixture.ts
  • src/renderer/src/components/sidebar/sidebar-list-goldens.test.tsx
  • src/renderer/src/components/sidebar/sidebar-workspace-option-items.test.ts
  • src/renderer/src/components/sidebar/visible-worktrees-manual-lineage-order.test.ts
  • src/renderer/src/components/sidebar/worktree-list-groups-host-collision.test.ts
  • src/renderer/src/components/sidebar/worktree-list-groups-host-labels.test.ts
  • src/renderer/src/components/sidebar/worktree-list-groups-imported-worktrees.test.ts
  • src/renderer/src/components/sidebar/worktree-list-groups-lineage-nesting.test.ts
  • src/renderer/src/components/sidebar/worktree-list-groups-mobile-parity.test.ts
  • src/renderer/src/components/sidebar/worktree-list-groups-nested-project-groups.test.ts
  • src/renderer/src/components/sidebar/worktree-list-groups-notice-host-labels.test.ts
  • src/renderer/src/components/sidebar/worktree-list-groups-pending-creations.test.ts
  • src/renderer/src/components/sidebar/worktree-list-groups-pinned-host-labels.test.ts
  • src/renderer/src/components/sidebar/worktree-list-groups-project-groups.test.ts
  • src/renderer/src/components/sidebar/worktree-list-groups-project-host-setups.test.ts
  • src/renderer/src/components/sidebar/worktree-list-groups-project-order.test.ts
  • src/renderer/src/components/sidebar/worktree-list-groups-section-label-disambiguation.test.ts
  • src/renderer/src/components/sidebar/worktree-list/grouping/build-rows.folder-workspace-lanes.test.ts
  • src/renderer/src/components/sidebar/worktree-list/grouping/build-rows.test-fixture.ts
  • src/renderer/src/components/sidebar/worktree-list/grouping/build-rows.test.ts
  • src/renderer/src/components/sidebar/worktree-list/grouping/cross-project-lineage.test.ts
  • src/renderer/src/components/sidebar/worktree-list/grouping/imported-rows.test.ts
  • src/renderer/src/components/sidebar/worktree-list/grouping/pr-group-key.ts
  • src/renderer/src/components/sidebar/worktree-list/grouping/recent-project-rank-scan-budget.test.ts
  • src/renderer/src/components/sidebar/worktree-list/listing/folder-path-status-routing.ts
  • src/renderer/src/components/sidebar/worktree-list/listing/host-filtering.test.ts
  • src/renderer/src/components/sidebar/worktree-list/listing/rendered-row-keys.ts
  • src/renderer/src/components/sidebar/worktree-list/listing/use-folder-path-statuses.ts
  • src/renderer/src/components/sidebar/worktree-list/listing/use-section-rows.ts
  • src/renderer/src/components/sidebar/worktree-list/navigation/worktree-reveal-group-keys.ts
  • src/renderer/src/components/sidebar/worktree-list/rows/SectionHeader.tsx
  • src/renderer/src/components/sidebar/worktree-list/rows/header-visuals.ts
  • src/renderer/src/components/sidebar/worktree-list/viewport/scroll-adjustment.test.ts
  • src/renderer/src/components/sidebar/worktree-list/viewport/sticky-headers.test.ts
  • src/renderer/src/components/sidebar/worktree-list/viewport/virtual-rows.test.ts
  • src/shared/sidebar-list/folder-workspace-lanes.ts
  • src/shared/sidebar-list/group-keys.ts
  • src/shared/sidebar-list/group-sections.ts
  • src/shared/sidebar-list/host-filtering.ts
  • src/shared/sidebar-list/pinned-group-rows.ts
  • src/shared/sidebar-list/project-group-sections.ts
  • src/shared/sidebar-list/row-types.ts
  • src/shared/sidebar-list/section-order.ts
  • src/shared/sidebar-list/sidebar-row-plan.ts
  • src/shared/sidebar-list/visible-worktrees.ts
  • src/shared/sidebar-list/worktree-group-keys.ts
  • src/shared/sidebar-list/worktree-grouping.ts
💤 Files with no reviewable changes: 3
  • src/shared/sidebar-list/pinned-group-rows.ts
  • src/shared/sidebar-list/host-filtering.ts
  • src/shared/sidebar-list/group-keys.ts
🚧 Files skipped from review as they are similar to previous changes: 2
  • src/shared/sidebar-list/folder-workspace-lanes.ts
  • src/shared/sidebar-list/project-group-sections.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.

📝 Walkthrough

The PR moves sidebar-list contracts and helpers into shared modules. It replaces positional row planning with planSidebarRows, adds shared visible-worktree selection, and separates PR grouping from cache lookup. Renderer imports and header rendering use the shared APIs. New fixtures and golden tests cover grouping, sorting, filtering, lineage, pinned placement, host scope, and external inbox scenarios.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 46f83

No actionable sidebar behavior change was identified; the PR is mergeable after normal checks.

Pre-merge checks: Passed 4 · Inconclusive 1

Failed Failed checks: 1 inconclusive

Check name Status Explanation Resolution
Docstring Coverage Inconclusive Docstring coverage is 29.51% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 61 functions across 88 files. (6 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title clearly identifies the sidebar refactor and move to shared code. It is concise and directly related to the main change.
Description check Passed The description follows the required structure and clearly explains the user impact, implementation, rationale, testing, visual proof, and compatibility considerations. It reports the unrun Electron t…
Full details: Description check

Explanation

The description follows the required structure and clearly explains the user impact, implementation, rationale, testing, visual proof, and compatibility considerations. It reports the unrun Electron test and leaves some checklist items unchecked, but it remains mostly complete.

Full details: Docstring Coverage

Explanation

Docstring coverage is 29.51% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 61 functions across 88 files. (6 skipped: 6 over the file limit.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

- The builder takes a PR-lane reader instead of the PR cache and settings;
  getPRGroupKey stays in the window, so github-cache-key no longer moves.
- Pinned, All and PR-lane headers carry only an icon key; the window names
  and draws them at paint (status headers keep their data label).
- The status icon key carries the status id; the window looks up its look.
- Window callers use planSidebarRows directly; the positional helper is a
  test fixture.
- Folder path-status routing and rendered-row keys stay in the window.

Goldens pass byte-identical.
@Jinwoo-H
Jinwoo-H merged commit 0013011 into main Oct 17, 2026
33 checks passed
LINDECKER-Charles added a commit to LINDECKER-Charles/orca that referenced this pull request Oct 19, 2026
The workspace-list builder moved to src/shared/sidebar-list (stablyai#29906), and
the sidebar now draws one header per project group id when two hosts
publish the same group (stablyai#30128). The branch linked each host's copy to a
parent on its own host, which assumed every copy was drawn; with copies
collapsed, parents now resolve by id over the drawn groups, the same links
reveal walks. Same-id copies from two hosts can still close a parent loop,
so section emission and getProjectGroupOccurrencePaths share one parent
map that cuts it with breakProjectGroupParentCycles.

compareProjectGroupSiblings adopts the device-independent sidebar order
from stablyai#29946 instead of localeCompare.

createOnTarget moves to project-group-create-on-host.ts, and two repo RPC
test expectations fold into one, to keep both files under max-lines.
jbamlee added a commit to jbamlee/orca that referenced this pull request Oct 20, 2026
… is in

A project set up on several hosts is one sidebar section anchored on each Orca's own row, and
placement read only the anchor's projectGroupId. Move to group changes one host's row, so only the
Orca anchored on that row drew the project inside the group; the others showed the group empty.

- getProjectSectionMemberRepoIds maps every catalog row to its section, including rows whose
  workspaces are pinned away or absent.
- getProjectSectionGroupRepo picks the first member in manual order that is in a known group,
  else the anchor. Placement, in-group rank and the reveal path read that row.
- Header rows carry it as projectGroupRepo; header drag buckets, ranks and commits use it, and the
  commit names that row's host.

Ported onto src/shared/sidebar-list after stablyai#29906.

Fixes stablyai#29876

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jbamlee added a commit to jbamlee/orca that referenced this pull request Oct 29, 2026
… is in

A project set up on several hosts is one sidebar section anchored on each Orca's own row, and
placement read only the anchor's projectGroupId. Move to group changes one host's row, so only the
Orca anchored on that row drew the project inside the group; the others showed the group empty.

- getProjectSectionMemberRepoIds maps every catalog row to its section, including rows whose
  workspaces are pinned away or absent.
- getProjectSectionGroupRepo picks the first member in manual order that is in a known group,
  else the anchor. Placement, in-group rank and the reveal path read that row.
- Header rows carry it as projectGroupRepo; header drag buckets, ranks and commits use it, and the
  commit names that row's host.

Ported onto src/shared/sidebar-list after stablyai#29906.

Fixes stablyai#29876

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jbamlee added a commit to jbamlee/orca that referenced this pull request Oct 30, 2026
… is in

A project set up on several hosts is one sidebar section anchored on each Orca's own row, and
placement read only the anchor's projectGroupId. Move to group changes one host's row, so only the
Orca anchored on that row drew the project inside the group; the others showed the group empty.

- getProjectSectionMemberRepoIds maps every catalog row to its section, including rows whose
  workspaces are pinned away or absent.
- getProjectSectionGroupRepo picks the first member in manual order that is in a known group,
  else the anchor. Placement, in-group rank and the reveal path read that row.
- Header rows carry it as projectGroupRepo; header drag buckets, ranks and commits use it, and the
  commit names that row's host.

Ported onto src/shared/sidebar-list after stablyai#29906.

Fixes stablyai#29876

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jbamlee added a commit to jbamlee/orca that referenced this pull request Nov 1, 2026
… is in

A project set up on several hosts is one sidebar section anchored on each Orca's own row, and
placement read only the anchor's projectGroupId. Move to group changes one host's row, so only the
Orca anchored on that row drew the project inside the group; the others showed the group empty.

- getProjectSectionMemberRepoIds maps every catalog row to its section, including rows whose
  workspaces are pinned away or absent.
- getProjectSectionGroupRepo picks the first member in manual order that is in a known group,
  else the anchor. Placement, in-group rank and the reveal path read that row.
- Header rows carry it as projectGroupRepo; header drag buckets, ranks and commits use it, and the
  commit names that row's host.

Ported onto src/shared/sidebar-list after stablyai#29906.

Fixes stablyai#29876

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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