Repository navigation
Fix title-churn beachball in transcript adoption and sidebar rows - #6460
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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:
📝 WalkthroughWalkthroughThis PR addresses two root causes of the main-thread beachball ( ChangesAgent Transcript Adoption Pipeline
Sidebar Active-State Scoping
Sequence DiagramsequenceDiagram
participant TitleObs as Title Observer
participant Service as AgentChatTranscriptService
participant TC as TerminalController
participant Detached as Task.detached
participant Registry as AgentChatSessionRegistry
TitleObs->>Service: title-change notification
Service->>Service: scheduleTitleDetectedAdoption(titleChange)
Note over Service: debounce per surface, cancel prior timer
Service->>Service: timer expires → flushTitleDetectedAdoption
Service->>TC: adoptDetectedAgentSession(titleChange:) → Bool
TC->>Service: adoptDetectedClaudeSession(workspaceID:surfaceID:...)
Service->>Registry: liveSession(surfaceID:)
Registry-->>Service: existing record or nil
Service->>Service: scheduleClaudeTranscriptResolution(key, surfaceID)
Note over Service: cancel prior task if key changed
Service->>Detached: async newestClaudeTranscript
Detached-->>Service: resolved transcript path or nil
Service->>Service: applyClaudeTranscriptResolution(guard: key current)
alt bound session exists, transcript unbound
Service->>Registry: update session transcript binding
else no bound session
Service->>Registry: adopt newly resolved session
Note over Service: re-schedule if session incompatible
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (3 errors, 1 warning)
✅ Passed checks (19 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@Sources/Mobile/AgentChat/AgentChatSessionRegistry.swift`:
- Around line 66-69: The liveSession(surfaceID:) method performs an O(n) scan of
all records on every call, which degrades performance at scale. Create a private
dictionary property that indexes live sessions by surfaceID (mapping surfaceID
strings to sessionID keys for sessions in non-ended state). Update this index
whenever the session state changes: in the update method when updating session
state, in adoptDetectedSession when adding new sessions, in noteHookEvent when
state transitions occur, and in sweepDeadProcesses after removing dead sessions.
Then refactor the liveSession method to use this index dictionary for O(1)
lookup instead of scanning records.values, ensuring the index stays in sync with
the actual live sessions.
In `@Sources/Mobile/AgentChat/AgentChatTranscriptService.swift`:
- Around line 285-293: The deliveredTitleKeys cache in the
scheduleTitleDetectedAdoption method is never cleared when Claude becomes
inactive on a surface, causing new generic Claude titles to be suppressed
indefinitely on reused surfaces. You need to clear the
deliveredTitleKeys[surfaceID] entry (and associated pending state in
pendingTitleChanges[surfaceID]) in two scenarios: first, when a non-Claude title
arrives for that surface in the scheduling logic, and second, when the bound
session ends or is deallocated. This ensures the cache is reset when the surface
context changes, allowing new Claude title detections to be properly processed.
- Around line 401-419: When an existing bound session is found for the surface
(in the first branch handling `if let bound = registry.liveSession`), the code
updates the bound session with the resolved transcript path but fails to claim
the `resolved.sessionID`. This violates the resolver's session-id exclusion
contract, allowing another surface to later adopt the same transcript. Fix this
by rekeying or merging the provisional record to use `resolved.sessionID`
instead of `bound.sessionID` before returning, ensuring the resolved session ID
is properly claimed and marked as used in the registry.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9379e3bd-81c0-4c4e-9ff0-bfe1415f66c9
📒 Files selected for processing (6)
Sources/AppDelegate.swiftSources/ContentView.swiftSources/Mobile/AgentChat/AgentChatSessionRegistry.swiftSources/Mobile/AgentChat/AgentChatTranscriptResolver.swiftSources/Mobile/AgentChat/AgentChatTranscriptService.swiftSources/TerminalController+MobileChat.swift
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
Sources/Mobile/AgentChat/AgentChatTranscriptService.swift (4)
537-540:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not let stale ended records clear a newer surface occupant’s state.
This cleanup is keyed only by
surfaceID. If an old session’s.endedupdate arrives after a new provisional Claude session has already been created on the same surface,clearTitleDetectionStatecancels the new session’s pending transcript resolution. Only clear when there is no current non-ended live session for that surface.Suggested patch direction
if stateChanged, record.state == .ended { - if let surfaceID = record.surfaceID { + if let surfaceID = record.surfaceID, + registry.liveSession(surfaceID: surfaceID)?.sessionID == nil { clearTitleDetectionState(surfaceID: surfaceID) }As per coding guidelines, per-surface transcript/session caches need stale-cache handling when titles/agents change.
🤖 Prompt for 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. In `@Sources/Mobile/AgentChat/AgentChatTranscriptService.swift` around lines 537 - 540, The clearTitleDetectionState call in the stateChanged and record.state == .ended check is keyed only by surfaceID without verifying if a newer active session already exists on that surface, causing stale ended records to incorrectly cancel newer provisional sessions. Before calling clearTitleDetectionState with the surfaceID, add a check to confirm there is no current non-ended live session for that surface ID, and only proceed with the cleanup if that condition is met. This ensures stale session cleanup respects active surface occupants that may have been created after the old session ended.Source: Coding guidelines
444-455:⚠️ Potential issue | 🟠 Major | ⚡ Quick winOnly claim a resolved transcript after it is actually bound.
Line 445 marks
resolved.sessionIDas claimed before checking whether the live surface already has a transcript. If that guard returns because another path bound a different transcript first, this result is never used but remains excluded from future detection. Move the claim to the branches that bind/adopt the resolved path, or only keep it when the existing bound path matchesresolved.path.Suggested patch direction
detectionScanAt.removeValue(forKey: surfaceID) -claimedDetectedTranscriptSessionIDs.insert(resolved.sessionID) if let bound = registry.liveSession(surfaceID: surfaceID) { - guard bound.transcriptPath == nil else { return } + if let existingPath = bound.transcriptPath { + if existingPath == resolved.path { + claimedDetectedTranscriptSessionIDs.insert(resolved.sessionID) + } + return + } + claimedDetectedTranscriptSessionIDs.insert(resolved.sessionID) registry.update(sessionID: bound.sessionID) { record in record.workspaceID = workspaceID record.surfaceID = surfaceID record.workingDirectory = workingDirectory record.transcriptPath = resolved.path @@ } +claimedDetectedTranscriptSessionIDs.insert(resolved.sessionID) registry.adoptDetectedSession(As per coding guidelines, cached transcript/session adoption state must not silently substitute stale or incorrect identity data.
Also applies to: 457-465
🤖 Prompt for 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. In `@Sources/Mobile/AgentChat/AgentChatTranscriptService.swift` around lines 444 - 455, The claimedDetectedTranscriptSessionIDs collection is marking resolved.sessionID as claimed on line 445 before verifying whether the transcript will actually be bound to the session. If the guard condition on the next line returns early due to an existing transcript path, the sessionID remains incorrectly marked as claimed even though the resolved path was never used. Move the insertion of resolved.sessionID into claimedDetectedTranscriptSessionIDs to only occur after confirming the resolved path will be bound to the session, specifically within the registry.update call block where the transcriptPath is actually being set, or add an additional condition to only claim when the resolved path matches an existing bound path.Source: Coding guidelines
344-350:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInclude the workspace/current binding in the resolution freshness guard.
Line 350 dedupes scans without
workspaceID, but the task still capturesworkspaceIDand later writes it back. If a surface is re-adopted or moved to another workspace while the scan is in flight and the working directory/title key are unchanged, the stale task can restore the old workspace binding. AddworkspaceIDtoClaudeTranscriptResolutionKeyand/or guard against the current live binding before applying.Suggested patch direction
private typealias ClaudeTranscriptResolutionKey = ( + workspaceID: String, workingDirectory: String, claimedSessionIDs: Set<String>, titleKey: String?, forceScan: Bool ) @@ let key: ClaudeTranscriptResolutionKey = ( + workspaceID: workspaceID, workingDirectory: workingDirectory, claimedSessionIDs: claimed, titleKey: Self.specificClaudeTitleKey(titleHint), forceScan: forceScan ) @@ guard transcriptResolutionKeys[surfaceID] == key else { return } +if let current = registry.liveSession(surfaceID: surfaceID), + current.workspaceID != workspaceID || current.workingDirectory != workingDirectory { + return +}As per coding guidelines, transcript/session adoption caches must handle stale-cache failures in correctness-sensitive flows.
Also applies to: 378-385, 448-452
🤖 Prompt for 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. In `@Sources/Mobile/AgentChat/AgentChatTranscriptService.swift` around lines 344 - 350, The ClaudeTranscriptResolutionKey used in the freshness guard does not include workspaceID, which can cause stale cache issues when a surface is re-adopted or moved to another workspace. Add workspaceID to the ClaudeTranscriptResolutionKey tuple definition (alongside workingDirectory, claimedSessionIDs, titleKey, and forceScan) to ensure the deduplication guard properly handles workspace changes. Additionally, add a guard check to verify the current live workspace binding matches before applying a cached resolution, preventing stale workspace bindings from being restored when the surface binding has changed.Source: Coding guidelines
611-641:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not force-scan on volatile Claude title text.
specificClaudeTitleKeycurrently uses the full normalized title for any✳-prefixed or Claude-containing title except a couple of generic forms. Spinner/token/elapsed-title churn can therefore become a newspecific:*key each update, makingisSpecificClaudeTitletrue and bypassingdetectionScanThrottle. Collapse volatile status titles to"generic:claude"unless a stable session/project identifier is extracted.Suggested patch direction
private static func specificClaudeTitleKey(_ title: String?) -> String? { guard var title = title?.trimmingCharacters(in: .whitespacesAndNewlines), !title.isEmpty else { return nil @@ } let normalized = title.lowercased() guard !normalized.isEmpty, + normalized.contains("claude"), normalized != "claude code", !normalized.hasPrefix("claude ·") else { return nil } + // Keep this path to stable identifiers only; volatile status/counter text + // should fall back to generic:claude so it stays throttled. return "specific:\(normalized)" }As per coding guidelines, title-change handlers must not introduce scalable transcript/directory rescans without a bounded size or benchmark.
🤖 Prompt for 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. In `@Sources/Mobile/AgentChat/AgentChatTranscriptService.swift` around lines 611 - 641, The specificClaudeTitleKey function is creating unique specific keys for volatile titles that change frequently (spinner characters, elapsed time, token counts), which bypasses the detectionScanThrottle mechanism. Modify specificClaudeTitleKey to return nil for titles that contain transient status indicators or unstable content, so they fall back to "generic:claude" in the claudeTitleDetectionKey method. Only return specific keys for titles that represent stable session/project identifiers. This prevents each title update from being treated as a new specific Claude title and triggering unnecessary rescans.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@Sources/Mobile/AgentChat/AgentChatTranscriptService.swift`:
- Around line 537-540: The clearTitleDetectionState call in the stateChanged and
record.state == .ended check is keyed only by surfaceID without verifying if a
newer active session already exists on that surface, causing stale ended records
to incorrectly cancel newer provisional sessions. Before calling
clearTitleDetectionState with the surfaceID, add a check to confirm there is no
current non-ended live session for that surface ID, and only proceed with the
cleanup if that condition is met. This ensures stale session cleanup respects
active surface occupants that may have been created after the old session ended.
- Around line 444-455: The claimedDetectedTranscriptSessionIDs collection is
marking resolved.sessionID as claimed on line 445 before verifying whether the
transcript will actually be bound to the session. If the guard condition on the
next line returns early due to an existing transcript path, the sessionID
remains incorrectly marked as claimed even though the resolved path was never
used. Move the insertion of resolved.sessionID into
claimedDetectedTranscriptSessionIDs to only occur after confirming the resolved
path will be bound to the session, specifically within the registry.update call
block where the transcriptPath is actually being set, or add an additional
condition to only claim when the resolved path matches an existing bound path.
- Around line 344-350: The ClaudeTranscriptResolutionKey used in the freshness
guard does not include workspaceID, which can cause stale cache issues when a
surface is re-adopted or moved to another workspace. Add workspaceID to the
ClaudeTranscriptResolutionKey tuple definition (alongside workingDirectory,
claimedSessionIDs, titleKey, and forceScan) to ensure the deduplication guard
properly handles workspace changes. Additionally, add a guard check to verify
the current live workspace binding matches before applying a cached resolution,
preventing stale workspace bindings from being restored when the surface binding
has changed.
- Around line 611-641: The specificClaudeTitleKey function is creating unique
specific keys for volatile titles that change frequently (spinner characters,
elapsed time, token counts), which bypasses the detectionScanThrottle mechanism.
Modify specificClaudeTitleKey to return nil for titles that contain transient
status indicators or unstable content, so they fall back to "generic:claude" in
the claudeTitleDetectionKey method. Only return specific keys for titles that
represent stable session/project identifiers. This prevents each title update
from being treated as a new specific Claude title and triggering unnecessary
rescans.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ad50ff25-39ca-4915-b7f7-d52dc51be58c
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (2)
Sources/Mobile/AgentChat/AgentChatTranscriptResolver.swiftSources/Mobile/AgentChat/AgentChatTranscriptService.swift
💤 Files with no reviewable changes (1)
- Sources/Mobile/AgentChat/AgentChatTranscriptResolver.swift
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/Mobile/AgentChat/AgentChatTranscriptService.swift (1)
143-153:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDon’t let a stale transcript-bound detected session suppress later Claude launches.
A hookless title-detected session can stay
.idlewithtranscriptPathafter the terminal title moves away from Claude. If the same surface later starts a new hookless Claude, this branch returns before scheduling resolution, so the old transcript remains bound. Please invalidate/end the detected live session on a non-Claude transition, or gate this short-circuit on a still-current title/session generation.As per coding guidelines, cached session-surface identity needs invalidation or rescan triggers when it can become stale.
🤖 Prompt for 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. In `@Sources/Mobile/AgentChat/AgentChatTranscriptService.swift` around lines 143 - 153, The guard statement that checks bound.transcriptPath == nil allows stale title-detected sessions to block new Claude launches on the same surface. Either invalidate the bound session when a non-Claude terminal title transition is detected (before returning true), or add a validation check before the guard statement to confirm the session is still current by verifying its title generation or session generation matches the current state, ensuring stale transcript-bound sessions do not suppress new Claude launches.Source: Coding guidelines
🤖 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 `@Sources/Mobile/AgentChat/AgentChatTranscriptService`+TitleDetection.swift:
- Around line 169-190: The claimedDetectedTranscriptSessionIDs.insert call is
happening before confirming that the transcript will actually be bound to a
session, which means if the guard bound.transcriptPath == nil check fails and we
return early, the sessionID is already marked as claimed and will be excluded
from future scans even though it was never actually bound. Move the insert
statement to occur only after successful binding: add
claimedDetectedTranscriptSessionIDs.insert(resolved.sessionID) after the
registry.update call completes within the if let bound block, and also add it
after the registry.adoptDetectedSession call to handle both the update and
adoption scenarios.
---
Outside diff comments:
In `@Sources/Mobile/AgentChat/AgentChatTranscriptService.swift`:
- Around line 143-153: The guard statement that checks bound.transcriptPath ==
nil allows stale title-detected sessions to block new Claude launches on the
same surface. Either invalidate the bound session when a non-Claude terminal
title transition is detected (before returning true), or add a validation check
before the guard statement to confirm the session is still current by verifying
its title generation or session generation matches the current state, ensuring
stale transcript-bound sessions do not suppress new Claude launches.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: dc3ead02-7426-4ad8-aecb-d0049fc774d5
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (4)
Sources/Mobile/AgentChat/AgentChatTranscriptResolver.swiftSources/Mobile/AgentChat/AgentChatTranscriptService+TitleDetection.swiftSources/Mobile/AgentChat/AgentChatTranscriptService.swiftcmux.xcodeproj/project.pbxproj
💤 Files with no reviewable changes (1)
- Sources/Mobile/AgentChat/AgentChatTranscriptResolver.swift
Greptile SummaryThis PR fixes the main-thread beachball caused by Claude terminal title churn. It debounces per-surface title-change adoption through the existing burst coalescer, moves transcript discovery off the main actor into a cancellable detached utility task, and isolates sidebar row selection state into each row so unchanged rows stay behind
Confidence Score: 5/5Safe to merge — changes are scoped to the title-change adoption path and sidebar row rendering, with no new data-loss or correctness regressions found. The transcript-resolution path correctly guards stale results via key equality before applying, forced retries are capped at 3 per surface, and the registry's live-session index is updated incrementally so the hot pre/postToolUse hook path avoids any O(N) scan. The sidebar selection fix uses a well-established row-local publisher projection and the equatable contract remains consistent with the new state model. No files require special attention. The previously noted internal-visibility widening across the AgentChatTranscriptService split is the main design tradeoff but is inherent to Swift's cross-file extension model and was acknowledged in prior review rounds. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant GTC as GhosttyTitleChange
participant ATS as AgentChatTranscriptService (MainActor)
participant BC as BurstCoalescer (250ms)
participant TC as TerminalController (MainActor)
participant DT as Task.detached (utility)
participant MA as MainActor (apply result)
GTC->>ATS: scheduleTitleDetectedAdoption(change)
ATS->>ATS: check pendingTitleChanges and deliveredTitleKeys
ATS->>BC: signal flushTitleDetectedAdoptions
Note over BC: coalesces bursts for 250ms
BC->>ATS: flushTitleDetectedAdoptions
ATS->>TC: titleAdoptionHandler(pending.change)
TC->>ATS: adoptDetectedClaudeSession
ATS->>ATS: scheduleClaudeTranscriptResolution
ATS->>DT: Task.detached utility scan
Note over DT: filesystem scan off main actor
DT-->>MA: await scanTask.value
MA->>ATS: applyClaudeTranscriptResolution
ATS->>ATS: guard key still matches
ATS->>ATS: registry.update or adoptDetectedSession
%%{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"}}}%%
sequenceDiagram
participant GTC as GhosttyTitleChange
participant ATS as AgentChatTranscriptService (MainActor)
participant BC as BurstCoalescer (250ms)
participant TC as TerminalController (MainActor)
participant DT as Task.detached (utility)
participant MA as MainActor (apply result)
GTC->>ATS: scheduleTitleDetectedAdoption(change)
ATS->>ATS: check pendingTitleChanges and deliveredTitleKeys
ATS->>BC: signal flushTitleDetectedAdoptions
Note over BC: coalesces bursts for 250ms
BC->>ATS: flushTitleDetectedAdoptions
ATS->>TC: titleAdoptionHandler(pending.change)
TC->>ATS: adoptDetectedClaudeSession
ATS->>ATS: scheduleClaudeTranscriptResolution
ATS->>DT: Task.detached utility scan
Note over DT: filesystem scan off main actor
DT-->>MA: await scanTask.value
MA->>ATS: applyClaudeTranscriptResolution
ATS->>ATS: guard key still matches
ATS->>ATS: registry.update or adoptDetectedSession
Reviews (3): Last reviewed commit: "fix: retain detected transcript claims w..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| private func refreshLiveSessionIndex(surfaceID: String?) { | ||
| guard let surfaceID else { return } | ||
| if let newest = records.values | ||
| .filter({ $0.surfaceID == surfaceID && $0.state != .ended }) | ||
| .max(by: { $0.lastActivityAt < $1.lastActivityAt }) { | ||
| liveSessionIDBySurfaceID[surfaceID] = newest.sessionID | ||
| } else { | ||
| liveSessionIDBySurfaceID.removeValue(forKey: surfaceID) | ||
| } | ||
| } |
There was a problem hiding this comment.
refreshLiveSessionIndex filters and reduces over records.values (all sessions, including historical ended ones) to find the newest live session for a surface. It is called twice per update() and noteHookEvent() (once for previous.surfaceID, once for record.surfaceID). Before this PR, noteHookEvent had no per-event scan overhead; now every pre/postToolUse hook event during a Claude tool storm pays O(N) twice. Sessions are never removed from records, so N grows over the app's lifetime. The cache is valuable — but maintaining it via a full scan on every mutation, rather than an incremental update (e.g., only comparing the mutated record against the current cached winner), adds work on the write-hot path to avoid it on the read path. A tighter maintenance strategy — re-evaluate only when previous.surfaceID == record.surfaceID and the mutated record is the cached one or newer — would keep the O(1) read without the per-hook scan.
| guard !claimedDetectedTranscriptSessionIDs.contains(resolved.sessionID) else { | ||
| scheduleClaudeTranscriptResolution( | ||
| workspaceID: workspaceID, | ||
| workingDirectory: workingDirectory, | ||
| surfaceID: surfaceID, | ||
| excludingSessionID: registry.liveSession(surfaceID: surfaceID)?.sessionID, | ||
| titleHint: titleHint, | ||
| forceScan: true | ||
| ) | ||
| return | ||
| } | ||
| if let claimed = registry.record(sessionID: resolved.sessionID), | ||
| claimed.surfaceID != nil, | ||
| claimed.surfaceID != surfaceID { | ||
| scheduleClaudeTranscriptResolution( | ||
| workspaceID: workspaceID, | ||
| workingDirectory: workingDirectory, | ||
| surfaceID: surfaceID, | ||
| excludingSessionID: registry.liveSession(surfaceID: surfaceID)?.sessionID, | ||
| titleHint: titleHint, | ||
| forceScan: true | ||
| ) | ||
| return |
There was a problem hiding this comment.
forceScan: true retries bypass the throttle without a per-surface retry cap
When applyClaudeTranscriptResolution finds that the resolved transcript is already in claimedDetectedTranscriptSessionIDs or bound to a different surface, it re-invokes scheduleClaudeTranscriptResolution with forceScan: true. forceScan: true bypasses the 4-second detectionScanThrottle check entirely. The key equality guard (transcriptResolutionKeys[surfaceID]) and the growing claimed set bound the total retries to the number of claimable transcripts in the working directory, but two surfaces simultaneously racing to resolve transcripts from the same dense project directory (e.g., a shared monorepo) can trigger several back-to-back forced scans before settling. There is no ceiling on how many forced rescans a single applyClaudeTranscriptResolution chain can spawn for a surface. Adding a per-surface forced-retry counter (reset on clearTitleDetectionState) and capping it at, say, 3 would prevent pathological multi-surface collision storms from generating unbounded disk I/O.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/Mobile/AgentChat/AgentChatTranscriptService.swift (1)
447-452:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep Claude Code chrome titles on the generic key.
Line 452 makes titles like
Claude Code · …/Claude Code (…)into uniquespecific:keys because line 449 only excludesclaude ·. Those status/chrome titles then useforceScan: truein the unresolved bound path, bypassing the 4s throttle and rescanning the scalable.claude/projects/<cwd>transcript collection per churned key. Treatclaude codestatus prefixes as generic instead.Proposed fix
let normalized = title.lowercased() + let isClaudeCodeChrome = normalized == "claude code" + || normalized.hasPrefix("claude code ·") + || normalized.hasPrefix("claude code (") + || normalized.hasPrefix("claude code -") + || normalized.hasPrefix("claude code —") guard !normalized.isEmpty, - normalized != "claude code", + !isClaudeCodeChrome, !normalized.hasPrefix("claude ·") else { return nil }As per coding guidelines, production event-driven paths over scalable user data should avoid repeated scans and prefer bounded/coalesced work; this path can rescan transcript files for every churned title key.
🤖 Prompt for 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. In `@Sources/Mobile/AgentChat/AgentChatTranscriptService.swift` around lines 447 - 452, The guard statement in this function needs to treat Claude Code chrome titles as generic instead of assigning them unique specific keys. Currently, the check on line 449 only excludes the exact match "claude code", and the prefix check on line 450 only excludes "claude ·". This causes titles like "Claude Code · something" or "Claude Code (something)" to fall through and receive unique "specific:" keys, bypassing throttling and causing repeated scans. Update the guard statement to also check that the normalized title does not start with "claude code " (with a space after "code") as a prefix, treating all Claude Code status titles as generic keys that should return nil.Source: Coding guidelines
🤖 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 `@Sources/Mobile/AgentChat/AgentChatSessionRegistry.swift`:
- Around line 75-77: After ending a dead PID's session in the liveSession
method, the code returns a replacement record from the index without verifying
that the replacement's process is alive. If multiple records exist for the same
surface with dead processes, a dead session can be reported as live. After
retrieving the replacement record via
liveSessionIDBySurfaceID[surfaceID].flatMap, recursively call
liveSession(surfaceID:) instead of directly returning the record to ensure the
replacement is also validated and any subsequent dead processes are properly
handled.
---
Outside diff comments:
In `@Sources/Mobile/AgentChat/AgentChatTranscriptService.swift`:
- Around line 447-452: The guard statement in this function needs to treat
Claude Code chrome titles as generic instead of assigning them unique specific
keys. Currently, the check on line 449 only excludes the exact match "claude
code", and the prefix check on line 450 only excludes "claude ·". This causes
titles like "Claude Code · something" or "Claude Code (something)" to fall
through and receive unique "specific:" keys, bypassing throttling and causing
repeated scans. Update the guard statement to also check that the normalized
title does not start with "claude code " (with a space after "code") as a
prefix, treating all Claude Code status titles as generic keys that should
return nil.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8936c72f-feb3-49a5-9dbf-821a80b52b42
📒 Files selected for processing (4)
Sources/Mobile/AgentChat/AgentChatSessionRegistry.swiftSources/Mobile/AgentChat/AgentChatTranscriptResolver.swiftSources/Mobile/AgentChat/AgentChatTranscriptService+TitleDetection.swiftSources/Mobile/AgentChat/AgentChatTranscriptService.swift
This branch was successfully deployed
1 participant
Summary
Closes #6459
Closes #5392
This fixes the main-thread beachball caused by terminal title churn feeding two expensive main-thread paths:
AgentChatTranscriptService.observeAgentTitleChangessynchronously routed every Claude title update through workspace-wide adoption, andadoptDetectedClaudeSessioncould scan the Claude project directory and parse the newest transcript on the main actor.VerticalTabsSidebarpassed global selected-workspace state through the row builder, so focus/selection changes made the wholeForEachdepend on the same AttributeGraph selection node instead of letting unchanged rows stay behindTabItemView.equatable().Changes
TerminalController.adoptDetectedAgentSession(titleChange:)path instead of scanning every terminal in the workspace.adoptDetectedClaudeSessionmain-actor work to cheap registry updates and scheduling. Claude transcript discovery/parsing now runs in a cancellable detached utility task, then hops back to the main actor only to apply a still-current result.TabItemViewvia a row-localselectedTabIdPublisherboolean projection. The parent no longer passesisActive, so unchanged rows keep the same equatable inputs when selection changes.Testability / verification
This is a main-thread timing/perf bug, so I did not fabricate a unit test. The meaningful verification is the user-controlled dev build +
arch -arm64 lldbrepro from #6459: split a workspace, rapidly switch pane focus, and confirm the 140% CPU spike andModifiedViewList.applyNodes/ recursiveswift_releaseteardown are gone.Per request, I did not run a local build,
reload.sh,xcodebuild, or local tests. CI is the verification bar for compilation/checks on this branch.Localization
No user-facing strings were added or changed.
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Fixes the main-thread beachball from Claude title churn and stops sidebar rows from redrawing on selection changes. Moves to per-surface, coalesced title handling and resolves transcripts off the main thread with safer live-session indexing and claim management.
Bug Fixes
TerminalController.adoptDetectedAgentSession(titleChange:), avoiding workspace-wide sweeps; cleared per-surface state and canceled scans on title changes or session end.$HOME; cancellable.selectedTabIdPublisherso unchanged rows remainEquatableand don’t redraw on selection changes.Refactors
AgentChatTranscriptService+TitleDetection.swift; madeAgentChatTranscriptResolverSendable.Written for commit e906242. Summary will update on new commits.
Summary by CodeRabbit