diff --git a/src/CodeShellManager/MainWindow.xaml.cs b/src/CodeShellManager/MainWindow.xaml.cs index 61bf63e..abe661d 100644 --- a/src/CodeShellManager/MainWindow.xaml.cs +++ b/src/CodeShellManager/MainWindow.xaml.cs @@ -1635,6 +1635,13 @@ private void UpdateSidebarActiveState() private void UpdateActiveTerminalHighlight() { string? activeId = _vm.ActiveSession?.Id; + + // Keep each bridge's output dispatcher priority in sync with focus, so a chatty + // background session posts at Background priority and can't sit ahead of the + // active pane's rendering or its keystrokes (issue #70). + foreach (var s in _vm.Sessions) + if (s.Bridge != null) s.Bridge.IsForeground = s.Id == activeId; + foreach (var (id, ui) in _sessionUi) { if (id == activeId) diff --git a/src/CodeShellManager/Terminal/OutputCoalescer.cs b/src/CodeShellManager/Terminal/OutputCoalescer.cs new file mode 100644 index 0000000..3229f59 --- /dev/null +++ b/src/CodeShellManager/Terminal/OutputCoalescer.cs @@ -0,0 +1,72 @@ +using System; +using System.Text; + +namespace CodeShellManager.Terminal; + +/// +/// Collapses many small PTY output chunks into a single scheduled post (issue #70). +/// +/// Without this, every PTY chunk from every session became its own dispatcher work item. +/// Chatty background sessions (Claude spinners, status-line repaints) flooded the shared +/// UI queue, and both the foreground terminal's rendering *and* its keystrokes — which +/// arrive on the same UI thread via WebView2's WebMessageReceived — had to wait behind +/// that backlog. See issue #70 for the full trace. +/// +/// The scheduler and emitter are injected so the state machine is testable with no WPF +/// dispatcher and no WebView2 present. +/// +public sealed class OutputCoalescer +{ + private readonly object _lock = new(); + private readonly StringBuilder _buffer = new(); + private readonly Action _schedule; + private readonly Action _emit; + + // True between scheduling a flush and that flush draining the buffer. While set, + // further appends piggyback on the already-queued flush instead of adding to the queue. + private bool _flushPending; + + /// Marshals the flush onto the UI thread (Dispatcher.BeginInvoke in production). + /// Delivers one coalesced payload (PostWebMessageAsString in production). + public OutputCoalescer(Action schedule, Action emit) + { + _schedule = schedule; + _emit = emit; + } + + /// Buffers a PTY chunk. Safe to call from any thread. Never blocks on the UI thread. + public void Append(string data) + { + if (string.IsNullOrEmpty(data)) return; + + bool scheduleNow = false; + lock (_lock) + { + _buffer.Append(data); + if (!_flushPending) + { + _flushPending = true; + scheduleNow = true; + } + } + + // Outside the lock: the scheduler may run the flush inline on this thread. + if (scheduleNow) _schedule(Flush); + } + + private void Flush() + { + string payload; + lock (_lock) + { + payload = _buffer.ToString(); + _buffer.Clear(); + // Cleared *before* emitting so a chunk arriving during the emit below + // schedules a fresh flush rather than being silently dropped. + _flushPending = false; + } + + // Emit outside the lock — it re-enters WebView2 and must not hold up Append. + if (payload.Length > 0) _emit(payload); + } +} diff --git a/src/CodeShellManager/Terminal/TerminalBridge.cs b/src/CodeShellManager/Terminal/TerminalBridge.cs index 518e8b7..3f13ad1 100644 --- a/src/CodeShellManager/Terminal/TerminalBridge.cs +++ b/src/CodeShellManager/Terminal/TerminalBridge.cs @@ -36,6 +36,17 @@ public sealed class TerminalBridge : IDisposable // Output that arrived before the page finished loading is buffered here private readonly System.Text.StringBuilder _outputBuffer = new(); + // Coalesces PTY chunks into one dispatcher post per tick (issue #70). + private readonly OutputCoalescer _coalescer; + + /// + /// True when this session's pane is the active one. Foreground output posts at + /// Normal priority; background sessions post at Background priority so a chatty + /// off-screen session can't delay the pane the user is actually typing into. + /// Set by MainWindow whenever MainViewModel.ActiveSession changes. + /// + public bool IsForeground { get; set; } + // Diagnostics — gated by AppSettings.DebugTerminalTrace. Zero cost when off. /// AppSettings reference whose DebugTerminalTrace flag gates [DEBUG-tt] logging. public AppSettings? DebugSettings { get; set; } @@ -96,6 +107,30 @@ private void PostBootDoneIfNeeded() public TerminalBridge(WebView2 webView) { _webView = webView; + _coalescer = new OutputCoalescer(ScheduleFlush, PostOutput); + } + + // Queues one coalesced flush. Background sessions yield to the foreground pane so + // their output can't sit ahead of the active session's rendering or its keystrokes. + private void ScheduleFlush(Action flush) + { + var dispatcher = WpfApplication.Current?.Dispatcher; + if (dispatcher == null) return; + dispatcher.BeginInvoke( + IsForeground + ? System.Windows.Threading.DispatcherPriority.Normal + : System.Windows.Threading.DispatcherPriority.Background, + flush); + } + + // Runs on the UI thread. One WebView2 post per coalesced batch. + private void PostOutput(string data) + { + string json = JsonSerializer.Serialize(new { type = "output", data }); + try { _webView.CoreWebView2?.PostWebMessageAsString(json); } + catch { } + if (DebugSettings?.DebugTerminalTrace == true) + Trace($"OUTPUT flush len={data.Length}"); } /// @@ -200,16 +235,10 @@ void NavCompleted(object? s, CoreWebView2NavigationCompletedEventArgs e) buffered = _outputBuffer.ToString(); _outputBuffer.Clear(); } - if (buffered.Length > 0) - { - string json = System.Text.Json.JsonSerializer.Serialize( - new { type = "output", data = buffered }); - WpfApplication.Current?.Dispatcher.BeginInvoke(() => - { - try { _webView.CoreWebView2?.PostWebMessageAsString(json); } - catch { } - }); - } + // Route through the coalescer rather than posting directly: chunks that land + // between `_ready = true` above and this drain go to the coalescer, so sharing + // one buffer keeps load-time output in arrival order. + if (buffered.Length > 0) _coalescer.Append(buffered); navDone.TrySetResult(true); } @@ -253,20 +282,10 @@ private void OnPtyData(string rawData) return; } - string json = JsonSerializer.Serialize(new { type = "output", data = rawData }); - long enqueueAt = DebugSettings?.DebugTerminalTrace == true ? Environment.TickCount64 : 0; - int len = rawData.Length; - WpfApplication.Current?.Dispatcher.BeginInvoke(() => - { - // Capture latency before any work so Trace's file I/O doesn't inflate the - // measurement, then post the WebView2 message before tracing so the trace - // overhead doesn't delay terminal rendering. - long latencyMs = enqueueAt != 0 ? Environment.TickCount64 - enqueueAt : 0; - try { _webView.CoreWebView2?.PostWebMessageAsString(json); } - catch { } - if (enqueueAt != 0) - Trace($"OUTPUT post dispatcher-latency={latencyMs}ms len={len}"); - }); + // Buffer instead of posting per chunk. Many chunks arriving before the dispatcher + // gets a turn collapse into a single post, so background sessions can no longer + // flood the shared UI queue and starve the foreground pane (issue #70). + _coalescer.Append(rawData); } private void OnAcceleratorKeyPressed(object? sender, WpfKeyEventArgs e) diff --git a/src/CodeShellManager/ViewModels/MainViewModel.cs b/src/CodeShellManager/ViewModels/MainViewModel.cs index 3802d43..2f9a09f 100644 --- a/src/CodeShellManager/ViewModels/MainViewModel.cs +++ b/src/CodeShellManager/ViewModels/MainViewModel.cs @@ -60,6 +60,19 @@ public partial class MainViewModel : ObservableObject public int AlertCount => Sessions.Count(s => s.NeedsAttention); + // AlertCount is an O(N) scan and every raise invalidates WPF bindings. It's touched on + // the keystroke path (via AlertCleared), where the value is almost always unchanged, so + // suppress no-op raises rather than re-running binding invalidation per character (#70). + private int _lastRaisedAlertCount = -1; + + private void RaiseAlertCountIfChanged() + { + int count = AlertCount; + if (count == _lastRaisedAlertCount) return; + _lastRaisedAlertCount = count; + OnPropertyChanged(nameof(AlertCount)); + } + public event Action? SessionClosed; public event Action? GroupsChanged; public event Action? SelectionChanged; @@ -267,31 +280,34 @@ public void RegisterSession(SessionViewModel vm) if (vm.Bridge != null) { - vm.Bridge.UserInput += () => - { - vm.AlertDetector?.NotifyUserInteracted(); - App.Current.Dispatcher.Invoke(() => OnPropertyChanged(nameof(AlertCount))); - }; + // Runs on the UI thread for every keystroke (WebView2 raises WebMessageReceived + // there), so it must stay cheap. NotifyUserInteracted already fires AlertCleared + // unconditionally, whose handler below raises AlertCount — so this deliberately + // does not raise it a second time (issue #70). + vm.Bridge.UserInput += () => vm.AlertDetector?.NotifyUserInteracted(); } if (vm.AlertDetector != null) { + // BeginInvoke, not Invoke: AlertRaised arrives on a System.Threading.Timer + // callback, and blocking a threadpool thread on a busy UI thread serves no + // purpose — no caller consumes a result. vm.AlertDetector.AlertRaised += alert => { - App.Current.Dispatcher.Invoke(() => + App.Current.Dispatcher.BeginInvoke(() => { vm.RaiseAlert(alert.Message, alert.Type); - OnPropertyChanged(nameof(AlertCount)); + RaiseAlertCountIfChanged(); if (Settings.ShowToastNotifications) ToastHelper.Show(vm.DisplayName, alert.Message, Settings.ShowNotificationSound); }); }; vm.AlertDetector.AlertCleared += _ => { - App.Current.Dispatcher.Invoke(() => + App.Current.Dispatcher.BeginInvoke(() => { vm.ClearAlert(); - OnPropertyChanged(nameof(AlertCount)); + RaiseAlertCountIfChanged(); }); }; } diff --git a/tests/CodeShellManager.Tests/OutputCoalescerTests.cs b/tests/CodeShellManager.Tests/OutputCoalescerTests.cs new file mode 100644 index 0000000..cd35469 --- /dev/null +++ b/tests/CodeShellManager.Tests/OutputCoalescerTests.cs @@ -0,0 +1,144 @@ +using CodeShellManager.Terminal; +using Xunit; + +namespace CodeShellManager.Tests; + +/// +/// Covers the coalescing state machine that collapses N PTY output chunks into a single +/// dispatcher post (issue #70). The scheduler and emitter are injected so these run +/// headlessly with no WPF dispatcher and no WebView2. +/// +public class OutputCoalescerTests +{ + /// Captures scheduled flushes so the test controls when they run. + private sealed class ManualScheduler + { + private readonly List _queued = new(); + public int ScheduleCount { get; private set; } + public List Emitted { get; } = new(); + + public void Schedule(Action flush) + { + ScheduleCount++; + _queued.Add(flush); + } + + public void Emit(string payload) => Emitted.Add(payload); + + /// Runs every flush queued so far (mimics the dispatcher draining). + public void Drain() + { + var batch = _queued.ToList(); + _queued.Clear(); + foreach (var f in batch) f(); + } + } + + private static (OutputCoalescer, ManualScheduler) Build() + { + var s = new ManualScheduler(); + return (new OutputCoalescer(s.Schedule, s.Emit), s); + } + + [Fact] + public void SingleAppend_SchedulesOneFlush_AndEmitsThatData() + { + var (c, s) = Build(); + + c.Append("hello"); + + Assert.Equal(1, s.ScheduleCount); + Assert.Empty(s.Emitted); // nothing emitted until the dispatcher runs the flush + s.Drain(); + Assert.Equal(new[] { "hello" }, s.Emitted); + } + + [Fact] + public void ManyAppendsBeforeFlush_ScheduleOnlyOneFlush() + { + var (c, s) = Build(); + + for (int i = 0; i < 50; i++) c.Append("x"); + + Assert.Equal(1, s.ScheduleCount); + } + + [Fact] + public void ManyAppendsBeforeFlush_EmitOnceWithConcatenatedDataInOrder() + { + var (c, s) = Build(); + + c.Append("a"); + c.Append("b"); + c.Append("c"); + s.Drain(); + + Assert.Equal(new[] { "abc" }, s.Emitted); + } + + [Fact] + public void AppendAfterFlush_SchedulesANewFlush() + { + var (c, s) = Build(); + + c.Append("first"); + s.Drain(); + c.Append("second"); + s.Drain(); + + Assert.Equal(2, s.ScheduleCount); + Assert.Equal(new[] { "first", "second" }, s.Emitted); + } + + [Fact] + public void FlushWithNothingBuffered_DoesNotEmit() + { + var (c, s) = Build(); + + c.Append("only"); + s.Drain(); // drains "only" + s.Drain(); // no-op: queue is empty, nothing new buffered + + Assert.Equal(new[] { "only" }, s.Emitted); + } + + [Fact] + public void DataAppendedDuringFlush_IsNotLost() + { + var s = new ManualScheduler(); + OutputCoalescer? c = null; + bool reentered = false; + // Emit re-enters Append, simulating a PTY chunk landing while the flush runs. + c = new OutputCoalescer(s.Schedule, payload => + { + s.Emit(payload); + if (!reentered) + { + reentered = true; + c!.Append("late"); + } + }); + + c.Append("early"); + s.Drain(); // emits "early", during which "late" is appended + s.Drain(); // must flush "late" + + Assert.Equal(new[] { "early", "late" }, s.Emitted); + } + + [Fact] + public void ConcurrentAppends_LoseNoData() + { + var (c, s) = Build(); + const int threads = 8, perThread = 500; + + Parallel.For(0, threads, _ => + { + for (int i = 0; i < perThread; i++) c.Append("z"); + }); + // Drain repeatedly: appends racing with a flush may leave a second flush pending. + for (int i = 0; i < 5; i++) s.Drain(); + + Assert.Equal(threads * perThread, string.Concat(s.Emitted).Length); + } +}