Repository navigation
Reduce typing latency in large busy workspaces - #7863
Merged
Merged
Conversation
added 7 commits
July 13, 2026 20:07
Collaborator
Author
|
Handoff: pushed the 12 completed local performance commits through 449ec2e. The worktree has an unfinished merge of 27810b9 with nine unresolved paths; preserve it. Ghostty dependency manaflow-ai/ghostty#114 combines the 8 MiB per-surface scrollback cap with current absolute-scroll restoration. Confirmed remaining gap: Codex fire-and-forget hooks can fan out detached CLI plus 30-second watchdog trees under a slow socket; that bounded-delivery fix is not implemented in this PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/CmuxEventLogWriter.swift (1)
141-150: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winBatch unbuffered system calls into a single write.
FileHandle.write(contentsOf:)executes a direct, unbuffered write system call. Emitting hundreds of tiny individual frames in a tight loop imposes unnecessary I/O overhead. Consider coalescing the data chunks to execute a single bulk write, flushing only when crossing the file-size rotation boundary.⚡ Proposed fix to batch frames before writing
- for data in lines { - if currentSize + UInt64(data.count) > maxEventLogBytes { - try handle.close() - try rotate(fileManager: fileManager) - handle = try FileHandle(forWritingTo: eventLogURL) - currentSize = 0 - } - try handle.write(contentsOf: data) - currentSize += UInt64(data.count) - } + var batch = Data() + for data in lines { + if currentSize + UInt64(batch.count + data.count) > maxEventLogBytes { + if !batch.isEmpty { + try handle.write(contentsOf: batch) + currentSize += UInt64(batch.count) + batch.removeAll(keepingCapacity: true) + } + try handle.close() + try rotate(fileManager: fileManager) + handle = try FileHandle(forWritingTo: eventLogURL) + currentSize = 0 + } + batch.append(data) + } + if !batch.isEmpty { + try handle.write(contentsOf: batch) + currentSize += UInt64(batch.count) + }🤖 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/CmuxEventLogWriter.swift` around lines 141 - 150, Update the write loop in the event-log append flow to coalesce consecutive data chunks into a single buffer and perform one FileHandle.write(contentsOf:) call per batch. Flush the accumulated buffer before rotating when adding the next chunk would exceed maxEventLogBytes, then reset the batch and continue writing while preserving currentSize accounting and rotation behavior.
🤖 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/SharedLiveAgentIndex.swift`:
- Around line 27-40: Mark the pure static helper hookEventReloadInterval, along
with any static constants it reads if required by Swift isolation rules, as
nonisolated so non-MainActor callers can invoke it synchronously. Preserve its
current clamping and interval calculation behavior.
---
Outside diff comments:
In `@Sources/CmuxEventLogWriter.swift`:
- Around line 141-150: Update the write loop in the event-log append flow to
coalesce consecutive data chunks into a single buffer and perform one
FileHandle.write(contentsOf:) call per batch. Flush the accumulated buffer
before rotating when adding the next chunk would exceed maxEventLogBytes, then
reset the batch and continue writing while preserving currentSize accounting and
rotation behavior.
🪄 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: 49b98eca-f65e-4b10-af26-1598b1780b97
📒 Files selected for processing (35)
Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/Concurrency/AtomicBooleanGate.swiftPackages/macOS/CmuxFoundation/Sources/CmuxFoundationAtomicsC/CmuxFoundationAtomicsC.cPackages/macOS/CmuxFoundation/Sources/CmuxFoundationAtomicsC/include/CmuxFoundationAtomicsC.hPackages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/Concurrency/AtomicBooleanGateTests.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Lifecycle/TerminalSurfaceRuntimeTeardownCoordinator.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Lifecycle/TerminalSurfaceRuntimeTeardownRequest.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Runtime/TerminalScrollbackBudget.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalManualIOWrite.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface+Debug.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface+RuntimeLifecycle.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface+RuntimeSurfaceCreation.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface.swiftPackages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/TerminalScrollbackBudgetTests.swiftPackages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/TerminalSurfaceRuntimeTeardownCoordinatorTests.swiftPackages/macOS/CmuxTerminal/Tests/GhosttyRuntimeTestStubs/GhosttyRuntimeTestStubs.cPackages/macOS/CmuxTerminal/Tests/GhosttyRuntimeTestStubs/include/GhosttyRuntimeTestStubs.hPackages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/Interop/GhosttyRuntimeCInterop.swiftSources/CmuxEventBus.swiftSources/CmuxEventLogWriter.swiftSources/CmuxEventStream.swiftSources/CmuxTopProcessArguments.swiftSources/GhosttyTerminalView.swiftSources/Mobile/MobileHostService.swiftSources/Mobile/MobileTerminalByteTee.swiftSources/SharedLiveAgentIndex.swiftSources/SharedLiveAgentIndexLoader.swiftSources/VaultAgentProcessCandidateSelector.swiftSources/VaultAgentProcessScanner.swiftcmux.xcodeproj/project.pbxprojcmuxTests/CmuxEventBusTests.swiftcmuxTests/MobileHostAuthorizationTests.swiftcmuxTests/SharedLiveAgentIndexAgentLivenessTests.swiftcmuxTests/TerminalAndGhosttyTests.swiftcmuxTests/VaultAgentProcessCandidateSelectorTests.swiftghostty
Comment on lines
+27
to
+40
| private static let maxEventReloadInterval: TimeInterval = 30.0 | ||
| private static let liveAgentsPerReloadIntervalStep = 8 | ||
|
|
||
| static func hookEventReloadInterval(liveAgentCount: Int) -> TimeInterval { | ||
| let clampedAgentCount = max(0, liveAgentCount) | ||
| let intervalSteps = max( | ||
| 1, | ||
| (clampedAgentCount + liveAgentsPerReloadIntervalStep - 1) / liveAgentsPerReloadIntervalStep | ||
| ) | ||
| return min( | ||
| maxEventReloadInterval, | ||
| TimeInterval(intervalSteps) * minEventReloadInterval | ||
| ) | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Consider marking the new pure interval helper nonisolated.
hookEventReloadInterval only reads its own static constants and performs pure arithmetic — it doesn't touch any instance/actor state. Since Swift 5.10, static let/static func declared inside an @MainActor class are MainActor-isolated by default, so callers outside this actor (e.g. future non-MainActor callers, or unit tests that aren't themselves @MainActor) would need await for what is otherwise a trivial, thread-safe computation.
♻️ Suggested nonisolated annotations
- private static let minEventReloadInterval: TimeInterval = 5.0
- private static let maxEventReloadInterval: TimeInterval = 30.0
- private static let liveAgentsPerReloadIntervalStep = 8
+ private nonisolated static let minEventReloadInterval: TimeInterval = 5.0
+ private nonisolated static let maxEventReloadInterval: TimeInterval = 30.0
+ private nonisolated static let liveAgentsPerReloadIntervalStep = 8
- static func hookEventReloadInterval(liveAgentCount: Int) -> TimeInterval {
+ nonisolated static func hookEventReloadInterval(liveAgentCount: Int) -> TimeInterval {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private static let maxEventReloadInterval: TimeInterval = 30.0 | |
| private static let liveAgentsPerReloadIntervalStep = 8 | |
| static func hookEventReloadInterval(liveAgentCount: Int) -> TimeInterval { | |
| let clampedAgentCount = max(0, liveAgentCount) | |
| let intervalSteps = max( | |
| 1, | |
| (clampedAgentCount + liveAgentsPerReloadIntervalStep - 1) / liveAgentsPerReloadIntervalStep | |
| ) | |
| return min( | |
| maxEventReloadInterval, | |
| TimeInterval(intervalSteps) * minEventReloadInterval | |
| ) | |
| } | |
| private nonisolated static let minEventReloadInterval: TimeInterval = 5.0 | |
| private nonisolated static let maxEventReloadInterval: TimeInterval = 30.0 | |
| private nonisolated static let liveAgentsPerReloadIntervalStep = 8 | |
| nonisolated static func hookEventReloadInterval(liveAgentCount: Int) -> TimeInterval { | |
| let clampedAgentCount = max(0, liveAgentCount) | |
| let intervalSteps = max( | |
| 1, | |
| (clampedAgentCount + liveAgentsPerReloadIntervalStep - 1) / liveAgentsPerReloadIntervalStep | |
| ) | |
| return min( | |
| maxEventReloadInterval, | |
| TimeInterval(intervalSteps) * minEventReloadInterval | |
| ) | |
| } |
🤖 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/SharedLiveAgentIndex.swift` around lines 27 - 40, Mark the pure
static helper hookEventReloadInterval, along with any static constants it reads
if required by Swift isolation rules, as nonisolated so non-MainActor callers
can invoke it synchronously. Preserve its current clamping and interval
calculation behavior.
Source: Coding guidelines
added 2 commits
July 14, 2026 13:35
…enecks # Conflicts: # Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Debug/ControlCommandCoordinator+Debug.swift # Sources/GhosttyTerminalView.swift # Sources/Mobile/MobileWorkspaceListObserver.swift # Sources/PortScanner.swift # Sources/TabManager.swift # Sources/TerminalController.swift # cmux.xcodeproj/project.pbxproj # cmuxTests/PortScannerTests.swift # ghostty
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/Mobile/MobileHostService.swift`:
- Around line 1605-1608: Remove the redundant
debugHasTerminalOutputSubscribersForTesting() production wrapper; tests should
access MobileHostEventSubscriptionTracker.hasTerminalOutputSubscribers()
directly through `@testable` import, preserving the existing internal API.
🪄 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: 720c23d4-125c-4529-8da8-b8e24b9e4c01
📒 Files selected for processing (25)
Packages/Shared/CmuxAgentChat/Tests/CmuxAgentChatTests/Resources/ArtifactParity/codex-adversarial.jsonlPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/ControlCommandContext.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/ControlCommandCoordinator.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Debug/ControlCommandCoordinator+Debug.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Debug/ControlCommandCoordinator+DebugInteraction.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Debug/ControlDebugContext.swiftPackages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandContextTestStubs+Debug.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Navigation/CuratedSettingEntry+Default.swiftPackages/macOS/CmuxSettingsUI/Tests/CmuxSettingsUITests/SettingsRowAnchorResolutionTests.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface+RuntimeLifecycle.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface.swiftResources/Localizable.xcstringsSources/AppDelegate.swiftSources/CmuxTopProcessCPUTracker.swiftSources/CmuxTopProcessEnumeration.swiftSources/CmuxTopSnapshot.swiftSources/CmuxTopSnapshotScopeCache.swiftSources/ContentView.swiftSources/GhosttyTerminalView.swiftSources/Mobile/AgentChat/AgentChatSessionRegistry.swiftSources/Mobile/MobileHostService.swiftSources/Mobile/MobileWorkspaceListObserver.swiftSources/PortScanSnapshotStore.swiftSources/PortScanner+SnapshotProjection.swiftSources/PortScanner.swift
💤 Files with no reviewable changes (1)
- Resources/Localizable.xcstrings
Comment on lines
+1605
to
+1608
| nonisolated static func debugHasTerminalOutputSubscribersForTesting() -> Bool { | ||
| MobileHostEventSubscriptionTracker.hasTerminalOutputSubscribers() | ||
| } | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove redundant test-only seam.
As per path instructions, do not add test-only seams to production Swift source when the internal state can be reached directly via @testable import. hasTerminalOutputSubscribers() is already internal and is accessible to tests, so this debug wrapper is unnecessary.
♻️ Proposed fix
- nonisolated static func debugHasTerminalOutputSubscribersForTesting() -> Bool {
- MobileHostEventSubscriptionTracker.hasTerminalOutputSubscribers()
- }
-📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| nonisolated static func debugHasTerminalOutputSubscribersForTesting() -> Bool { | |
| MobileHostEventSubscriptionTracker.hasTerminalOutputSubscribers() | |
| } |
🤖 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/MobileHostService.swift` around lines 1605 - 1608, Remove the
redundant debugHasTerminalOutputSubscribersForTesting() production wrapper;
tests should access
MobileHostEventSubscriptionTracker.hasTerminalOutputSubscribers() directly
through `@testable` import, preserving the existing internal API.
Source: Path instructions
austinywang
added a commit
that referenced
this pull request
Jul 14, 2026
The rebase conflict resolution in the socket-verbs commit accidentally staged the stale local ghostty checkout (eb500e9), reverting main's pointer bump from #7863 (a630590) and breaking the app build on the new ghostty_renderer_event_e type. No cmux-side change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
3 of 6 tasks
austinywang
added a commit
that referenced
this pull request
Jul 15, 2026
This was referenced Jul 15, 2026
4 of 6 tasks
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Browser focus ownership is retained as correctness support for hidden browser view unmount/remount. It is not presented as a performance improvement.
Profile evidence
The one-minute Nightly typing capture contained 28 workspaces, 38 panes, and 55 surfaces:
scanAgentPortsoccupied 29,368 samples and repeatedly invoked process enumeration andlsofThese captures identify owners and justify candidate fixes. They do not yet prove end-to-end improvement.
Scope audit
Removed after independent review because the captures did not justify them:
report_pwdschedulingAn independent audit classified all 167 paths in the then-current net diff and found no remaining unrelated implementation. The six paths added afterward are isolated Git/PR owner diagnostics and regression tests required by independent review, bringing the current net diff to 173 paths. The branch history still shows the reverted experiments above because several commits mix retained compile fixes with later-trimmed hunks; the final verified net diff should be squash-merged rather than rewritten across those red/green test pairs.
Verification status
mainate634825742; candidate source is frozen at pushed heada9033b4014pending an exact Xcode 26.3 / macOS SDK 26.2 Release buildDraft blockers
This PR remains a draft until those gates pass.
Summary by CodeRabbit