From 7e0c9c760c8a8307c8490dc08707e78ffce8cfbd Mon Sep 17 00:00:00 2001 From: onevcat Date: Sun, 7 Jun 2026 23:14:58 +0900 Subject: [PATCH] Coalesce OSC-9 progress reports to cut tab-bar lag during agent activity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Agents (e.g. Claude Code) emit OSC-9 progress reports far faster than the UI needs. The bridge previously cancelled and re-created a 15s reset Task and wrote observable state on every report, churning the main thread while the agent streamed output; the determinate bar then relaid out a GeometryReader frame width and restarted a 0.2s easeInOut per distinct percent. The visible symptom is a laggy / stuttering progress indicator and elevated CPU while an agent is busy. - GhosttySurfaceBridge: route OSC-9 through `ingestProgressReport`, coalescing behind two single-purpose tasks — a leading-edge-then-trailing flush at the throttle cadence (50ms) and a slow stale-watch (1s wakeups, 15s timeout) that clears a bar whose reports stopped without a REMOVE. Identical reports dedupe at ingest, REMOVE wins over an unapplied trailing value, and the flush task guards cancellation before clearing its handle. The clock and intervals are injected for testing. - SET_TITLE and TerminalTabManager.updateTitle/updateDirty skip no-op writes a TUI re-emits constantly, so an unchanged value no longer invalidates the tab. - GhosttySurfaceProgressBar: bucket determinate percent to 5% steps and render the fill with a leading-anchored scaleEffect instead of a GeometryReader frame width, dropping the per-percent animation so a step composites rather than relaying out. Ported from upstream supacode #347, adapted to the fork's bridge, tab manager, and surface-level progress bar (the fork has no tab-stripe / shimmer views). --- .../Terminal/Models/TerminalTabManager.swift | 6 + .../Views/GhosttySurfaceProgressBar.swift | 32 ++-- .../Ghostty/GhosttySurfaceBridge.swift | 144 +++++++++++++--- supacodeTests/GhosttySurfaceBridgeTests.swift | 160 ++++++++++++++++++ .../GhosttySurfaceProgressBarTests.swift | 29 ++++ 5 files changed, 339 insertions(+), 32 deletions(-) create mode 100644 supacodeTests/GhosttySurfaceProgressBarTests.swift diff --git a/supacode/Features/Terminal/Models/TerminalTabManager.swift b/supacode/Features/Terminal/Models/TerminalTabManager.swift index 14be24f0..b8d74f37 100644 --- a/supacode/Features/Terminal/Models/TerminalTabManager.swift +++ b/supacode/Features/Terminal/Models/TerminalTabManager.swift @@ -38,6 +38,9 @@ final class TerminalTabManager { func updateTitle(_ id: TerminalTabID, title: String) -> Bool { guard let index = tabs.firstIndex(where: { $0.id == id }) else { return false } guard !tabs[index].isTitleLocked else { return false } + // A TUI re-emits the same title constantly; skip the no-op write so it + // doesn't invalidate the tab bar while an agent streams output. + guard tabs[index].title != title else { return false } let previousDisplayTitle = tabs[index].displayTitle tabs[index].title = title return tabs[index].displayTitle != previousDisplayTitle @@ -98,6 +101,9 @@ final class TerminalTabManager { func updateDirty(_ id: TerminalTabID, isDirty: Bool) { guard let index = tabs.firstIndex(where: { $0.id == id }) else { return } + // OSC-9 drives this on every progress tick; skip the no-op write so an + // unchanged dirty flag doesn't re-render the tab bar during agent activity. + guard tabs[index].isDirty != isDirty else { return } tabs[index].isDirty = isDirty } diff --git a/supacode/Features/Terminal/Views/GhosttySurfaceProgressBar.swift b/supacode/Features/Terminal/Views/GhosttySurfaceProgressBar.swift index 1d0e699e..1f7efcdd 100644 --- a/supacode/Features/Terminal/Views/GhosttySurfaceProgressBar.swift +++ b/supacode/Features/Terminal/Views/GhosttySurfaceProgressBar.swift @@ -13,7 +13,7 @@ struct GhosttySurfaceProgressBar: View { default: .accentColor } let progress: Int? = - progressValue ?? (progressState == GHOSTTY_PROGRESS_STATE_PAUSE ? 100 : nil) + progressValue.map(Self.bucketedPercent) ?? (progressState == GHOSTTY_PROGRESS_STATE_PAUSE ? 100 : nil) let accessibilityLabel: String = switch progressState { case GHOSTTY_PROGRESS_STATE_ERROR: "Terminal progress - Error" @@ -33,17 +33,15 @@ struct GhosttySurfaceProgressBar: View { } } - GeometryReader { geometry in - ZStack(alignment: .leading) { - if let progress { - Rectangle() - .fill(color) - .frame( - width: geometry.size.width * CGFloat(progress) / 100, - height: geometry.size.height - ) - .animation(.easeInOut(duration: 0.2), value: progress) - } else { + Group { + if let progress { + // Leading-anchored scaleEffect composites a percent step instead of + // relaying out a GeometryReader frame width on every OSC-9 tick. + Rectangle() + .fill(color) + .scaleEffect(x: CGFloat(progress) / 100, y: 1, anchor: .leading) + } else { + GeometryReader { geometry in ZStack(alignment: .leading) { Rectangle() .fill(color.opacity(0.3)) @@ -67,4 +65,14 @@ struct GhosttySurfaceProgressBar: View { .accessibilityLabel(accessibilityLabel) .accessibilityValue(accessibilityValue) } + + /// Quantize a determinate percent to 5% steps so a 0->100 sweep collapses to + /// ~20 distinct values that mostly no-op at the view's equality gates. 0 and + /// the >=100 terminus pass through unchanged so the bar still empties and tops + /// out exactly. + static func bucketedPercent(_ percent: Int) -> Int { + guard percent > 0 else { return 0 } + guard percent < 100 else { return 100 } + return (percent / 5) * 5 + } } diff --git a/supacode/Infrastructure/Ghostty/GhosttySurfaceBridge.swift b/supacode/Infrastructure/Ghostty/GhosttySurfaceBridge.swift index 59c0c7e7..30e39272 100644 --- a/supacode/Infrastructure/Ghostty/GhosttySurfaceBridge.swift +++ b/supacode/Infrastructure/Ghostty/GhosttySurfaceBridge.swift @@ -21,10 +21,40 @@ final class GhosttySurfaceBridge { var onDesktopNotification: ((String, String) -> Void)? var onCommandFinished: ((Int?, UInt64) -> Void)? var onPromptTitle: ((ghostty_action_prompt_title_e) -> Void)? - private var progressResetTask: Task? + + // Coalesce OSC-9 progress: a flush task applies the latest value at the + // throttle cadence while it moves, and a slow stale-watch clears a bar whose + // reports stopped without a REMOVE. + private let clock: any Clock + private let progressThrottleInterval: Duration + private let progressIdleInterval: Duration + private let progressStaleTimeout: Duration + private var pendingProgress: ProgressUpdate? + private var appliedProgress: ProgressUpdate? + private var progressReportCount = 0 + private var progressFlushTask: Task? + private var progressStaleTask: Task? + + init( + clock: any Clock = ContinuousClock(), + progressThrottleInterval: Duration = .milliseconds(50), + progressIdleInterval: Duration = .seconds(1), + progressStaleTimeout: Duration = .seconds(15) + ) { + self.clock = clock + self.progressThrottleInterval = progressThrottleInterval + self.progressIdleInterval = progressIdleInterval + self.progressStaleTimeout = progressStaleTimeout + } deinit { - progressResetTask?.cancel() + progressFlushTask?.cancel() + progressStaleTask?.cancel() + } + + private struct ProgressUpdate: Equatable { + let state: ghostty_action_progress_report_state_e + let value: Int? } func handleAction(target: ghostty_target_s, action: ghostty_action_s) -> Bool { @@ -200,7 +230,8 @@ final class GhosttySurfaceBridge { private func handleTitleAndPath(_ action: ghostty_action_s) -> Bool { switch action.tag { case GHOSTTY_ACTION_SET_TITLE: - if let title = string(from: action.action.set_title.title) { + // TUIs re-emit the same title constantly; skip the no-op write + a11y post. + if let title = string(from: action.action.set_title.title), title != state.title { state.title = title onTitleChange?(title) if let surfaceView { @@ -243,23 +274,10 @@ final class GhosttySurfaceBridge { switch action.tag { case GHOSTTY_ACTION_PROGRESS_REPORT: let report = action.action.progress_report - progressResetTask?.cancel() - state.progressValue = report.progress == -1 ? nil : Int(report.progress) - if report.state == GHOSTTY_PROGRESS_STATE_REMOVE { - state.progressState = nil - state.progressValue = nil - progressResetTask = nil - } else { - state.progressState = report.state - progressResetTask = Task { @MainActor [weak self] in - try? await ContinuousClock().sleep(for: .seconds(15)) - guard let self, !Task.isCancelled else { return } - self.state.progressState = nil - self.state.progressValue = nil - self.onProgressReport?(GHOSTTY_PROGRESS_STATE_REMOVE) - } - } - onProgressReport?(report.state) + ingestProgressReport( + state: report.state, + value: report.progress == -1 ? nil : Int(report.progress) + ) return true case GHOSTTY_ACTION_COMMAND_FINISHED: @@ -289,6 +307,92 @@ final class GhosttySurfaceBridge { } } + /// Coalescing entry point for OSC-9 progress. REMOVE clears immediately; a + /// value identical to what's already shown only refreshes the stale window. + func ingestProgressReport(state: ghostty_action_progress_report_state_e, value: Int?) { + guard state != GHOSTTY_PROGRESS_STATE_REMOVE else { + flushProgressRemoval() + return + } + // The counter is the stale watch's liveness signal; bump it on every report. + progressReportCount &+= 1 + startProgressStaleWatchIfNeeded() + let update = ProgressUpdate(state: state, value: value) + guard update != appliedProgress else { return } + pendingProgress = update + scheduleProgressFlush() + } + + /// Leading-edge then trailing throttle: paint a new value immediately when no + /// flush is in flight, then batch any further changes into one flush per + /// throttle interval. Idles to nothing once the value stops moving. + private func scheduleProgressFlush() { + guard progressFlushTask == nil else { return } + applyPendingProgress() + progressFlushTask = Task { @MainActor [weak self] in + guard let self else { return } + try? await self.clock.sleep(for: self.progressThrottleInterval) + // Check cancellation before clearing the handle: a cancelled task (REMOVE + // raced a reschedule) must not clobber the live task's handle. + guard !Task.isCancelled else { return } + self.progressFlushTask = nil + guard self.pendingProgress != nil else { return } + self.scheduleProgressFlush() + } + } + + /// Slow watch that clears a bar whose reports stopped without a REMOVE (e.g. + /// the process died). Wakes at the idle cadence, not the throttle cadence, so + /// a held bar doesn't pin a high-frequency wakeup on the main thread. + private func startProgressStaleWatchIfNeeded() { + guard progressStaleTask == nil else { return } + let startCount = progressReportCount + progressStaleTask = Task { @MainActor [weak self] in + guard let self else { return } + var lastSeenCount = startCount + var idleElapsed: Duration = .zero + while !Task.isCancelled { + try? await self.clock.sleep(for: self.progressIdleInterval) + guard !Task.isCancelled else { return } + if self.progressReportCount != lastSeenCount { + lastSeenCount = self.progressReportCount + idleElapsed = .zero + continue + } + idleElapsed += self.progressIdleInterval + if idleElapsed >= self.progressStaleTimeout { + self.flushProgressRemoval() + return + } + } + } + } + + private func applyPendingProgress() { + guard let pending = pendingProgress else { return } + pendingProgress = nil + guard pending != appliedProgress else { return } + appliedProgress = pending + state.progressState = pending.state + state.progressValue = pending.value + onProgressReport?(pending.state) + } + + private func flushProgressRemoval() { + // REMOVE wins over any unapplied trailing value: applying it first would + // emit a spurious determinate paint that coalesces away before render, + // since the bar is clearing anyway. + progressFlushTask?.cancel() + progressFlushTask = nil + progressStaleTask?.cancel() + progressStaleTask = nil + pendingProgress = nil + appliedProgress = nil + state.progressState = nil + state.progressValue = nil + onProgressReport?(GHOSTTY_PROGRESS_STATE_REMOVE) + } + private func handleMouseAndLink(_ action: ghostty_action_s) -> Bool { switch action.tag { case GHOSTTY_ACTION_MOUSE_SHAPE: diff --git a/supacodeTests/GhosttySurfaceBridgeTests.swift b/supacodeTests/GhosttySurfaceBridgeTests.swift index bb75132d..8a471983 100644 --- a/supacodeTests/GhosttySurfaceBridgeTests.swift +++ b/supacodeTests/GhosttySurfaceBridgeTests.swift @@ -1,9 +1,14 @@ +import Clocks import GhosttyKit import Testing @testable import supacode +// Serialized: the coalescing tests drive a TestClock with two concurrent +// sleepers (flush + stale watch); parallel execution can race `advance` before +// a task suspends and flake. @MainActor +@Suite(.serialized) struct GhosttySurfaceBridgeTests { @Test func activeTextReturnsNilWithoutSurfaceView() { let bridge = GhosttySurfaceBridge() @@ -52,4 +57,159 @@ struct GhosttySurfaceBridgeTests { #expect(callbackCount == 1) #expect(bridge.state.configChangeCount == 1) } + + @Test func coalescesBurstOfProgressReports() async { + let clock = TestClock() + let bridge = GhosttySurfaceBridge( + clock: clock, + progressThrottleInterval: .milliseconds(50), + progressIdleInterval: .milliseconds(50), + progressStaleTimeout: .seconds(15) + ) + var callbackCount = 0 + bridge.onProgressReport = { _ in callbackCount += 1 } + + // Leading edge applies the first report immediately; the rest coalesce. + bridge.ingestProgressReport(state: GHOSTTY_PROGRESS_STATE_SET, value: 10) + bridge.ingestProgressReport(state: GHOSTTY_PROGRESS_STATE_SET, value: 20) + bridge.ingestProgressReport(state: GHOSTTY_PROGRESS_STATE_SET, value: 50) + #expect(bridge.state.progressValue == 10) + #expect(callbackCount == 1) + + // One throttle tick flushes only the latest coalesced value. + await clock.advance(by: .milliseconds(50)) + #expect(bridge.state.progressValue == 50) + #expect(callbackCount == 2) + } + + @Test func staleProgressClearsAfterTimeout() async { + let clock = TestClock() + let bridge = GhosttySurfaceBridge( + clock: clock, + progressThrottleInterval: .milliseconds(50), + progressIdleInterval: .milliseconds(50), + progressStaleTimeout: .milliseconds(200) + ) + var lastState: ghostty_action_progress_report_state_e? + bridge.onProgressReport = { lastState = $0 } + + bridge.ingestProgressReport(state: GHOSTTY_PROGRESS_STATE_INDETERMINATE, value: nil) + #expect(bridge.state.progressState == GHOSTTY_PROGRESS_STATE_INDETERMINATE) + + // No further reports: the driver synthesizes a REMOVE once the window lapses. + await clock.advance(by: .milliseconds(200)) + #expect(bridge.state.progressState == nil) + #expect(lastState == GHOSTTY_PROGRESS_STATE_REMOVE) + } + + @Test func continuedReportsKeepProgressAlivePastStaleWindow() async { + let clock = TestClock() + let bridge = GhosttySurfaceBridge( + clock: clock, + progressThrottleInterval: .milliseconds(50), + progressIdleInterval: .milliseconds(50), + progressStaleTimeout: .milliseconds(100) + ) + var lastState: ghostty_action_progress_report_state_e? + bridge.onProgressReport = { lastState = $0 } + + bridge.ingestProgressReport(state: GHOSTTY_PROGRESS_STATE_INDETERMINATE, value: nil) + // A long indeterminate run re-fires identical reports; the stale timer must + // keep resetting even though the value never changes. + for _ in 0..<6 { + bridge.ingestProgressReport(state: GHOSTTY_PROGRESS_STATE_INDETERMINATE, value: nil) + await clock.advance(by: .milliseconds(50)) + } + #expect(bridge.state.progressState == GHOSTTY_PROGRESS_STATE_INDETERMINATE) + #expect(lastState == GHOSTTY_PROGRESS_STATE_INDETERMINATE) + } + + @Test func progressDriverRestartsAfterStaleRemoval() async { + let clock = TestClock() + let bridge = GhosttySurfaceBridge( + clock: clock, + progressThrottleInterval: .milliseconds(50), + progressIdleInterval: .milliseconds(50), + progressStaleTimeout: .milliseconds(100) + ) + bridge.onProgressReport = { _ in } + + bridge.ingestProgressReport(state: GHOSTTY_PROGRESS_STATE_INDETERMINATE, value: nil) + // No further reports: the stale window synthesizes a REMOVE and tears down + // the driver. + await clock.advance(by: .milliseconds(100)) + #expect(bridge.state.progressState == nil) + + // A report after the stale REMOVE must re-arm the driver, not freeze. + bridge.ingestProgressReport(state: GHOSTTY_PROGRESS_STATE_SET, value: 30) + #expect(bridge.state.progressValue == 30) + bridge.ingestProgressReport(state: GHOSTTY_PROGRESS_STATE_SET, value: 60) + await clock.advance(by: .milliseconds(50)) + #expect(bridge.state.progressValue == 60) + } + + @Test func determinateValuePaintsPromptlyAfterIdlePeriod() async { + let clock = TestClock() + let bridge = GhosttySurfaceBridge( + clock: clock, + progressThrottleInterval: .milliseconds(50), + progressIdleInterval: .milliseconds(50), + progressStaleTimeout: .seconds(15) + ) + bridge.onProgressReport = { _ in } + + bridge.ingestProgressReport(state: GHOSTTY_PROGRESS_STATE_SET, value: 10) + #expect(bridge.state.progressValue == 10) + + // Sit idle well past the throttle window, then a fresh value must paint on + // its leading edge instead of waiting for a slow idle tick. + await clock.advance(by: .seconds(1)) + bridge.ingestProgressReport(state: GHOSTTY_PROGRESS_STATE_SET, value: 80) + #expect(bridge.state.progressValue == 80) + } + + @Test func identicalReportsNeverReapply() async { + let clock = TestClock() + let bridge = GhosttySurfaceBridge( + clock: clock, + progressThrottleInterval: .milliseconds(50), + progressIdleInterval: .milliseconds(50), + progressStaleTimeout: .seconds(15) + ) + var callbackCount = 0 + bridge.onProgressReport = { _ in callbackCount += 1 } + + bridge.ingestProgressReport(state: GHOSTTY_PROGRESS_STATE_INDETERMINATE, value: nil) + #expect(callbackCount == 1) + + // A flood of identical reports keeps the bar alive but never re-applies, so + // the downstream callback fires exactly once across the whole stream. + for _ in 0..<10 { + bridge.ingestProgressReport(state: GHOSTTY_PROGRESS_STATE_INDETERMINATE, value: nil) + await clock.advance(by: .milliseconds(50)) + } + #expect(callbackCount == 1) + #expect(bridge.state.progressState == GHOSTTY_PROGRESS_STATE_INDETERMINATE) + } + + @Test func removeWinsOverUnappliedTrailingValue() { + let bridge = GhosttySurfaceBridge( + clock: TestClock(), + progressThrottleInterval: .milliseconds(50), + progressStaleTimeout: .seconds(15) + ) + var states: [ghostty_action_progress_report_state_e] = [] + bridge.onProgressReport = { states.append($0) } + + // First SET applies on the leading edge; the second sits un-applied in + // pendingProgress because no throttle tick has fired yet. + bridge.ingestProgressReport(state: GHOSTTY_PROGRESS_STATE_SET, value: 50) + bridge.ingestProgressReport(state: GHOSTTY_PROGRESS_STATE_SET, value: 100) + // REMOVE before the tick drops the trailing 100 and clears. + bridge.ingestProgressReport(state: GHOSTTY_PROGRESS_STATE_REMOVE, value: nil) + + #expect(bridge.state.progressState == nil) + #expect(bridge.state.progressValue == nil) + #expect(states == [GHOSTTY_PROGRESS_STATE_SET, GHOSTTY_PROGRESS_STATE_REMOVE]) + } } diff --git a/supacodeTests/GhosttySurfaceProgressBarTests.swift b/supacodeTests/GhosttySurfaceProgressBarTests.swift new file mode 100644 index 00000000..6b76dff6 --- /dev/null +++ b/supacodeTests/GhosttySurfaceProgressBarTests.swift @@ -0,0 +1,29 @@ +import Testing + +@testable import supacode + +struct GhosttySurfaceProgressBarTests { + @Test func zeroPassesThrough() { + #expect(GhosttySurfaceProgressBar.bucketedPercent(0) == 0) + } + + @Test func terminusPassesThrough() { + #expect(GhosttySurfaceProgressBar.bucketedPercent(100) == 100) + #expect(GhosttySurfaceProgressBar.bucketedPercent(150) == 100) + } + + @Test func midValuesQuantizeDownToFivePercentSteps() { + #expect(GhosttySurfaceProgressBar.bucketedPercent(1) == 0) + #expect(GhosttySurfaceProgressBar.bucketedPercent(4) == 0) + #expect(GhosttySurfaceProgressBar.bucketedPercent(5) == 5) + #expect(GhosttySurfaceProgressBar.bucketedPercent(7) == 5) + #expect(GhosttySurfaceProgressBar.bucketedPercent(47) == 45) + #expect(GhosttySurfaceProgressBar.bucketedPercent(99) == 95) + } + + @Test func collapsesASweepToAtMostTwentyOneDistinctValues() { + let distinct = Set((0...100).map(GhosttySurfaceProgressBar.bucketedPercent)) + // 0, 5, 10, ..., 95, 100 -> 21 buckets. + #expect(distinct.count == 21) + } +} -- 2.51.2