fix(sidebar): stop labeling split-pane agent rows with the focused pane's title - #11070
pythonstrup wants to merge 5 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthrough
Merge Risk: ⚪ Minimal · up to The change makes split-pane rows use stable per-pane labels instead of the focused pane's live title, preventing duplicate and shifting names while preserving single-pane behavior. No actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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. Comment |
pythonstrup
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/shared/agent-row-conversation-name.ts (1)
127-131: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCondense this implementation-detail comment.
Keep the focused-pane rationale, but remove the PTY implementation walkthrough and repeated precedence explanation.
Suggested revision
- // Why: `tab.title` carries only the focused pane's live title (pty-connection - // keeps it that way so split agents don't make the tab flicker). In a split - // tab it names one pane and mislabels its siblings, and it flips as focus - // moves — so drop it there and let each row keep its own per-pane label. The - // tab-owned names above stay: the user gave those to the whole tab. + // A split tab's live title names only its focused pane, not individual agent rows.As per coding guidelines, “Comments must be concise, limited to non-obvious information, and preferably one line.”
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 38e8adcf-2ab4-47a3-b3e1-2c0b73bb54fd
📥 Commits
Reviewing files that changed from the base of the PR and between 256c0bc6f48b9c0503f777eb9ae2e37f14779d44 and 20a6c259499b10bcc64d9de38018ef5ee1ff62ca.
📒 Files selected for processing (5)
src/renderer/src/components/dashboard/build-dashboard-snapshot.tssrc/renderer/src/components/dashboard/use-agent-row-conversation-name.test.tssrc/renderer/src/components/dashboard/use-agent-row-conversation-name.tssrc/shared/agent-row-conversation-name.test.tssrc/shared/agent-row-conversation-name.ts
20a6c25 to
439c1a2
Compare
Greptile SummaryThis PR fixes a mislabeling bug in the sidebar and dashboard pop-out board where all agent rows in a split-pane tab displayed the same name — the live title of whichever pane was focused — and that name flipped whenever focus changed. The root cause is that
Confidence Score: 5/5Safe to merge. The change is a minimal, surgical boolean guard on a single string-resolution path; all three tab-owned name sources are unaffected, single-pane tabs are untouched, and missing layout data falls back to the pre-fix behavior rather than introducing any new wrong value. The fix isolates the mislabeling to exactly the Files Needing Attention: No files require special attention. The change is contained within the five listed files, with the shared resolver being the most critical; its tests directly pin the new behavior.
|
| Filename | Overview |
|---|---|
| src/shared/agent-row-conversation-name.ts | Adds optional tabHasSplitPanes parameter (defaults to false) that suppresses live-title sourcing for split tabs while keeping customTitle, quickCommandLabel, and generatedTitle intact. |
| src/renderer/src/components/dashboard/use-agent-row-conversation-name.ts | Adds a third useAppStore selector that computes a boolean split-pane flag from terminalLayoutsByTabId; all three hook calls correctly precede the cannotOwnTabName early-return, preserving React hook ordering rules. |
| src/renderer/src/components/dashboard/build-dashboard-snapshot.ts | Threads tabHasSplitPanes into the per-card rowConversationName call using the already-available worktree-scoped terminalLayoutsByTabId, keeping the board in sync with the sidebar hook. |
| src/shared/agent-row-conversation-name.test.ts | Adds tests verifying live title is dropped (including OpenCode semantic form) in a split tab, while customTitle, quickCommandLabel, and generatedTitle are preserved. |
| src/renderer/src/components/dashboard/use-agent-row-conversation-name.test.ts | Updates existing tests to carry the new terminalLayoutsByTabId store slice and adds two new scenarios — split layout returns null, leaf layout returns the live title — that would catch regression to the pre-fix behavior on main. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["getAgentRowConversationName(tab, agentType, generatedTitlesEnabled, tabHasSplitPanes)"] --> B{customTitle?}
B -- yes --> C["return customTitle"]
B -- no --> D{quickCommandLabel?}
D -- yes --> E["return quickCommandLabel"]
D -- no --> F{tabHasSplitPanes?}
F -- yes --> G["liveTitle = ''"]
F -- no --> H["liveTitle = tab.title?.trim()"]
G --> I{isMeaningfulOpenCodeTerminalTitle?}
H --> I
I -- yes --> J["return liveTitle (OpenCode)"]
I -- no --> K{generatedTitlesEnabled && generatedTitle?}
K -- yes --> L["return generatedTitle"]
K -- no --> M{liveTitle empty?}
M -- yes --> N["return null (split rows fall back to per-pane label)"]
M -- no --> O["return conversationNameFromLiveTitle(liveTitle, ...)"]
Reviews (3): Last reviewed commit: "docs(sidebar): trim the split-title comm..." | Re-trigger Greptile
|
Re: the nitpick on the comment at // Why: pty-connection only propagates the focused pane's title to the tab
// (deliberately, to stop split agents flickering), so in a split it mislabels siblings.What went is the part you were right about: the precedence restatement ("the tab-owned names above stay…") narrated the three short-circuits directly above it, and a test already pins that behavior. I didn't take the suggested one-liner verbatim, though, because it drops the one thing a reader can't derive from this file — that // pty-connection.ts:2682
// Why: only the focused pane should drive the tab title — otherwise two
// agents in split panes cause rapid title flickering as each emits OSC sequences.
if (manager.getActivePane()?.id === pane.id) {
deps.updateTabTitle(deps.tabId, paneTitle)
}Without that pointer, "the tab title only ever tracks one pane" reads like a bug worth fixing rather than a deliberate anti-flicker measure, and the next reader has no way to check the claim before acting on it. The guideline is "1 LINE if possible" — this is the case where the second line carries the non-obvious half. |
ddf1ed0 to
9ac3e28
Compare
9ac3e28 to
45cc0fe
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Sidebar and dashboard agent rows render per pane but resolved their label from tab-level state. `tab.title` intentionally tracks only the focused pane, so every row in a split tab showed the focused pane's name and both rows relabeled whenever the user clicked the other split. Pass the row's own pane title into the conversation-name resolver and let it replace the live-title source only. Tab-owned names (customTitle, quickCommandLabel, generatedTitle) keep their precedence, since those are names the user gave the tab. Single-pane tabs are unchanged: their `entry.terminalTitle` equals `tab.title`. Fixes stablyai#11069 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Before an agent sets a real title, the tab and pane both carry the identity echo, which the resolver rejects — rows fall back to their own prompt and already look correct. Pin that, so the shared-name symptom stays understood as something that starts at the first real title, not at row creation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ne's title Sidebar and dashboard agent rows render per pane but took their name from `tab.title`, which by design carries only the focused pane's live title — pty-connection keeps it that way so two agents in splits do not make the tab title flicker. In a split tab that title names one pane and mislabels its siblings, and it flips as focus moves, so every row in the tab showed the same name and all of them changed when the user clicked another split. Drop the live title for rows whose tab has more than one pane and let each row keep its own per-pane label. Tab-owned names (customTitle, quickCommandLabel, generatedTitle) still apply to every row in the tab — those are names the user gave the tab, not one pane. Single-pane tabs, which are the case stablyai#9989 was written for, are untouched. The split test is the layout root being a split node, so the row subscribes to splits rather than to every title frame. Fixes stablyai#11069 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…outs Use the selector result already in scope instead of re-reading the whole store map, matching how the pty lookup a few lines up resolves its layout. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The five-line version narrated the precedence the code right above it already shows. Keep the pointer to where the focused-pane invariant is actually enforced (pty-connection.ts) — that is the part a reader cannot derive from this file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
45cc0fe to
6dddb49
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
Closing as superseded: #14707 (STA-2811) fixed the same bug by resolving each split-pane row's own leaf pane title via paneLiveTitle, which covers this PR's scenario without suppressing live titles in splits. |
2 participants
Fixes #11069
Summary
AS-IS — Sidebar and dashboard agent rows are rendered per pane, but they take their name from
tab.title. That field carries only the focused pane's live title —pty-connection.tskeeps it that way on purpose, so two agents in splits don't make the tab title flicker, anduse-terminal-pane-lifecycle.tsre-syncs it on every focus change. In a split tab that title names one pane and mislabels its siblings, so every row in the tab showed the same name, and all of them changed when the user clicked another split.TO-BE — A tab with more than one pane no longer lends its live title to any row. Each row keeps its own per-pane label instead, so split rows stop reading as duplicates and stop flipping.
Introduced by #9989, whose design note reads "There is no additional preference: row identity consistently follows tab identity". Split panes are the case where one tab holds several independent sessions, and they were not considered.
What changed
getAgentRowConversationNamegains atabHasSplitPanesflag that suppresses the live-title source:That covers both live-title branches — the OpenCode semantic
OC | …title and the generalconversationNameFromLiveTitlepath.Tab-owned names deliberately still apply to every row in the tab:
customTitle,quickCommandLabel, andgeneratedTitleare names the user (or Orca) gave the whole tab, not one pane, and none of them flip on focus. Only the live title is pane-specific by nature, and only it is dropped.When a row has no conversation name it falls back to
getAgentRowPrimaryText(entry)— the pane's own prompt, keyed bypaneKey. That is the same per-pane labeling users already see before an agent's first turn, when the identity-echo title (✳ Claude Code) is rejected and no conversation name exists.The split test is
layout.root.type === 'split', which is true exactly when the tab holds more than one leaf. The hook selects the boolean, not the layout, so a row re-renders on splits rather than on every title frame.Both surfaces are updated —
use-agent-row-conversation-name.ts(sidebar worktree-card rows and dashboard rows) andbuild-dashboard-snapshot.ts(the pop-out board, which mirrors the hook and had the same defect).Single-pane tabs — the case #9989 was written for — are untouched.
Side effects, both on the pop-out board
Every consumer of
conversationNamewas traced. Four are pure display fallbacks or optional-field checks that behave identically; two are worth stating.1. The kanban card heading. The card falls back differently from the sidebar row. The sidebar drops to the pane's own prompt;
AgentKanbanCardusescard.conversationName ?? card.worktreeName, andworktreeInFooteris keyed off the same value. So for split panes the heading now reads the worktree name and the worktree leaves the footer.That heading is still shared between the two cards — but it is now a stable, true shared value (they really are in one worktree) instead of one pane's title shown on both and flipping on focus. The per-pane distinction stays visible in the card body, which renders each pane's own
lastUserMessage/lastAgentMessage. Verified deliberately rather than assumed, because the two surfaces do not share a fallback.2. Board search (#11042).
conversationNameis one of the ten fields incardSearchText, so it is a search key, not only a label. Today a split tab puts the focused pane's title into both cards' search text:So the term this PR removes from the haystack is the wrong one, and the per-pane keys (
task,lastUserMessage,lastAgentMessage) stay. Search stops depending on which split has focus.cardSearchTextruns.filter(Boolean), so an absent value is skipped rather than stringified.Known limits, deliberately not fixed here
runtimePaneTitlesByTabId) is keyed by ephemeral numeric pane ids, while rows are keyed bytabId:leafId. Giving split rows real titles means adding a leaf-keyed live-title channel — worth doing, but it is infrastructure, not this bug fix. Showing this pane's prompt beats showing another pane's title.layout.titlesByLeafId, set via the pane rename shortcut) are still not surfaced on agent rows. Same root cause as this bug, separate symptom; noted on [Bug]: Split panes in one tab share a single sidebar agent-row name that flips to whichever pane was clicked last #11069.root: nullsnapshot reverts to today's behavior.persistLayoutSnapshotwrites unconditionally, so a snapshot taken while the container is being torn down can store a null root. The split test then reads false and the rows fall back to the pre-fix shared title — a transient reversion to the status quo, never a new wrong name, so it is not worth a guard.Screenshots
No visual change — no layout, spacing, color, typography, or component change. The only difference is which text an already-existing row resolves, shown in the before/after above and pinned by the added tests.
Testing
pnpm lintpnpm typecheckpnpm test— re-run after the rebase: 39,607 / 39,609 passing (3,747 of 3,748 files). The 2 failures are pre-existing and environmental — both insrc/relay/agent-exec-handler.test.ts, which fails identically on a cleanmaincheckout because an exportedGIT_ASKPASSin the local shell flows throughprocess.envinto the credential guard's expected spawn env. Nothing insrc/relayimports anything this PR touches. The suites that own every file this PR does touch (dashboard, dashboard-popout, sidebar, shared resolver) pass 1790/1790.pnpm buildAdded tests:
src/shared/agent-row-conversation-name.test.ts— a split tab drops the live title (including the OpenCode semantic form) while the same tab keeps it when unsplit;customTitle/quickCommandLabel/generatedTitlestill resolve in a split tab.src/renderer/src/components/dashboard/use-agent-row-conversation-name.test.ts— two rows on one tab with a split layout root both resolve tonull(each row then keeps its own label); the same tab with a leaf root still resolves the live title. The split test fails onmain.AI Review Report
Reviewed with Claude Opus 5 in Claude Code, including an adversarial pass on an earlier version of this PR that the review rejected — that version routed rows through
entry.terminalTitleon the assumption it was the pane's live OSC title. It is not: on the local pathentry.terminalTitleresolves tolayout.titlesByLeafId[leafId] ?? tab.title, wheretitlesByLeafIdholds user-assigned pane names (src/shared/types.ts), so for an ordinary split it collapses totab.titleand the fix was a no-op. It was reverted in this branch rather than shipped. What follows is the review of the change that remains.Correctness
layout.root.type === 'split'is structural: the layout root is a split node exactly when the tab has ≥2 leaves. No heuristic, no ephemeral id, no cross-map lookup.getCompactAgentPrimary→getAgentRowPrimaryText(agent.entry)→entry.prompt, which is stored perpaneKeyinagentStatusByPaneKey.getAgentRowConversationName; exactly two production call sites exist and both pass the flag.mobile/does not use this resolver.main— one conflict, one line:mainhad wrapped the same expression inboundedLabelOrUndefined(label bounding). Resolved by composing both rather than choosing, and the consumer sweep was re-run against the new base, which is where the board-search consumer above surfaced.liveTitlelocal; the three tab-owned short-circuits sit upstream of it and are pinned by a test.false, so any future caller behaves as it does today.removeLeafFromTreepromotes the surviving sibling, so closing one pane of a two-way split leaves a leaf root. No orphaned single-leaf split can strip a now-single-pane tab of its title.Cmd+Shift+Enter) — the case most likely to silently undo this fix, and it does not: expand only setsdisplay: none/flexinline styles, andgetLayoutChildNodesfilters by class rather than visibility, so the hidden sibling still serializes and the root stayssplit.row.tab.activationPaneKeyis set only for subagent child rows, which the builder skips and the hook nulls out, so the two ids agree on every path that reaches the resolver.useAppStorecalls precede thecannotOwnTabNameearly return, so no hook is called conditionally.Cross-platform (macOS / Linux / Windows) — explicitly checked. No shortcuts, accelerators, modifier keys, shortcut labels, path construction, shell invocation, or Electron platform APIs are touched; the change is a boolean and a string. The resolver's existing Windows and UNC path-rejection cases pass unchanged.
SSH / remote / local — no Git commands, no process or credential assumptions, no local-only paths. Identical for local, remote-server, and SSH worktrees, and for folder workspaces as well as git worktrees. Worth calling out: the earlier rejected approach behaved differently on local versus remote panes (main owns agent status locally, the renderer owns it for remote runtimes), which is what made it wrong. The layout root is the same on every host.
Agent, integration, and git-provider compatibility — provider-neutral. Claude, Codex, Gemini, Cursor, OpenCode and the rest all reach the same branch; OpenCode's semantic title is a live title and is suppressed with the others in a split tab, which is intentional and tested. No git-provider surface (GitHub, GitLab, others) is involved.
Performance — one added store selector per row returning a boolean primitive, so it cannot churn on identity and does not subscribe rows to title frames. The WeakMap-backed per-tab-array index from #9989 is untouched and its "one index build per immutable tab array" test still passes. The snapshot builder reuses the worktree-scoped layout map already in scope rather than re-reading the store.
UI quality — no token, primitive, or style change, so
docs/STYLEGUIDE.mdis not engaged and light/dark rendering is unaffected. Split rows go from "two identical names that swap on click" to two distinct per-pane labels. Under SSH latency the rows no longer depend on focus-driven title propagation at all.Security — no security-relevant surface. No new string reaches the UI; one existing source is suppressed. No command execution, path handling, auth, secrets, IPC channel, dependency, persisted state, or schema change. No follow-up needed.
Notes
tab.titleis persisted, so before this change both rows started out sharing whichever pane was focused at quit).🤖 Generated with Claude Code