Skip to content

Name the default fork destination in tab menus - #184

Merged
lawrencecchen merged 4 commits into
mainfrom
fix-fork-conversation-default-label
Jul 15, 2026
Merged

lawrencecchen merged 4 commits into
mainfrom
fix-fork-conversation-default-label

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Jul 15, 2026 •

Copy link
Copy Markdown

The primary tab context-menu action now names its configured destination, such as “Fork Conversation to the Right”. The existing destination submenu remains available.

Tests: swift test --filter 'BonsplitTests.testTabContextMenuBuilderCreatesForkConversationSubmenu|BonsplitTests.testTabContextMenuBuilderKeepsForkConversationVisibleWhenOpenAvailabilityIsRefreshing'


View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.


Summary by cubic

Show the configured default fork destination in the tab context menu’s primary action (e.g., “Fork Conversation to the Right”). Replace the fork availability APIs with a hidden/refreshing/available state so the item can stay visible but disabled while loading; submenu unchanged and en/ja labels added.

Written for commit e099cf4. Summary will update on new commits.

Review in cubic

@coderabbitai

coderabbitai Bot commented Jul 15, 2026 •

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@lawrencecchen, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 40 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9769b8ab-dea8-492f-8c87-92d729ad995d

📥 Commits

Reviewing files that changed from the base of the PR and between 893ca62 and e099cf4.

📒 Files selected for processing (5)
  • Sources/Bonsplit/Internal/Views/TabBarView.swift
  • Sources/Bonsplit/Internal/Views/TabItemView.swift
  • Sources/Bonsplit/Public/BonsplitController.swift
  • Sources/Bonsplit/Public/Types/TabContextAction.swift
  • Tests/BonsplitTests/BonsplitTests.swift
📝 Walkthrough

Walkthrough

The fork conversation context-menu item now displays a localized title based on its default destination, with English and Japanese strings added. Tests now select the updated left-destination title.

Changes

Fork Conversation Titles

Layer / File(s) Summary
Destination-specific menu titles
Sources/Bonsplit/Internal/Views/TabItemView.swift, Sources/Bonsplit/Resources/*/Localizable.strings
The menu title is derived from the configured fork action, with localized strings for right, left, top, bottom, new tab, and new workspace destinations.
Updated menu title expectations
Tests/BonsplitTests/BonsplitTests.swift
Fork conversation submenu tests now locate the root item using "Fork Conversation to the Left".

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: austinywang

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: naming the default fork destination in tab menus.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-fork-conversation-default-label

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.

@greptile-apps

greptile-apps Bot commented Jul 15, 2026

Copy link
Copy Markdown

Greptile Summary

This PR updates the primary "Fork Conversation" tab context-menu item to display the configured destination in its title (e.g. "Fork Conversation to the Right") rather than the generic "Fork Conversation". Both English and Japanese localizations are included, and the two existing tests are updated to assert the new title strings.

  • The new forkConversationDefaultTitle(for:) helper maps each TabContextAction fork case to a localized string, with .forkConversation grouped with .forkConversationRight as a fallback.
  • The submenu listing all destinations is preserved alongside the renamed primary action.
  • Tests cover only .forkConversationLeft as the default; the remaining five destinations have no title-string coverage.

Confidence Score: 4/5

Safe to merge; the change is purely presentational and the logic is straightforward.

The new helper correctly routes all six fork destinations to distinct localized strings, and both localization files are in sync. The only notable concern is the default branch in the switch — because TabContextAction is a public enum, a future fork-destination case would silently fall back to "Fork Conversation to the Right" without a compile-time warning.

Sources/Bonsplit/Internal/Views/TabItemView.swift — specifically the default branch in forkConversationDefaultTitle.

Important Files Changed

Filename Overview
Sources/Bonsplit/Internal/Views/TabItemView.swift Adds forkConversationDefaultTitle(for:) to produce a destination-specific title for the primary fork menu item; one minor concern with the non-exhaustive default branch.
Sources/Bonsplit/Resources/en.lproj/Localizable.strings Adds six new localization keys for the fork destination titles in English; consistent with the code.
Sources/Bonsplit/Resources/ja.lproj/Localizable.strings Adds six new localization keys for the fork destination titles in Japanese; translations look correct.
Tests/BonsplitTests/BonsplitTests.swift Updates two tests to assert the new destination-specific title string; both tests use .forkConversationLeft as the default, leaving the other five destinations (right, top, bottom, newTab, newWorkspace) without title coverage.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[TabContextMenuBuilder.makeMenu] --> B{state.canForkConversation?}
    B -- Yes --> C[forkConversationDefaultTitle\nfor: state.forkConversationDefaultAction]
    C --> D{action}
    D -- forkConversationLeft --> E["Fork Conversation to the Left"]
    D -- forkConversationRight / forkConversation --> F["Fork Conversation to the Right"]
    D -- forkConversationTop --> G["Fork Conversation to the Top"]
    D -- forkConversationBottom --> H["Fork Conversation to the Bottom"]
    D -- forkConversationNewTab --> I["Fork Conversation to New Tab"]
    D -- forkConversationNewWorkspace --> J["Fork Conversation to New Workspace"]
    D -- default --> F
    E & F & G & H & I & J --> K[addAction title=… action=.forkConversation]
    K --> L[Primary menu item fires .forkConversation]
    B -- Yes --> M[forkConversationSubmenuItem\nshows all destinations with checkmark]
    B -- No --> N[Skip fork section]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[TabContextMenuBuilder.makeMenu] --> B{state.canForkConversation?}
    B -- Yes --> C[forkConversationDefaultTitle\nfor: state.forkConversationDefaultAction]
    C --> D{action}
    D -- forkConversationLeft --> E["Fork Conversation to the Left"]
    D -- forkConversationRight / forkConversation --> F["Fork Conversation to the Right"]
    D -- forkConversationTop --> G["Fork Conversation to the Top"]
    D -- forkConversationBottom --> H["Fork Conversation to the Bottom"]
    D -- forkConversationNewTab --> I["Fork Conversation to New Tab"]
    D -- forkConversationNewWorkspace --> J["Fork Conversation to New Workspace"]
    D -- default --> F
    E & F & G & H & I & J --> K[addAction title=… action=.forkConversation]
    K --> L[Primary menu item fires .forkConversation]
    B -- Yes --> M[forkConversationSubmenuItem\nshows all destinations with checkmark]
    B -- No --> N[Skip fork section]
Loading

Reviews (1): Last reviewed commit: "Name default fork destination in tab men..." | Re-trigger Greptile

Comment on lines +1729 to +1740
case .forkConversationRight,
.forkConversation:
return localized(
"tabContext.forkConversation.default.right",
defaultValue: "Fork Conversation to the Right"
)
default:
return localized(
"tabContext.forkConversation.default.right",
defaultValue: "Fork Conversation to the Right"
)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Non-exhaustive switch bypasses compiler safety

The default branch makes the switch non-exhaustive. TabContextAction is a public CaseIterable enum, so adding a new fork-destination case in the future (e.g., .forkConversationCurrentPane) would silently fall through to "Fork Conversation to the Right" here without any compiler warning. The existing forkConversationSubmenuItem helper already guards against this pattern by using isForkConversationDestination — but forkConversationDefaultTitle has no equivalent guard. Since .forkConversation is explicitly listed alongside .forkConversationRight, the default arm only catches truly unrelated actions (like .move or .reload), which would never be valid default fork actions anyway. Removing default and making the switch exhaustive over all TabContextAction cases would let the compiler enforce completeness as the enum evolves.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@lawrencecchen
lawrencecchen merged commit bf9728c into main Jul 15, 2026
5 checks passed
@lawrencecchen
lawrencecchen deleted the fix-fork-conversation-default-label branch July 15, 2026 10:14
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