Skip to content

Source worktree pickers from dashboard model - #208

Merged
TraderSamwise merged 1 commit into
masterfrom
chore/tui-next-40
Jun 20, 2026
Merged

Source worktree pickers from dashboard model#208
TraderSamwise merged 1 commit into
masterfrom
chore/tui-next-40

Conversation

@TraderSamwise

@TraderSamwise TraderSamwise commented Jun 20, 2026

Copy link
Copy Markdown
Owner

Summary

  • render the dashboard worktree list overlay from service-backed dashboard worktree groups
  • build dashboard migrate picker choices from the same dashboard model instead of local worktree enumeration
  • add focused tests that fail if dashboard mode falls back to local worktree reads

Verification

  • yarn vitest src/tui/screens/overlay-renderers.test.ts src/multiplexer/navigation.test.ts
  • yarn typecheck
  • yarn lint
  • yarn build
  • yarn vitest
  • pre-push hook: yarn typecheck && yarn lint && yarn test

Summary by CodeRabbit

Release Notes

  • Improvements

    • Optimized worktree selection and migration operations to use dashboard-cached data when available, reducing redundant data fetching in dashboard mode.
  • Tests

    • Added comprehensive test coverage for worktree picker and list overlay functionality in dashboard context.

@vercel

vercel Bot commented Jun 20, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
app Ready Ready Preview, Comment Jun 20, 2026 8:23pm

@TraderSamwise

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

No new commits to review since the last review.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3cf31aa6-aa2b-493e-88fb-82315ac4773a

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
📝 Walkthrough

Walkthrough

Two functions — showMigratePicker in navigation.ts and buildWorktreeListOverlayOutput in overlay-renderers.ts — are updated to prefer dashboardWorktreeGroupsCache when in dashboard mode instead of unconditionally calling live worktree service functions. Both changes include new Vitest tests that assert cache-driven behavior and confirm live fallbacks are not invoked.

Changes

Dashboard cache preferred over live worktree lookups

Layer / File(s) Summary
showMigratePicker cache-first population
src/multiplexer/navigation.ts, src/multiplexer/navigation.test.ts
showMigratePicker conditionally builds migratePickerWorktrees from dashboardWorktreeGroupsCache (normalizing (main) entries, mapping to { name, path }, and deduplicating by path) when in dashboard mode; otherwise falls back to listWorktrees/findMainRepo. Tests verify the cache path produces correct picker entries and assert that listWorktrees and findMainRepo are not called.
buildWorktreeListOverlayOutput cache-first population
src/tui/screens/overlay-renderers.ts, src/tui/screens/overlay-renderers.test.ts
buildWorktreeListOverlayOutput derives its worktree list from dashboardWorktreeGroupsCache in dashboard mode; the element type now allows path to be optional, and (main) marking changes from a first-index rule to a path-aware condition. Tests verify cache-driven rendering and that listAllWorktrees is not invoked.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • TraderSamwise/aimux#141: Refactors the worktree-list overlay UI in src/tui/screens/overlay-renderers.ts using the shared renderOverlayBox/modal-band system, directly touching the same buildWorktreeListOverlayOutput function modified in this PR.

Poem

🐇 Hop, hop, no more live calls to make,
The cache holds the worktrees — just go and take!
(main) by path, not by index alone,
De-duped and normalized, cleanly shown.
The tests stand guard so no service is stirred,
A rabbit's best trick: skip what's not preferred! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main change: sourcing worktree pickers from the dashboard model instead of local worktree enumeration.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/tui-next-40

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

@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/multiplexer/navigation.test.ts`:
- Around line 3-13: The test is mocking and asserting the wrong dependency for
the fallback behavior. The code now uses listAllWorktrees() as the fallback in
dashboard mode, but the test still mocks listWorktrees. Replace the
listWorktrees mock with a listAllWorktrees mock that throws an error with
message indicating that the worktree function should not run, ensuring the test
properly verifies that the fallback uses listAllWorktrees() and maintains the
regression guard against calling local worktree functions in dashboard mode.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1dc4a7a5-e1e6-4c8e-b7d5-11cbe35da5d7

📥 Commits

Reviewing files that changed from the base of the PR and between e14f70e and f756697.

📒 Files selected for processing (4)
  • src/multiplexer/navigation.test.ts
  • src/multiplexer/navigation.ts
  • src/tui/screens/overlay-renderers.test.ts
  • src/tui/screens/overlay-renderers.ts

Comment thread src/multiplexer/navigation.test.ts
@TraderSamwise
TraderSamwise merged commit ef5e449 into master Jun 20, 2026
3 checks passed
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