Repository navigation
Fix idle CPU usage from animation timeline and git polling - #2561
austinywang wants to merge 6 commits into
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughController now installs a persistent NSHostingView root and drives overlay visibility from a model callback; overlay model flash lifecycle was rewritten to use token-guarded state and a cancellable delayed-reset work item; TabManager polling cadence and immediate selected-workspace refresh scheduling were changed and exposed for testing. Changes
Sequence Diagram(s)sequenceDiagram
participant Controller as "WindowTmuxWorkspacePaneOverlayController"
participant Model as "TmuxWorkspacePaneOverlayModel"
participant Hosting as "NSHostingView / SwiftUI View"
participant Scheduler as "DispatchQueue (reset workItem)"
Controller->>Model: update(state:)
Model-->>Controller: onStateChange()
Controller->>Hosting: renderModelState() -> set hostingView.rootView
alt Model.hasVisibleContent
Controller->>Controller: update containerView.alpha/isHidden
Hosting->>Hosting: renderOverlay(in: currentDate)
else
Controller->>Controller: hide containerView
end
Note over Model,Scheduler: when flash starts -> schedule delayed reset
Model->>Scheduler: dispatch workItem (captures token/workspace)
Scheduler-->>Model: workItem executes if token/workspace match
Model-->>Controller: onStateChange() (flash cleared)
Controller->>Hosting: renderModelState() updates view
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Greptile SummaryThis PR fixes two sources of idle CPU/GPU usage: Confidence Score: 5/5Safe to merge — no logic, data integrity, or runtime correctness issues found; remaining findings are style and concurrency-annotation suggestions. All findings are P2: a missing MainActor.assumeIsolated wrapper (runtime-safe, compiler hygiene only) and multiple intermediate Combine emissions on sequential @published mutations (sub-frame, visually harmless). Flash-reset timing logic, workspace-change guards, and git polling deduplication all look correct. Sources/WorkspaceContentView.swift — scheduleFlashReset DispatchWorkItem should use MainActor.assumeIsolated for strict concurrency hygiene. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[apply state called] --> B{didChangeWorkspace?}
B -- yes --> C[Set unreadRects / flashRect / flashReason\nCancelFlashReset\nflashStartedAt = nil\nReturn early]
B -- no --> D{flashRect == nil?}
D -- yes --> E[CancelFlashReset\nflashStartedAt = nil]
D -- no --> F{flashToken changed?}
E --> G[Update lastFlashToken]
F -- yes --> H[flashStartedAt = now\nscheduleFlashReset after 0.9s]
F -- no --> G
H --> G
G --> I[CombineLatest3 emits]
C --> I
I --> J{hasVisibleContent?}
J -- yes --> K[containerView shown]
J -- no --> L[containerView hidden]
K --> M{showsAnimatedFlash?}
M -- yes --> N[TimelineView animation]
M -- no --> O{unreadRects non-empty?}
O -- yes --> P[Static Canvas]
O -- no --> Q[Color.clear]
H --> R[DispatchWorkItem after 0.9s]
R --> S{workspaceId and flashToken match?}
S -- yes --> T[flashStartedAt = nil]
S -- no --> U[No-op]
Reviews (1): Last reviewed commit: "wip" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| let workItem = DispatchWorkItem { [weak self] in | ||
| guard let self else { return } | ||
| guard self.lastWorkspaceId == workspaceId, | ||
| self.lastFlashToken == flashToken else { | ||
| return | ||
| } | ||
|
|
||
| self.flashStartedAt = nil | ||
| } | ||
| flashResetWorkItem = workItem | ||
| DispatchQueue.main.asyncAfter( | ||
| deadline: .now() + FocusFlashPattern.duration, | ||
| execute: workItem | ||
| ) |
There was a problem hiding this comment.
@MainActor-isolated property access in @Sendable closure
DispatchWorkItem takes a @Sendable closure, so Swift Concurrency's type checker sees access to self.lastWorkspaceId, self.lastFlashToken, and self.flashStartedAt (all @MainActor-isolated) as potential data races. At runtime this is safe because the work item is dispatched to DispatchQueue.main, but the compiler cannot verify the equivalence between DispatchQueue.main and @MainActor in a @Sendable closure without an explicit annotation.
Wrapping the body with MainActor.assumeIsolated documents the intent and avoids warnings under -strict-concurrency=complete:
| let workItem = DispatchWorkItem { [weak self] in | |
| guard let self else { return } | |
| guard self.lastWorkspaceId == workspaceId, | |
| self.lastFlashToken == flashToken else { | |
| return | |
| } | |
| self.flashStartedAt = nil | |
| } | |
| flashResetWorkItem = workItem | |
| DispatchQueue.main.asyncAfter( | |
| deadline: .now() + FocusFlashPattern.duration, | |
| execute: workItem | |
| ) | |
| let workItem = DispatchWorkItem { [weak self] in | |
| MainActor.assumeIsolated { | |
| guard let self else { return } | |
| guard self.lastWorkspaceId == workspaceId, | |
| self.lastFlashToken == flashToken else { | |
| return | |
| } | |
| self.flashStartedAt = nil | |
| } | |
| } |
| modelVisibilityCancellable = Publishers.CombineLatest3( | ||
| model.$unreadRects, | ||
| model.$flashRect, | ||
| model.$flashStartedAt | ||
| ) | ||
| .receive(on: RunLoop.main) | ||
| .sink { [weak self] unreadRects, flashRect, flashStartedAt in | ||
| guard let self else { return } | ||
| let hasVisibleContent = !unreadRects.isEmpty || (flashRect != nil && flashStartedAt != nil) | ||
| self.containerView.alphaValue = hasVisibleContent ? 1 : 0 | ||
| self.containerView.isHidden = !hasVisibleContent | ||
| } |
There was a problem hiding this comment.
CombineLatest3 emissions from sequential @Published mutations
apply() mutates unreadRects, flashRect, flashReason, and flashStartedAt in sequence. Each assignment fires a separate @Published event, so CombineLatest3 emits up to four times before settling on the final state. With receive(on: RunLoop.main), each event is queued as a separate RunLoop callback and the sink runs once per event — meaning containerView.isHidden and containerView.alphaValue can be toggled multiple times within the same logical update.
This is visually harmless today (AppKit defers compositing), but it produces unnecessary AppKit property churn on every overlay update. Coalescing the sink with .removeDuplicates() would eliminate the intermediate states without changing any externally observable behaviour.
![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
🧹 Nitpick comments (3)
Sources/TabManager.swift (1)
1030-1051: Consider a low-frequency fallback probe for non-open cached PR states.With this filter, background polling won’t re-check panels currently marked
merged/closed, so remote reopen transitions can stay stale until explicit selection/manual triggers. A very low-rate fallback probe (e.g., sampled focused panel) would preserve the CPU win while improving eventual consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/TabManager.swift` around lines 1030 - 1051, The current trackedWorkspaceGitMetadataPeriodicPollCandidatePanelIds function excludes panels whose pullRequest.status != .open so closed/merged PRs never get background rechecked; change the selection logic to include a very-low-frequency fallback (e.g., always include workspace.focusedPanelId even if its pullRequest.status != .open, or probabilistically sample a small subset of non-open panels from workspace.panelPullRequests) before building the filtered Set; ensure you still respect activeProbeKeys by constructing WorkspaceGitProbeKey(workspaceId: workspace.id, panelId: panelId) and skipping probes already in activeProbeKeys, but allow the fallback panel(s) through so reopen transitions are eventually detected.Sources/WorkspaceContentView.swift (1)
127-129: Skip no-op@Publishedwrites inapply(_:).Now that the overlay host is driven directly from this model, assigning the same
unreadRects/flashRect/flashReasonstill emits and forces another visibility/body pass. Cheap equality guards here would trim a bit more idle churn.♻️ Proposed change
- unreadRects = state.unreadRects - flashRect = state.flashRect - flashReason = state.flashReason + if unreadRects != state.unreadRects { + unreadRects = state.unreadRects + } + if flashRect != state.flashRect { + flashRect = state.flashRect + } + if flashReason != state.flashReason { + flashReason = state.flashReason + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/WorkspaceContentView.swift` around lines 127 - 129, The apply(_:) path currently assigns unreadRects, flashRect and flashReason unconditionally which triggers redundant `@Published` emissions; change apply(_:) to compare incoming state values to the current properties (unreadRects, flashRect, flashReason) and only assign when the new value is != the existing value so you avoid no-op publishes and extra SwiftUI updates. Locate the apply(_:) implementation (and the properties unreadRects, flashRect, flashReason) and add cheap equality guards per property (or a combined equality check) to skip assignments and early-return when nothing changed.Sources/ContentView.swift (1)
1337-1348: Keep overlay visibility as a single model-owned rule.This re-derives visibility from
unreadRects/flashRect/flashStartedAtinside the controller. Now that the view is driven byTmuxWorkspacePaneOverlayModel, it would be safer to expose one visibility contract from the model and consume that here; otherwise the container hiding logic can drift from the view’s render logic on a future change.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ContentView.swift` around lines 1337 - 1348, The controller currently recomputes visibility from model.$unreadRects, model.$flashRect and model.$flashStartedAt inside modelVisibilityCancellable; instead, add a single visibility contract on TmuxWorkspacePaneOverlayModel (for example a published property like isVisible or a visibilityPublisher) that encapsulates the logic, and update this sink to subscribe only to that model property (e.g., model.$isVisible or model.visibilityPublisher) and set containerView.alphaValue/isHidden based on that single boolean; update TmuxWorkspacePaneOverlayModel to compute and publish visibility from unreadRects/flashRect/flashStartedAt so the view and controller share one source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/WorkspaceContentView.swift`:
- Around line 140-143: The code currently cancels the flash timer and clears
flashStartedAt whenever state.flashRect is nil; instead only cancel/reset when
the flash is truly finished or the token changed. Update the condition around
cancelFlashReset()/flashStartedAt = nil to check the flash token: only cancel if
state.flashRect == nil AND (state.flashToken == nil OR state.flashToken !=
flashToken) so transient nil rects that keep the same state.flashToken don't
stop the in-flight flash. Ensure you reference state.flashRect,
state.flashToken, flashToken, flashStartedAt, and cancelFlashReset() when making
the change.
---
Nitpick comments:
In `@Sources/ContentView.swift`:
- Around line 1337-1348: The controller currently recomputes visibility from
model.$unreadRects, model.$flashRect and model.$flashStartedAt inside
modelVisibilityCancellable; instead, add a single visibility contract on
TmuxWorkspacePaneOverlayModel (for example a published property like isVisible
or a visibilityPublisher) that encapsulates the logic, and update this sink to
subscribe only to that model property (e.g., model.$isVisible or
model.visibilityPublisher) and set containerView.alphaValue/isHidden based on
that single boolean; update TmuxWorkspacePaneOverlayModel to compute and publish
visibility from unreadRects/flashRect/flashStartedAt so the view and controller
share one source of truth.
In `@Sources/TabManager.swift`:
- Around line 1030-1051: The current
trackedWorkspaceGitMetadataPeriodicPollCandidatePanelIds function excludes
panels whose pullRequest.status != .open so closed/merged PRs never get
background rechecked; change the selection logic to include a very-low-frequency
fallback (e.g., always include workspace.focusedPanelId even if its
pullRequest.status != .open, or probabilistically sample a small subset of
non-open panels from workspace.panelPullRequests) before building the filtered
Set; ensure you still respect activeProbeKeys by constructing
WorkspaceGitProbeKey(workspaceId: workspace.id, panelId: panelId) and skipping
probes already in activeProbeKeys, but allow the fallback panel(s) through so
reopen transitions are eventually detected.
In `@Sources/WorkspaceContentView.swift`:
- Around line 127-129: The apply(_:) path currently assigns unreadRects,
flashRect and flashReason unconditionally which triggers redundant `@Published`
emissions; change apply(_:) to compare incoming state values to the current
properties (unreadRects, flashRect, flashReason) and only assign when the new
value is != the existing value so you avoid no-op publishes and extra SwiftUI
updates. Locate the apply(_:) implementation (and the properties unreadRects,
flashRect, flashReason) and add cheap equality guards per property (or a
combined equality check) to skip assignments and early-return when nothing
changed.
🪄 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: 3a2895a4-1de6-4c39-bc37-d9048b9c1305
📒 Files selected for processing (3)
Sources/ContentView.swiftSources/TabManager.swiftSources/WorkspaceContentView.swift
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Sources/ContentView.swift (1)
1377-1389: Avoid unconditional overlay rerenders here.Line 1377 still calls
renderModelState()on everyupdate(state:), and Line 1382 replaceshostingView.rootVieweven when the effective overlay state is unchanged. Since this controller is refreshed fromWindowAccessor, that keeps overlay work on the parent render path and chips away at the idle-CPU win this PR is targeting. LetonStateChangeown rerenders, or gaterenderModelState()behind a cheap state/signature check.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ContentView.swift` around lines 1377 - 1389, renderModelState() is being called unconditionally from update(state:) and always replaces hostingView.rootView causing unnecessary overlay rerenders; change this so either (a) move the render responsibility to onStateChange and stop calling renderModelState() from update(state:), or (b) add a cheap signature/state check in renderModelState() (compare a small snapshot/key derived from model.unreadRects, model.flashRect, model.flashStartedAt, model.flashReason, and model.hasVisibleContent) and return early when unchanged before setting hostingView.rootView or updating containerView; reference the renderModelState(), update(state:), onStateChange, hostingView.rootView and model.* fields to locate and gate the update.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Sources/ContentView.swift`:
- Around line 1377-1389: renderModelState() is being called unconditionally from
update(state:) and always replaces hostingView.rootView causing unnecessary
overlay rerenders; change this so either (a) move the render responsibility to
onStateChange and stop calling renderModelState() from update(state:), or (b)
add a cheap signature/state check in renderModelState() (compare a small
snapshot/key derived from model.unreadRects, model.flashRect,
model.flashStartedAt, model.flashReason, and model.hasVisibleContent) and return
early when unchanged before setting hostingView.rootView or updating
containerView; reference the renderModelState(), update(state:), onStateChange,
hostingView.rootView and model.* fields to locate and gate the update.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6a37d53c-0038-42dc-abd4-4e67e133bf3b
📒 Files selected for processing (3)
Sources/ContentView.swiftSources/WorkspaceContentView.swiftcmuxTests/WindowAndDragTests.swift
There was a problem hiding this comment.
🧹 Nitpick comments (2)
cmuxTests/TabManagerUnitTests.swift (1)
575-582: Tighten the assertion so this test validates retry preservation directly.At Line 576, the test only checks eventual PR success. Since the stubbed
ghsucceeds on the third global call, this can pass without proving retry state behavior. Consider asserting probe-attempt progression/count for this workspace/panel via the testing probe counter API, then asserting PR resolution.Suggested assertion upgrade
XCTAssertTrue( waitForCondition(timeout: 3.0) { workspace.panelPullRequests[panelId]?.number == 42 } ) + XCTAssertGreaterThanOrEqual( + manager.workspaceGitProbeAttemptCountForTesting(workspaceId: workspace.id, panelId: panelId), + 3, + "Expected retries to be preserved until the probe succeeds" + ) XCTAssertEqual(workspace.panelGitBranches[panelId]?.branch, "feature/bootstrap-refresh") XCTAssertEqual(workspace.pullRequest?.number, 42)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmuxTests/TabManagerUnitTests.swift` around lines 575 - 582, The test currently only verifies eventual PR success via waitForCondition and doesn't assert retry/probe progression; update the test to first read the testing probe counter API for this workspace/panel (use the probe counter for workspace and panelId) and assert that the probe attempt count increments as retries occur (e.g., becomes 1 then 2 before the final success), then keep the existing waitForCondition/assertions on workspace.panelPullRequests[panelId]?.number and workspace.pullRequest?.number == 42 to verify final resolution. Ensure you reference the same panelId and the probe counter API used by other tests so the assertions validate retry preservation directly.Sources/TabManager.swift (1)
1018-1021: Consider narrowing test-only API exposure.
workspaceGitProbeAttemptCountForTesting(...)is useful, but you may want to hide it from non-test builds to keep production surface area lean.Optional patch
+#if DEBUG func workspaceGitProbeAttemptCountForTesting(workspaceId: UUID, panelId: UUID) -> Int { let probeKey = WorkspaceGitProbeKey(workspaceId: workspaceId, panelId: panelId) return workspaceGitProbeTimersByKey[probeKey]?.count ?? 0 } +#endif🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/TabManager.swift` around lines 1018 - 1021, The method workspaceGitProbeAttemptCountForTesting(workspaceId:panelId:) exposes a test-only API in production; wrap its declaration in a conditional compilation block (e.g. `#if` canImport(XCTest) || DEBUG ... `#endif`) so it is only compiled for test/debug builds, leaving workspaceGitProbeTimersByKey and WorkspaceGitProbeKey unchanged and ensuring the function remains available to tests but not in release builds.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cmuxTests/TabManagerUnitTests.swift`:
- Around line 575-582: The test currently only verifies eventual PR success via
waitForCondition and doesn't assert retry/probe progression; update the test to
first read the testing probe counter API for this workspace/panel (use the probe
counter for workspace and panelId) and assert that the probe attempt count
increments as retries occur (e.g., becomes 1 then 2 before the final success),
then keep the existing waitForCondition/assertions on
workspace.panelPullRequests[panelId]?.number and workspace.pullRequest?.number
== 42 to verify final resolution. Ensure you reference the same panelId and the
probe counter API used by other tests so the assertions validate retry
preservation directly.
In `@Sources/TabManager.swift`:
- Around line 1018-1021: The method
workspaceGitProbeAttemptCountForTesting(workspaceId:panelId:) exposes a
test-only API in production; wrap its declaration in a conditional compilation
block (e.g. `#if` canImport(XCTest) || DEBUG ... `#endif`) so it is only compiled
for test/debug builds, leaving workspaceGitProbeTimersByKey and
WorkspaceGitProbeKey unchanged and ensuring the function remains available to
tests but not in release builds.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7b29ae92-236c-48c9-add0-f4aa0a4e8ab0
📒 Files selected for processing (2)
Sources/TabManager.swiftcmuxTests/TabManagerUnitTests.swift
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
cmuxTests/TabManagerUnitTests.swift (1)
591-664: Strengthen retry intent with an explicit attempt-count assertion.At Lines 657-663, the test currently proves eventual success, but not explicitly that retries occurred as intended. Since you already track calls in
.gh-call-count, assert>= 3to lock the retry contract and avoid future false positives from cached/alternate paths.Suggested assertion addition
XCTAssertTrue( waitForCondition(timeout: 3.0) { workspace.panelPullRequests[panelId]?.number == 42 } ) + let attempts = Int( + (try? String(contentsOf: ghCounterURL, encoding: .utf8)) + ?.trimmingCharacters(in: .whitespacesAndNewlines) ?? "" + ) ?? 0 + XCTAssertGreaterThanOrEqual( + attempts, + 3, + "Expected at least 3 `gh pr list` attempts to verify retry behavior" + ) XCTAssertEqual(workspace.panelGitBranches[panelId]?.branch, "feature/bootstrap-refresh") XCTAssertEqual(workspace.pullRequest?.number, 42)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmuxTests/TabManagerUnitTests.swift` around lines 591 - 664, The test testSelectingNewWorkspacePreservesInitialGitMetadataRetries should assert that the gh retry script was invoked at least 3 times; after the existing assertions (or right before the end of the withPrependedPath block) read the counter file at ghCounterURL.path (the .gh-call-count tracked by the gh script) and assert its integer value is >= 3 to guarantee retries occurred, failing the test if the file is missing or the parsed count is less than 3.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/WorkspaceContentView.swift`:
- Around line 165-171: The flash can restart after auto-clear because the
cleared runningFlashToken allows startFlash to run again with the same
state.flashToken; add a small latch: introduce a lastLatchedFlashToken (or reuse
a similarly-scoped var) and when you clear runningFlashToken in apply(...) also
set lastLatchedFlashToken = state.flashToken, then change the start condition
(the block that calls startFlash with workspaceId/state.flashToken/startedAt) to
require that state.flashToken != lastLatchedFlashToken (in addition to
state.flashRect != nil and runningFlashToken == nil) so the same token cannot
immediately re-enter the start path.
---
Nitpick comments:
In `@cmuxTests/TabManagerUnitTests.swift`:
- Around line 591-664: The test
testSelectingNewWorkspacePreservesInitialGitMetadataRetries should assert that
the gh retry script was invoked at least 3 times; after the existing assertions
(or right before the end of the withPrependedPath block) read the counter file
at ghCounterURL.path (the .gh-call-count tracked by the gh script) and assert
its integer value is >= 3 to guarantee retries occurred, failing the test if the
file is missing or the parsed count is less than 3.
🪄 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: c3a98ed4-970e-4c7d-9d8e-f68b40facc4f
📒 Files selected for processing (5)
Sources/ContentView.swiftSources/TabManager.swiftSources/WorkspaceContentView.swiftcmuxTests/TabManagerUnitTests.swiftcmuxTests/WindowAndDragTests.swift
🚧 Files skipped from review as they are similar to previous changes (3)
- cmuxTests/WindowAndDragTests.swift
- Sources/TabManager.swift
- Sources/ContentView.swift
| if runningFlashToken == nil, | ||
| state.flashRect != nil { | ||
| flashStartedAt = now() | ||
| startFlash( | ||
| workspaceId: state.workspaceId, | ||
| flashToken: state.flashToken, | ||
| startedAt: now() | ||
| ) |
There was a problem hiding this comment.
Latch the token after auto-clear.
Because Lines 202-203 clear runningFlashToken, the same flashToken falls back into the Lines 165-171 start path on the next apply(...) with a non-nil rect. Any later layout/unread update can replay the flash without a new attention event, which risks repeated animation work after the flash was supposed to be finished.
🛠️ Minimal fix
let workItem = DispatchWorkItem { [weak self] in
guard let self else { return }
guard self.lastWorkspaceId == workspaceId,
self.runningFlashToken == flashToken else {
return
}
self.flashStartedAt = nil
- self.runningFlashToken = nil
self.onStateChange?()
}Also applies to: 202-203
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/WorkspaceContentView.swift` around lines 165 - 171, The flash can
restart after auto-clear because the cleared runningFlashToken allows startFlash
to run again with the same state.flashToken; add a small latch: introduce a
lastLatchedFlashToken (or reuse a similarly-scoped var) and when you clear
runningFlashToken in apply(...) also set lastLatchedFlashToken =
state.flashToken, then change the start condition (the block that calls
startFlash with workspaceId/state.flashToken/startedAt) to require that
state.flashToken != lastLatchedFlashToken (in addition to state.flashRect != nil
and runningFlashToken == nil) so the same token cannot immediately re-enter the
start path.
lawrencecchen
added
the
stale-revisit
This branch was successfully deployed
3 participants
Summary
Closes #2540
TmuxWorkspacePaneOverlayViewpreviously usedTimelineView(.animation)unconditionally, causing the GPU/CPU to render every frame even with nothing visible. Now it conditionally switches betweenTimelineView(.animation)(only during flash), staticCanvas(unread indicators only), orColor.clear(nothing visible).DispatchWorkItemto clearflashStartedAtafterFocusFlashPattern.duration, so the view naturally falls back to the cheaper render path.Test plan
🤖 Generated with Claude Code
Summary by cubic
Stops continuous overlay animation and slows git/PR polling to cut idle CPU/GPU use. Closes #2540 and fixes overlay re-entrancy and prevents flash replay after completion.
Canvasfor unread, hide when empty viahasVisibleContent; latch completed flash tokens to avoid replay.FocusFlashPattern.duration; preserve across transient nil rects and resume when rect returns; cancel/reset on workspace change or when superseded.ghprobe timeout set to 5s.Written for commit 792e1c2. Summary will update on new commits.
Summary by CodeRabbit
Performance
New Features
Tests