Skip to content

feat(desktop): v2 diff viewer opens in its own tab + collapsed tab/pane title resolution#3420

Merged
saddlepaddle merged 4 commits into
mainfrom
diff-viewer
Apr 13, 2026
Merged

feat(desktop): v2 diff viewer opens in its own tab + collapsed tab/pane title resolution#3420
saddlepaddle merged 4 commits into
mainfrom
diff-viewer

Conversation

@saddlepaddle
Copy link
Copy Markdown
Collaborator

@saddlepaddle saddlepaddle commented Apr 13, 2026

Summary

  • Diff viewer opens in its own tab. Clicking a file in the Changes sidebar no longer hijacks the focused editor. openDiffPane scans all tabs for an existing diff pane (focuses + scrolls to the clicked file) and otherwise falls through to addTab directly. Manual splits still work; re-clicking from the sidebar finds a split-out diff pane in any tab rather than duplicating.
  • Collapsed tab/pane title resolution onto one canonical field. PaneDefinition.getTitle is tightened from (ctx) => ReactNode to (pane) => string | undefined. The file pane's rich JSX (italic-when-unpinned + has-changes dot) moves into the existing renderTitle hook where it belongs; 5 of 6 pane kinds already returned strings. A new resolveTabTitle(tab, tabs, registry) helper powers both the tab bar (Workspace.tsx) and the "Move to Tab" context menu (useDefaultContextMenuActions) — so tab-<uuid> labels are gone from everywhere they used to leak.
  • tab.titleOverride is reserved for user renames only. Every auto-default caller stripped: openFilePane / addTerminalTab / addChatTab / addBrowserTab, the three useWorkspaceHotkeys openers, the || "Terminal" fallback in useV2PresetExecution, preset execution's preset.name (now lives on the pane), and buildSetupPaneLayout's t.label (now lives on the pane). resolveTabTitle reads pane.titleOverride ?? registry[kind].getTitle(pane) for single-pane tabs, user rename wins over everything, and multi-pane tabs fall back to Tab N by LTR index.
  • Cleanup: getBrowserTabTitle free function deleted (duplicated browser.getTitle), tabTitle parameter dropped from store.openPane and its test updated, WorkspaceProps.getTabTitle prop deleted.

Test plan

  • bun run typecheck — all 25 packages pass
  • bun run lint:fix — clean
  • bun test packages/panes — 71/71
  • Click a file in the Changes sidebar from a focused file-editor tab → new "Changes" tab is created; the editor tab is untouched
  • Click a second changed file → no new tab; existing "Changes" tab focuses and scrolls to the new file
  • Rename the "Changes" tab to "My diff" → sticks through further sidebar clicks and splits (user rename wins)
  • Open a terminal / chat / file / browser tab → each shows its derived name, never tab-<uuid>
  • Split a Terminal tab with a Browser pane → tab label flips from "Terminal" to "Tab N"; reorder tabs → numbers track position
  • Run a named preset → tab + pane header show the preset name; split the tab → tab becomes "Tab N", pane header keeps the preset name
  • Bootstrap a fresh v2 workspace with labeled terminals → each tab shows its label, split → "Tab N", pane header keeps label
  • Right-click a pane → "Move to Tab" submenu lists resolved titles (no tab-<uuid>)

Summary by cubic

The v2 diff viewer opens in its own tab and never steals focus. Tab titles are pane‑derived and consistent; user renames win, “about:blank” shows “Browser”, and browser titles now preserve ports.

  • New Features

    • Clicking a changed file opens or focuses a dedicated diff tab; it reuses any existing diff pane across tabs and scrolls to the file, otherwise creates a new tab.
    • Tab titles are resolved via resolveTabTitle in @superset/panes (user rename > single‑pane pane title > “Tab N”); the “Move to Tab” menu uses the same logic, removing tab-<uuid> labels. Browser tabs use the page title or URL host (keeps port), falling back to “Browser”.
  • Migration

    • PaneDefinition.getTitle now accepts (pane) => string | undefined. Use renderTitle(ctx) for rich JSX.
    • WorkspaceProps.getTabTitle is removed and openPane no longer accepts tabTitle. Use resolveTabTitle and set auto names on pane.titleOverride; keep tab.titleOverride for user renames only.

Written for commit 7effd34. Summary will update on new commits.

Summary by CodeRabbit

  • Refactor

    • Centralized tab title resolution so titles are consistently derived (prefers explicit per-pane overrides, otherwise derives from pane content or a default "Tab X").
    • Pane title providers now accept a pane object and return plain text titles.
    • Workspace no longer accepts a custom external getTabTitle prop.
  • Chores

    • New title-resolution utility exposed for workspace components.
  • Tests

    • Adjusted tests to reflect removal of forced tab-title overrides.

… titles

openDiffPane now scans all tabs for an existing diff pane (focus + scroll)
and falls back to addTab, so clicking a file in the Changes sidebar never
hijacks the focused editor tab.

Collapses tab/pane title resolution onto a single canonical field:
PaneDefinition.getTitle is tightened to (pane) => string, file's rich JSX
moves into the existing renderTitle hook, and a new resolveTabTitle helper
powers both the tab bar and the "Move to Tab" context menu. tab.titleOverride
is reserved for user renames; every auto-default caller is stripped and
multi-pane tabs fall back to "Tab N" instead of "tab-<uuid>".
…user renames

Preset execution and workspace bootstrap were baking preset.name /
terminal.label onto tab.titleOverride, which meant those names persisted
misleadingly after a tab was split and couldn't be distinguished from a real
user rename. Move both to the pane's titleOverride instead, and teach
resolveTabTitle to read pane.titleOverride before falling through to
getTitle() for single-pane tabs. tab.titleOverride is now written only by
the tab-bar rename action; splitting a named tab flips the label to "Tab N"
while the pane keeps its name in its header, and user renames still win
over everything.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 13, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 029c3018-e7d1-4cc2-81f0-f6eaea4232d9

📥 Commits

Reviewing files that changed from the base of the PR and between 409b5f7 and 7effd34.

📒 Files selected for processing (1)
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx

📝 Walkthrough

Walkthrough

Moved per-tab title overrides into individual pane objects, introduced a centralized resolveTabTitle utility, changed PaneDefinition.getTitle to accept a pane, removed Workspace-level getTabTitle prop, and updated callers/tests/store to use pane-level title resolution.

Changes

Cohort / File(s) Summary
Pane title plumbing
apps/desktop/.../buildSetupPaneLayout.ts, apps/desktop/.../useV2PresetExecution/useV2PresetExecution.ts, apps/desktop/.../useWorkspaceHotkeys/useWorkspaceHotkeys.ts
Removed titleOverride from tab objects; terminal/preset pane creation now accepts optional titleOverride and callers stop passing tab-level title overrides.
Title resolution utility & exports
packages/panes/src/react/components/Workspace/utils/resolveTabTitle.ts, packages/panes/src/react/components/Workspace/index.ts, packages/panes/src/index.ts, packages/panes/src/react/index.ts
Added resolveTabTitle to compute tab titles (tab override → single-pane derived → default) and re-exported it from package barrels.
Workspace & Tab rendering
packages/panes/src/react/components/Workspace/Workspace.tsx, packages/panes/src/react/components/Workspace/components/Tab/components/Pane/Pane.tsx, packages/panes/src/react/types.ts
Removed external getTabTitle prop; Tab/Pane now use resolveTabTitle; changed PaneDefinition.getTitle signature to accept pane and return `string
Pane registry / Browser-specific changes
apps/desktop/.../usePaneRegistry/usePaneRegistry.tsx, apps/desktop/.../components/BrowserPane/BrowserPane.tsx, apps/desktop/.../components/BrowserPane/index.ts
Refactored registry getTitle implementations to operate on pane instead of renderer context; removed exported getBrowserTabTitle helper.
Context menu & page updates
apps/desktop/.../useDefaultContextMenuActions/useDefaultContextMenuActions.tsx, apps/desktop/.../page.tsx
useDefaultContextMenuActions now accepts paneRegistry and uses resolveTabTitle for "Move to Tab" labels; page no longer supplies tab-level title overrides and passes paneRegistry into the hook.
Store API & tests
packages/panes/src/core/store/store.ts, packages/panes/src/core/store/store.test.ts
Removed tabTitle?: string from WorkspaceStore.openPane args and eliminated propagation of tab title overrides; updated tests to match removed behavior.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Page as Page
  participant Hook as useDefaultContextMenuActions
  participant Registry as PaneRegistry
  participant Workspace as Workspace/TabBar

  Page->>Hook: call useDefaultContextMenuActions(paneRegistry)
  Hook->>Registry: request resolveTabTitle(tab, tabs, paneRegistry)
  Registry->>Workspace: ask pane definition getTitle(pane)
  Workspace-->>Registry: returns derived title or undefined
  Registry-->>Hook: returns final tab label
  Hook-->>Page: context-menu items with resolved labels
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 I nudged a pane and titles hopped in place,

Each pane now wears its name with gentle pride,
resolveTabTitle hums, no more tab-level chase,
My whiskers twitch where tidy names reside.
🥕 Hooray — panes blossom, every label tied!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the two main changes: diff viewer opens in its own tab and tab/pane title resolution is consolidated.
Description check ✅ Passed The PR description comprehensively covers all required template sections with detailed explanations of changes, test plan, and migration guidance.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch diff-viewer

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 and usage tips.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Apr 13, 2026

Greptile Summary

This PR delivers three related improvements to the v2 desktop workspace: (1) diff pane viewer opens in its own dedicated tab rather than hijacking the active editor; (2) tab/pane title resolution is unified through a single resolveTabTitle helper that eliminates raw tab-<uuid> labels everywhere they leaked; and (3) tab.titleOverride is reserved exclusively for user renames — all auto-default callers (hotkeys, preset execution, setup layout, browser/chat/terminal openers) have their hardcoded titleOverride strings stripped, with labels now derived from the pane registry's getTitle at render time.

Key changes:

  • New resolveTabTitle(tab, tabs, registry) — canonical helper used in the tab bar (Workspace.tsx) and the "Move to Tab" context menu (useDefaultContextMenuActions), replacing the old tab.titleOverride ?? getTabTitle?.(tab) ?? tab.id chain.
  • PaneDefinition.getTitle signature narrowed from (ctx: RendererContext) => ReactNode to (pane: Pane) => string | undefined; rich JSX decoration for the file pane moved to the existing renderTitle hook where it belongs.
  • openDiffPane refactored to scan all tabs (not only the active one) for an existing diff pane, reusing it instead of opening a duplicate; falls through to addTab to create a new dedicated tab when none is found.
  • store.openPane API simplifiedtabTitle parameter removed; WorkspaceProps.getTabTitle prop deleted; getBrowserTabTitle free function deleted (was a duplicate of browser.getTitle).
  • One regression identified: browser.getTitle now returns undefined for unnavigated (about:blank) browser panes. Pane.tsx falls back to pane.id (a raw UUID) when getTitle returns undefined and no titleOverride is set, so the pane header of a freshly opened browser tab shows a UUID instead of a human-readable label. A one-line return \"Browser\" fix is suggested inline.

Confidence Score: 4/5

Safe to merge after fixing the one-line browser getTitle fallback to prevent UUID appearing as pane header.

The architecture is well-thought-out: resolveTabTitle cleanly unifies two divergent title-resolution paths, the getTitle/renderTitle separation is correct for all panes, the openDiffPane tab-search is correct, and all 71 package tests pass. The single concrete regression — browser.getTitle returning undefined for about:blank causes the pane header to fall back to pane.id (a UUID) — is a targeted one-line fix and does not affect the primary diff-viewer or title-resolution flows.

usePaneRegistry.tsx: browser.getTitle missing "Browser" fallback; resolveTabTitle.ts: indexOf edge case with Tab 0.

Important Files Changed

Filename Overview
packages/panes/src/react/components/Workspace/utils/resolveTabTitle.ts New helper that canonicalises tab-bar titles: tab.titleOverride → single-pane title → "Tab N". Works correctly for all live callers; minor edge case where indexOf returns -1 gives "Tab 0".
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx getTitle/renderTitle split is clean for all panes except browser: getTitle can return undefined for about:blank, causing the pane header to display a raw UUID instead of a human-readable label.
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx openDiffPane now scans all tabs (not just active) for an existing diff pane, and falls through to addTab. Clean removal of titleOverride from auto-opened tabs.
packages/panes/src/core/store/store.ts tabTitle parameter removed from openPane interface and implementation; the pane-level titleOverride in CreatePaneInput carries that responsibility now.
packages/panes/src/react/types.ts PaneDefinition.getTitle tightened to (pane) => string
packages/panes/src/react/components/Workspace/components/Tab/components/Pane/Pane.tsx Pane header title now calls getTitle(pane) instead of getTitle(context); getTitle for browser can return undefined, which cascades to pane.id as fallback — the root cause of the pane header UUID regression.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["resolveTabTitle(tab, tabs, registry)"] --> B{tab.titleOverride?}
    B -- yes --> Z["return titleOverride (user rename wins)"]
    B -- no --> C{single pane?}
    C -- no --> D["return 'Tab N' (LTR index)"]
    C -- yes --> E{pane.titleOverride?}
    E -- yes --> F["return pane.titleOverride"]
    E -- no --> G["registry[kind].getTitle(pane)"]
    G -- string --> H["return string"]
    G -- undefined --> D

    subgraph TabBar["Tab Bar (Workspace.tsx)"]
        T1["getTabTitle = resolveTabTitle(tab, tabs, registry)"]
    end

    subgraph ContextMenu["Move to Tab Menu (useDefaultContextMenuActions)"]
        T2["label = resolveTabTitle(tab, tabs, paneRegistry)"]
    end

    subgraph PaneHeader["Pane Header (Pane.tsx)"]
        T3["title = pane.titleOverride ?? registry[kind].getTitle(pane) ?? pane.id"]
        T4["titleContent = registry[kind].renderTitle(ctx)"]
    end

    A --> TabBar
    A --> ContextMenu
Loading

Reviews (1): Last reviewed commit: "feat(desktop): pane-derived tab titles r..." | Re-trigger Greptile

Comment on lines 264 to 275
browser: {
getIcon: () => <Globe className="size-4" />,
getTitle: (ctx: RendererContext<PaneViewerData>) => {
const data = ctx.pane.data as BrowserPaneData;
return data.pageTitle || data.url;
getTitle: (pane) => {
const data = pane.data as BrowserPaneData;
if (data.pageTitle) return data.pageTitle;
if (data.url && data.url !== "about:blank") {
try {
return new URL(data.url).hostname;
} catch {}
}
return undefined;
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Browser pane header falls back to raw UUID when tab is unnavigated

browser.getTitle now returns undefined when the URL is "about:blank" and there's no pageTitle. Pane.tsx resolves the header title via:

pane.titleOverride ?? definition.getTitle?.(pane) ?? pane.id

When getTitle returns undefined and titleOverride is unset (which is the case for a freshly-opened browser tab), the pane header displays the raw pane UUID (e.g. "pane-abc123..."). Before this PR, browser.getTitle fell back to data.url (so at least "about:blank" appeared), and addBrowserTab set titleOverride: "Browser". Both safety nets are now gone.

Add a "Browser" fallback so unnavigated browser panes show a human-readable header title:

Suggested change
browser: {
getIcon: () => <Globe className="size-4" />,
getTitle: (ctx: RendererContext<PaneViewerData>) => {
const data = ctx.pane.data as BrowserPaneData;
return data.pageTitle || data.url;
getTitle: (pane) => {
const data = pane.data as BrowserPaneData;
if (data.pageTitle) return data.pageTitle;
if (data.url && data.url !== "about:blank") {
try {
return new URL(data.url).hostname;
} catch {}
}
return undefined;
},
getTitle: (pane) => {
const data = pane.data as BrowserPaneData;
if (data.pageTitle) return data.pageTitle;
if (data.url && data.url !== "about:blank") {
try {
return new URL(data.url).hostname;
} catch {}
}
return "Browser";
},

onlyPane.titleOverride ?? registry[onlyPane.kind]?.getTitle?.(onlyPane);
if (fromPane) return fromPane;
}
return `Tab ${tabs.indexOf(tab) + 1}`;
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 "Tab 0" can appear if tab is not found in tabs

tabs.indexOf(tab) relies on reference equality. If tab ever arrives from a different snapshot than tabs (e.g. a stale closure in an async context), indexOf returns -1 and the label becomes "Tab 0". All current callers pass tab as an element from the same tabs array so this is low-risk today, but the fallback is fragile.

A defensive guard makes the intent explicit and prevents a confusing "Tab 0" label:

Suggested change
return `Tab ${tabs.indexOf(tab) + 1}`;
return `Tab ${tabs.indexOf(tab) + 1 || tabs.length}`;

Or add a comment documenting the invariant so future callers know tab must be a member of tabs.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 13, 2026

🚀 Preview Deployment

🔗 Preview Links

Service Status Link
Neon Database (Neon) View Branch
Fly.io Electric (Fly.io) Failed to deploy
Vercel API (Vercel) Open Preview
Vercel Web (Vercel) Open Preview
Vercel Marketing (Vercel) Open Preview
Vercel Admin (Vercel) Open Preview
Vercel Docs (Vercel) Open Preview

Preview updates automatically with new commits

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx (1)

143-159: ⚠️ Potential issue | 🟠 Major

Prefer the active tab's diff pane before the global scan.

Line 146 starts with every tab in order, so a background diff tab can win before a split diff pane in the active tab. In that case, clicking the Changes sidebar jumps the user away from the current layout instead of reusing the split-out pane this flow is supposed to preserve.

💡 Suggested fix
 	const openDiffPane = useCallback(
 		(filePath: string) => {
 			const state = store.getState();
-			for (const tab of state.tabs) {
+			const activeTab = state.tabs.find((tab) => tab.id === state.activeTabId);
+			const tabSearchOrder = activeTab
+				? [activeTab, ...state.tabs.filter((tab) => tab.id !== activeTab.id)]
+				: state.tabs;
+
+			for (const tab of tabSearchOrder) {
 				for (const pane of Object.values(tab.panes)) {
 					if (pane.kind !== "diff") continue;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/page.tsx
around lines 143 - 159, The current openDiffPane callback scans every tab in
order so a background diff pane can be chosen before a diff pane in the
currently active tab; update openDiffPane (which uses store.getState(),
state.tabs, pane.kind and calls state.setPaneData, state.setActiveTab and
state.setActivePane) to first look for a diff pane inside the active tab (use
state.activeTab or activeTabId from state) and reuse that pane if found, and
only if not found fall back to scanning all other tabs for a diff pane; ensure
the same setPaneData/setActiveTab/setActivePane flow is applied when reusing the
active-tab pane.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx:
- Around line 266-275: The fallback title generator in getTitle for
BrowserPaneData currently uses new URL(data.url).hostname which drops port
numbers (e.g., localhost:3000 -> localhost); update that expression to use new
URL(data.url).host so the returned fallback includes the port when present,
keeping the surrounding try/catch and existing conditions intact.

In `@packages/panes/src/react/components/Workspace/utils/resolveTabTitle.ts`:
- Line 17: The fallback uses tabs.indexOf(tab) which can return -1 for
equivalent but non-identical tab objects; update resolveTabTitle to compute
index with a robust lookup (e.g., use tabs.findIndex(t => t.id === tab.id ||
t.key === tab.key || t.title === tab.title) instead of indexOf) and then build
the label with a safe fallback like Math.max(1, foundIndex + 1) so it never
produces "Tab 0".

---

Outside diff comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/page.tsx:
- Around line 143-159: The current openDiffPane callback scans every tab in
order so a background diff pane can be chosen before a diff pane in the
currently active tab; update openDiffPane (which uses store.getState(),
state.tabs, pane.kind and calls state.setPaneData, state.setActiveTab and
state.setActivePane) to first look for a diff pane inside the active tab (use
state.activeTab or activeTabId from state) and reuse that pane if found, and
only if not found fall back to scanning all other tabs for a diff pane; ensure
the same setPaneData/setActiveTab/setActivePane flow is applied when reusing the
active-tab pane.
🪄 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: 688be2d9-491d-4816-8cc9-fdc70d24c6a6

📥 Commits

Reviewing files that changed from the base of the PR and between 039edf2 and 17d4666.

📒 Files selected for processing (17)
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/buildSetupPaneLayout.ts
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useDefaultContextMenuActions/useDefaultContextMenuActions.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/BrowserPane/BrowserPane.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/BrowserPane/index.ts
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2PresetExecution/useV2PresetExecution.ts
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useWorkspaceHotkeys/useWorkspaceHotkeys.ts
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx
  • packages/panes/src/core/store/store.test.ts
  • packages/panes/src/core/store/store.ts
  • packages/panes/src/index.ts
  • packages/panes/src/react/components/Workspace/Workspace.tsx
  • packages/panes/src/react/components/Workspace/components/Tab/components/Pane/Pane.tsx
  • packages/panes/src/react/components/Workspace/index.ts
  • packages/panes/src/react/components/Workspace/utils/resolveTabTitle.ts
  • packages/panes/src/react/index.ts
  • packages/panes/src/react/types.ts
💤 Files with no reviewable changes (4)
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useWorkspaceHotkeys/useWorkspaceHotkeys.ts
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/BrowserPane/BrowserPane.tsx
  • packages/panes/src/core/store/store.test.ts
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/BrowserPane/index.ts

onlyPane.titleOverride ?? registry[onlyPane.kind]?.getTitle?.(onlyPane);
if (fromPane) return fromPane;
}
return `Tab ${tabs.indexOf(tab) + 1}`;
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.

⚠️ Potential issue | 🟡 Minor

Fallback can produce Tab 0 when tab is not the same object reference.

Line 17 uses tabs.indexOf(tab), which depends on referential equality; if callers pass an equivalent tab object, index becomes -1 and UI label becomes Tab 0.

🔧 Suggested fix
-	return `Tab ${tabs.indexOf(tab) + 1}`;
+	const idx = tabs.findIndex((t) => t.id === tab.id);
+	return `Tab ${Math.max(0, idx) + 1}`;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/panes/src/react/components/Workspace/utils/resolveTabTitle.ts` at
line 17, The fallback uses tabs.indexOf(tab) which can return -1 for equivalent
but non-identical tab objects; update resolveTabTitle to compute index with a
robust lookup (e.g., use tabs.findIndex(t => t.id === tab.id || t.key ===
tab.key || t.title === tab.title) instead of indexOf) and then build the label
with a safe fallback like Math.max(1, foundIndex + 1) so it never produces "Tab
0".

Unnavigated browser panes had their pane header fall through to pane.id
(a raw UUID) because getTitle returned undefined for about:blank and the
old titleOverride: "Browser" default was removed along with the other
auto-default titleOverride writes.
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

2 issues found across 17 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx">

<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx:271">
P3: Use `.host` instead of `.hostname` to preserve the port in browser tab titles. Currently `localhost:3000` and `localhost:4000` both resolve to `"localhost"`, making them indistinguishable.</violation>

<violation number="2" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx:274">
P2: Fresh browser panes display a raw UUID. When URL is `about:blank` and `pageTitle` is unset, `getTitle` returns `undefined`. Combined with the removal of `titleOverride: "Browser"` from `addBrowserTab`, the fallback in `Pane.tsx` resolves to `pane.id` (a UUID string). Return `"Browser"` here instead of `undefined`.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

return new URL(data.url).hostname;
} catch {}
}
return undefined;
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Apr 13, 2026

Choose a reason for hiding this comment

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

P2: Fresh browser panes display a raw UUID. When URL is about:blank and pageTitle is unset, getTitle returns undefined. Combined with the removal of titleOverride: "Browser" from addBrowserTab, the fallback in Pane.tsx resolves to pane.id (a UUID string). Return "Browser" here instead of undefined.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx, line 274:

<comment>Fresh browser panes display a raw UUID. When URL is `about:blank` and `pageTitle` is unset, `getTitle` returns `undefined`. Combined with the removal of `titleOverride: "Browser"` from `addBrowserTab`, the fallback in `Pane.tsx` resolves to `pane.id` (a UUID string). Return `"Browser"` here instead of `undefined`.</comment>

<file context>
@@ -262,9 +263,15 @@ export function usePaneRegistry(
+							return new URL(data.url).hostname;
+						} catch {}
+					}
+					return undefined;
 				},
 				renderPane: (ctx: RendererContext<PaneViewerData>) => (
</file context>
Suggested change
return undefined;
return "Browser";
Fix with Cubic

URL.hostname drops the port, so localhost:3000 and localhost:4000 both
rendered as "localhost" in the tab bar. URL.host keeps the port when one
is explicitly set.
@saddlepaddle saddlepaddle merged commit 3dd1de2 into main Apr 13, 2026
13 of 14 checks passed
MocA-Love pushed a commit to MocA-Love/superset that referenced this pull request Apr 14, 2026
…ne title resolution (superset-sh#3420)

* feat(desktop): v2 diff viewer opens in its own tab + pane-derived tab titles

openDiffPane now scans all tabs for an existing diff pane (focus + scroll)
and falls back to addTab, so clicking a file in the Changes sidebar never
hijacks the focused editor tab.

Collapses tab/pane title resolution onto a single canonical field:
PaneDefinition.getTitle is tightened to (pane) => string, file's rich JSX
moves into the existing renderTitle hook, and a new resolveTabTitle helper
powers both the tab bar and the "Move to Tab" context menu. tab.titleOverride
is reserved for user renames; every auto-default caller is stripped and
multi-pane tabs fall back to "Tab N" instead of "tab-<uuid>".

* feat(desktop): pane-derived tab titles reserve tab.titleOverride for user renames

Preset execution and workspace bootstrap were baking preset.name /
terminal.label onto tab.titleOverride, which meant those names persisted
misleadingly after a tab was split and couldn't be distinguished from a real
user rename. Move both to the pane's titleOverride instead, and teach
resolveTabTitle to read pane.titleOverride before falling through to
getTitle() for single-pane tabs. tab.titleOverride is now written only by
the tab-bar rename action; splitting a named tab flips the label to "Tab N"
while the pane keeps its name in its header, and user renames still win
over everything.

* fix(desktop): browser.getTitle falls back to "Browser" for about:blank

Unnavigated browser panes had their pane header fall through to pane.id
(a raw UUID) because getTitle returned undefined for about:blank and the
old titleOverride: "Browser" default was removed along with the other
auto-default titleOverride writes.

* fix(desktop): browser tab title uses URL.host to preserve port

URL.hostname drops the port, so localhost:3000 and localhost:4000 both
rendered as "localhost" in the tab bar. URL.host keeps the port when one
is explicitly set.
MocA-Love pushed a commit to MocA-Love/superset that referenced this pull request Apr 14, 2026
…ff viewer

Follow-up fixes for superset-sh#3403 and superset-sh#3420 cherry-picks:

hotkeys/types.ts:
- Widen PlatformKey and HotkeyDefinition.key to allow null per platform
  (mirrors upstream superset-sh#3422; needed here because v1 workspace uses fork's
  tRPC-based PREV/NEXT_WORKSPACE handlers that reference null-bound keys)

hotkeys/registry.ts:
- Re-add PREV_TAB, NEXT_TAB, PREV_WORKSPACE, NEXT_WORKSPACE as null-bound
  (matches upstream superset-sh#3422 final state; deleted by superset-sh#3403's auto-merge but
  fork still needs the IDs for v1 tRPC-based workspace nav handlers)

hotkeys/useRecordHotkeys.ts:
- Null-guard canonicalizeChord(defaultKey) so recording a new chord for
  an unbound hotkey doesn't throw

routes/workspace/$workspaceId/page.tsx:
- Restore navigateToWorkspace import (removed by cherry-pick auto-merge);
  fork's tRPC-based PREV/NEXT_WORKSPACE handlers still need it

routes/v2-workspace/$workspaceId/page.tsx:
- Remove tabTitle argument from openPane call (deprecated by superset-sh#3420;
  pane-level titleOverride handles tab titles now via resolveTabTitle)

FORK NOTE: the registry entries and types.ts widening overlap with PR#2
(superset-sh#3422 cherry-pick). When both PRs merge, git detects identical changes
and auto-resolves.
MocA-Love pushed a commit to MocA-Love/superset that referenced this pull request Apr 14, 2026
…ne title resolution (superset-sh#3420)

* feat(desktop): v2 diff viewer opens in its own tab + pane-derived tab titles

openDiffPane now scans all tabs for an existing diff pane (focus + scroll)
and falls back to addTab, so clicking a file in the Changes sidebar never
hijacks the focused editor tab.

Collapses tab/pane title resolution onto a single canonical field:
PaneDefinition.getTitle is tightened to (pane) => string, file's rich JSX
moves into the existing renderTitle hook, and a new resolveTabTitle helper
powers both the tab bar and the "Move to Tab" context menu. tab.titleOverride
is reserved for user renames; every auto-default caller is stripped and
multi-pane tabs fall back to "Tab N" instead of "tab-<uuid>".

* feat(desktop): pane-derived tab titles reserve tab.titleOverride for user renames

Preset execution and workspace bootstrap were baking preset.name /
terminal.label onto tab.titleOverride, which meant those names persisted
misleadingly after a tab was split and couldn't be distinguished from a real
user rename. Move both to the pane's titleOverride instead, and teach
resolveTabTitle to read pane.titleOverride before falling through to
getTitle() for single-pane tabs. tab.titleOverride is now written only by
the tab-bar rename action; splitting a named tab flips the label to "Tab N"
while the pane keeps its name in its header, and user renames still win
over everything.

* fix(desktop): browser.getTitle falls back to "Browser" for about:blank

Unnavigated browser panes had their pane header fall through to pane.id
(a raw UUID) because getTitle returned undefined for about:blank and the
old titleOverride: "Browser" default was removed along with the other
auto-default titleOverride writes.

* fix(desktop): browser tab title uses URL.host to preserve port

URL.hostname drops the port, so localhost:3000 and localhost:4000 both
rendered as "localhost" in the tab bar. URL.host keeps the port when one
is explicitly set.
MocA-Love pushed a commit to MocA-Love/superset that referenced this pull request Apr 14, 2026
…ff viewer

Follow-up fixes for superset-sh#3403 and superset-sh#3420 cherry-picks:

hotkeys/types.ts:
- Widen PlatformKey and HotkeyDefinition.key to allow null per platform
  (mirrors upstream superset-sh#3422; needed here because v1 workspace uses fork's
  tRPC-based PREV/NEXT_WORKSPACE handlers that reference null-bound keys)

hotkeys/registry.ts:
- Re-add PREV_TAB, NEXT_TAB, PREV_WORKSPACE, NEXT_WORKSPACE as null-bound
  (matches upstream superset-sh#3422 final state; deleted by superset-sh#3403's auto-merge but
  fork still needs the IDs for v1 tRPC-based workspace nav handlers)

hotkeys/useRecordHotkeys.ts:
- Null-guard canonicalizeChord(defaultKey) so recording a new chord for
  an unbound hotkey doesn't throw

routes/workspace/$workspaceId/page.tsx:
- Restore navigateToWorkspace import (removed by cherry-pick auto-merge);
  fork's tRPC-based PREV/NEXT_WORKSPACE handlers still need it

routes/v2-workspace/$workspaceId/page.tsx:
- Remove tabTitle argument from openPane call (deprecated by superset-sh#3420;
  pane-level titleOverride handles tab titles now via resolveTabTitle)

FORK NOTE: the registry entries and types.ts widening overlap with PR#2
(superset-sh#3422 cherry-pick). When both PRs merge, git detects identical changes
and auto-resolves.
MocA-Love pushed a commit to MocA-Love/superset that referenced this pull request Apr 14, 2026
All 9 upstream commits have been individually cherry-picked via PR#159~#163:

| Upstream | Our PR | Description |
|---|---|---|
| d656b7e (superset-sh#3415) | #159 (PR#1) | terminal clipboard handling |
| 31fcf19 (superset-sh#3416) | #162 (PR#4) | v1 split pane startup sizing fix |
| 039edf2 (superset-sh#3403) | #161 (PR#3) | Cmd+Alt+Arrow spatial pane focus |
| b18a00c (superset-sh#3421) | #159 (PR#1) | v2 right sidebar toggle reactive |
| 3dd1de2 (superset-sh#3420) | #161 (PR#3) | v2 diff viewer + tab title resolution |
| b42a114 (superset-sh#3418) | #159 (PR#1) | CodeMirror hotkey enablement |
| c925f4d (superset-sh#3422) | #160 (PR#2) | unbound defaults + restore prev/next tab/workspace |
| bb12c09 (superset-sh#3419) | #163 (PR#5) | version bump 1.5.3 |
| 47efa73 (superset-sh#3432) | #159 (PR#1) | pending/update-required error selectable |

Fork-specific features preserved:
- auto-updater (IS_FORK, GitHub Releases API)
- QuitMode/cleanupMainWindowResources lifecycle
- GitHubSyncService, SpreadsheetViewer
- BROWSER_RELOAD / BROWSER_HARD_RELOAD / SEARCH_IN_FILES hotkeys
- HotkeyCategory "Browser"
- v1 deep-link navigation (useSearch/WorkspaceSearchParams)
- v1 tRPC-based PREV/NEXT_WORKSPACE handlers
- v1 CLOSE_TERMINAL/CLOSE_TAB hotkey handlers
- v2 extra state (rightSidebarOpenViewWidth, showPresetsBar)
@Kitenite Kitenite deleted the diff-viewer branch May 6, 2026 04:53
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