diff --git a/src/ClaudeDo.Ui/CLAUDE.md b/src/ClaudeDo.Ui/CLAUDE.md index f6fdaf8b..014f9995 100644 --- a/src/ClaudeDo.Ui/CLAUDE.md +++ b/src/ClaudeDo.Ui/CLAUDE.md @@ -89,6 +89,7 @@ new editor boilerplate there rather than copying it a third time. - **IPrimeScheduleApi** — prime-schedule CRUD. - **UpdateCheckService** — polls releases; `LastCheckStatus`/`LatestVersion`/`CheckNowAsync` feed the shell's update banner. - **InheritanceResolver** — resolves the task → list → global override chain to `(value, source)` for the inherited badges. +- **OperationTiming** — NDJSON sink (`~/.todo-app/logs/operation-timing.ndjson`, one rolled `.1` at 4 MB) behind every hub invoke and bulk DB path; each line carries `pid` because app restarts interleave in one file. Successful calls under 25 ms are dropped, **failed/cancelled ones always land** — a 1 ms cancelled `BindAsync` is churn signal, not noise. `DetailsIsland.BindAsync:` carries the selection trigger (`TasksIslandViewModel.SelectionSource`, set via `SelectFrom` — never assign `SelectedTask` directly), because the first day of data showed ~1000 binds in 3.5 h, 91% cancelled, with an unexplained trigger. - **RepoScanner**, **InstallArtifactLocator**/**InstallerLocator**/**WorkerLocator**, **ForegroundHelper** (Win32 foreground before launching a terminal), **FocusClearing**. ## Converters @@ -118,6 +119,7 @@ snaps `CanResize="True"` windows, which is the opt-in), and it insets itself by - Context menus exist on both list and task rows; right-click selects before opening the menu. - "Run Now" CanExecute re-evaluates when worker connection state changes. - `Ellipse.spinner` (IslandStyles) is the shared indeterminate spinner (starting ConPTY pane, refining task row). -- **Every `IWorkerClient` call in a `[RelayCommand]` runs through an `OperationStatus` (`Services/OperationStatus.cs`), shown via the `OperationIndicator` control** — never a handcrafted spinner. Without it the spinner `StackPanel` from `MergeModalView.axaml` gets rebuilt at every call site, and `IsBusy` flags lock the button but show nothing. +- **Never set a `UserControl`'s own `DataContext` from a `StyledProperty`.** The property binding at the call site (`Status="{Binding RejectOp}"`) resolves *against that control's DataContext* — overwriting it re-targets the binding at the new value and the compiled getter throws `InvalidCastException`. Scope the DataContext to an inner panel instead (`DataContext="{Binding #Root.Status}"`, see `OperationIndicator.axaml`). Corollary: **the inner panel's DataContext is then null whenever the property is null**, so `IsVisible="{Binding ShowIndicator}"` can't resolve and falls back to the property default `true` — a permanent label-less spinner *plus* the stalled hint. `OperationIndicator` therefore collapses itself in code-behind when `Status` is null; wire every call site with `Status=`, never `DataContext=`. +- **Every `IWorkerClient` call in a `[RelayCommand]` runs through an `OperationStatus` (`Services/OperationStatus.cs`), shown via the `OperationIndicator` control** — never a handcrafted spinner. Without it the spinner `StackPanel` from `MergeModalView.axaml` gets rebuilt at every call site, and `IsBusy` flags lock the button but show nothing. **`Begin`/`Report`/`Dispose` must run on the UI thread** — only the tick is marshalled, so a `Task.Run`-wrapped call leaves the bindings updating off-thread (the startup update check did this). A `[RelayCommand]` satisfies this on its own. Separately, **`End` retires the generation** so tick work that was posted while the dispatcher was blocked can't drain afterwards and set `ShowIndicator` back to true on a finished operation — that was the real frozen-spinner bug, and the enabling condition (~2.4s blocked UI thread during window construction) is still there. - `SessionTerminalView` is the reusable log terminal (StyledProperties `Entries`, `Label`, `IsRunning`, `IsDone`, `IsFailed`) — used for both the task `Log` and the prep `PrepLog`. - `DetailsIslandView` is a pane-wide drag-and-drop file target (`DragDrop.AllowDrop`, Avalonia 12 `DataFormat.File`) with a "Drop to attach" overlay; `DescriptionStepsCard` shows the attachments list, an "Add file…" picker, and an explicit `DropStatus` line. Keys use the `details.attachments.*` locale namespace (en + de). diff --git a/src/ClaudeDo.Ui/Services/OperationStatus.cs b/src/ClaudeDo.Ui/Services/OperationStatus.cs index bcf04e1a..33613d37 100644 --- a/src/ClaudeDo.Ui/Services/OperationStatus.cs +++ b/src/ClaudeDo.Ui/Services/OperationStatus.cs @@ -7,6 +7,11 @@ namespace ClaudeDo.Ui.Services; /// independent instances per ViewModel are expected (one per command) — there is no shared or /// static state, which is exactly what a static DispatcherTimer would introduce and the cause of /// the order-dependent flakiness in Ui.Tests. +/// +/// **Call Begin/Report/Dispose from the UI thread.** Only the timer tick is marshalled; the other +/// three write their observable properties inline on the calling thread, so an off-thread caller +/// leaves the bindings updating off the UI thread. A [RelayCommand] satisfies this by itself (its +/// awaits resume on the UI thread); wrapping the call in Task.Run breaks it. public sealed partial class OperationStatus : ObservableObject { private static readonly TimeSpan GracePeriod = TimeSpan.FromMilliseconds(300); @@ -79,6 +84,12 @@ public sealed partial class OperationStatus : ObservableObject private void End(int generation) { if (generation != _generation) return; + // Retire the generation *before* touching the flags. The timer callback fires on a + // threadpool thread and only posts its UI work, so a tick that fired while the dispatcher + // was busy can drain *after* End — with the generation still current it would set + // ShowIndicator back to true on an already-finished operation, and since the timer is gone + // by then no later tick ever clears it. That is the frozen spinner. + _generation++; _tickTimer?.Dispose(); _tickTimer = null; IsRunning = false; diff --git a/src/ClaudeDo.Ui/Services/OperationTiming.cs b/src/ClaudeDo.Ui/Services/OperationTiming.cs index 5cd0a95b..2a53af7f 100644 --- a/src/ClaudeDo.Ui/Services/OperationTiming.cs +++ b/src/ClaudeDo.Ui/Services/OperationTiming.cs @@ -10,26 +10,49 @@ namespace ClaudeDo.Ui.Services; /// public sealed class OperationTiming { + /// Fast successful operations are dropped: the first day of data was 43% 3ms reconcile + /// ticks, which crowd out the outliers the file exists to surface. Failed/cancelled operations + /// are always kept regardless of duration — a 1ms cancelled call is a churn signal, not noise. + public const int DefaultMinMs = 25; + + /// One rollover file is kept (.1); at ~1000 lines/h a 4 MB cap holds weeks. + public const long DefaultMaxBytes = 4L * 1024 * 1024; + public static string DefaultPath => Path.Combine(Paths.AppDataRoot(), "logs", "operation-timing.ndjson"); public static OperationTiming Shared { get; } = new(DefaultPath); + private static readonly int ProcessId = Environment.ProcessId; + private readonly string _path; + private readonly int _minMs; + private readonly long _maxBytes; private readonly object _writeLock = new(); - public OperationTiming(string filePath) => _path = filePath; + public OperationTiming(string filePath, int minMs = DefaultMinMs, long maxBytes = DefaultMaxBytes) + { + _path = filePath; + _minMs = minMs; + _maxBytes = maxBytes; + } public void Record(string kind, string operation, TimeSpan elapsed, bool ok) { try { + var ms = (long)elapsed.TotalMilliseconds; + if (ok && ms < _minMs) return; + var line = JsonSerializer.Serialize(new { ts = DateTime.UtcNow, + // Runs of the app interleave in one file; without the pid, a restart looks like + // concurrent work by a single instance. + pid = ProcessId, kind, op = operation, - ms = (long)elapsed.TotalMilliseconds, + ms, ok, }); @@ -38,6 +61,7 @@ public sealed class OperationTiming var dir = Path.GetDirectoryName(_path); if (!string.IsNullOrEmpty(dir)) Directory.CreateDirectory(dir); + Roll(); File.AppendAllText(_path, line + Environment.NewLine); } } @@ -46,4 +70,12 @@ public sealed class OperationTiming // A measurement must never disturb the app. } } + + private void Roll() + { + if (!File.Exists(_path) || new FileInfo(_path).Length < _maxBytes) return; + var rolled = _path + ".1"; + File.Delete(rolled); + File.Move(_path, rolled); + } } diff --git a/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs b/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs index b8a202d2..34ade86c 100644 --- a/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs @@ -578,7 +578,7 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable public void ShowNotes() { - Bind(null); + Bind(null, "notes"); IsPrepMode = false; IsNotesMode = true; _ = Notes.LoadDayAsync(DateOnly.FromDateTime(DateTime.Today)); @@ -586,13 +586,15 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable public void ShowPrep() { - Bind(null); + Bind(null, "prep"); IsNotesMode = false; IsPrepMode = true; _ = Prep.LoadLastPrepLogIfEmptyAsync(); } - public void Bind(TaskRowViewModel? row) + /// What triggered the selection (); + /// logged with the bind timing so rebind churn can be attributed to a trigger. + public void Bind(TaskRowViewModel? row, string source = "?") { IsNotesMode = false; IsPrepMode = false; @@ -634,10 +636,10 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable // agent output, so the Output tab would open empty -- its work shows up as git changes. SelectedTab = row.IsManual || row.HasInteractiveSession ? "git" : "output"; - _ = BindAsync(row, ct); + _ = BindAsync(row, ct, source); } - private async System.Threading.Tasks.Task BindAsync(TaskRowViewModel row, CancellationToken ct) + private async System.Threading.Tasks.Task BindAsync(TaskRowViewModel row, CancellationToken ct, string source) { var sw = System.Diagnostics.Stopwatch.StartNew(); var ok = false; @@ -735,7 +737,7 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable ok = true; } catch (OperationCanceledException) { } - finally { OperationTiming.Shared.Record("db", "DetailsIsland.BindAsync", sw.Elapsed, ok); } + finally { OperationTiming.Shared.Record("db", $"DetailsIsland.BindAsync:{source}", sw.Elapsed, ok); } } private async System.Threading.Tasks.Task LoadChildOutcomesAsync(string parentTaskId, CancellationToken ct) diff --git a/src/ClaudeDo.Ui/ViewModels/Islands/TasksIslandViewModel.cs b/src/ClaudeDo.Ui/ViewModels/Islands/TasksIslandViewModel.cs index 7df5142a..1ebdc6b7 100644 --- a/src/ClaudeDo.Ui/ViewModels/Islands/TasksIslandViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/Islands/TasksIslandViewModel.cs @@ -47,6 +47,19 @@ public sealed partial class TasksIslandViewModel : ViewModelBase, IDisposable private readonly System.Timers.Timer _reconcileTimer = new(4_000); public event EventHandler? SelectionChanged; + + /// Why the selection last changed. Recorded with the detail-pane bind, so the timing + /// log attributes rebind churn to a trigger instead of leaving an anonymous count. + public string SelectionSource { get; private set; } = "init"; + + /// Sets and tags what triggered it — use this instead of + /// assigning the property, otherwise the bind is logged as "?" . + public void SelectFrom(TaskRowViewModel? row, string source) + { + SelectionSource = source; + SelectedTask = row; + } + public event EventHandler? FocusAddTaskRequested; public event EventHandler? TasksChanged; public event Action? NotesRequested; @@ -57,7 +70,7 @@ public sealed partial class TasksIslandViewModel : ViewModelBase, IDisposable [RelayCommand] private void OpenNotes() { - SelectedTask = null; + SelectFrom(null, "notes"); NotesRequested?.Invoke(); } @@ -359,7 +372,7 @@ public sealed partial class TasksIslandViewModel : ViewModelBase, IDisposable HasCompleted = false; ShowOpenLabel = false; ShowNotesRow = false; - if (listChanged) SelectedTask = null; + if (listChanged) SelectFrom(null, "list-change"); if (list is null) { IsLetClaudeVisible = false; IsQuickClaudeVisible = false; LoadTask = Task.CompletedTask; return; } HeaderTitle = list.Name; @@ -772,7 +785,7 @@ public sealed partial class TasksIslandViewModel : ViewModelBase, IDisposable row.ShowListChip = _currentList?.Kind == ListKind.Virtual; Items.Add(row); Regroup(); - SelectedTask = row; + SelectFrom(row, "new-task"); NewTaskTitle = ""; UpdateSubtitle(); TasksChanged?.Invoke(this, EventArgs.Empty); @@ -1272,7 +1285,7 @@ public sealed partial class TasksIslandViewModel : ViewModelBase, IDisposable row is null ? Task.CompletedTask : SetScheduledForAsync(row, null); [RelayCommand] - private void Select(TaskRowViewModel row) => SelectedTask = row; + private void Select(TaskRowViewModel row) => SelectFrom(row, "row-click"); public async System.Threading.Tasks.Task SelectByIdAsync(string taskId) { @@ -1282,7 +1295,7 @@ public sealed partial class TasksIslandViewModel : ViewModelBase, IDisposable } var row = Items.FirstOrDefault(r => r.Id == taskId); if (row is null) return false; - SelectedTask = row; + SelectFrom(row, "select-by-id"); return true; } diff --git a/src/ClaudeDo.Ui/ViewModels/IslandsShellViewModel.cs b/src/ClaudeDo.Ui/ViewModels/IslandsShellViewModel.cs index a3d2d657..608733e6 100644 --- a/src/ClaudeDo.Ui/ViewModels/IslandsShellViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/IslandsShellViewModel.cs @@ -303,7 +303,7 @@ public sealed partial class IslandsShellViewModel : ViewModelBase, IDisposable _git = git; Lists.SelectionChanged += (_, _) => Tasks.LoadForList(Lists.SelectedList); Lists.SelectionChanged += (_, _) => _ = RefreshStaleWorkerCheckAsync(); - Tasks.SelectionChanged += (_, _) => Details.Bind(Tasks.SelectedTask); + Tasks.SelectionChanged += (_, _) => Details.Bind(Tasks.SelectedTask, Tasks.SelectionSource); Tasks.NotesRequested += () => Details.ShowNotes(); Tasks.PrepRequested += () => Details.ShowPrep(); Tasks.ErrorReported += FlashFooterError; @@ -340,7 +340,7 @@ public sealed partial class IslandsShellViewModel : ViewModelBase, IDisposable Lists.LetClaudeHandleListCommand.Execute(row); }; Details.ErrorReported += FlashFooterError; - Details.CloseDetail = () => Tasks.SelectedTask = null; + Details.CloseDetail = () => Tasks.SelectFrom(null, "close-detail"); Details.DeleteFromList = row => { Tasks.LoadForList(Lists.SelectedList); @@ -354,7 +354,6 @@ public sealed partial class IslandsShellViewModel : ViewModelBase, IDisposable OnPropertyChanged(nameof(ConnectionText)); OnPropertyChanged(nameof(IsOffline)); OnPropertyChanged(nameof(CanOpenWorkerConnectionHelp)); - OpenWorkerConnectionHelpCommand.NotifyCanExecuteChanged(); } }; Worker.WorkerLogReceivedEvent += OnWorkerLogReceived; @@ -390,11 +389,16 @@ public sealed partial class IslandsShellViewModel : ViewModelBase, IDisposable { if (e.PropertyName == nameof(OperationStatus.IsRunning)) CheckForUpdatesCommand.NotifyCanExecuteChanged(); }; - // Fire-and-forget startup check — never block UI. - _ = Task.Run(async () => - { - try { await _updateCheck.CheckNowAsync(CancellationToken.None); } catch { } - }); + // Fire-and-forget startup check — never block UI. Deliberately *not* Task.Run: the check is + // pure async I/O (no sync blocking), and `OperationStatus` writes its observable properties + // on the calling thread. From a threadpool thread the final `ShowIndicator=false` never + // reaches the binding, so the indicator froze on the last tick ("Checking… 00:03"). + _ = StartupUpdateCheckAsync(); + } + + private async Task StartupUpdateCheckAsync() + { + try { await _updateCheck.CheckNowAsync(CancellationToken.None); } catch { } } public void Dispose() @@ -526,8 +530,12 @@ public sealed partial class IslandsShellViewModel : ViewModelBase, IDisposable if (Dialogs is not null) await Dialogs.ShowWorkerConnectionAsync(vm); } - [RelayCommand(CanExecute = nameof(CanOpenWorkerConnectionHelp))] - private Task OpenWorkerConnectionHelp() => OpenWorkerConnectionHelpAsync(); + // The gate lives in the body, not in CanExecute: the footer pill *is* this button, so a + // disabled command renders the "ONLINE" chip greyed out — which reads as a broken connection, + // the opposite of what it says. + [RelayCommand] + private Task OpenWorkerConnectionHelp() => + CanOpenWorkerConnectionHelp ? OpenWorkerConnectionHelpAsync() : Task.CompletedTask; [RelayCommand] private async Task OpenRepoImport() diff --git a/src/ClaudeDo.Ui/Views/Controls/OperationIndicator.axaml b/src/ClaudeDo.Ui/Views/Controls/OperationIndicator.axaml index c05b9b90..07649d2b 100644 --- a/src/ClaudeDo.Ui/Views/Controls/OperationIndicator.axaml +++ b/src/ClaudeDo.Ui/Views/Controls/OperationIndicator.axaml @@ -3,8 +3,13 @@ xmlns:svc="using:ClaudeDo.Ui.Services" xmlns:loc="using:ClaudeDo.Ui.Localization" x:Class="ClaudeDo.Ui.Views.Controls.OperationIndicator" + x:Name="Root" x:DataType="svc:OperationStatus"> + diff --git a/src/ClaudeDo.Ui/Views/Controls/OperationIndicator.axaml.cs b/src/ClaudeDo.Ui/Views/Controls/OperationIndicator.axaml.cs index 93ace2c7..df7c4d6e 100644 --- a/src/ClaudeDo.Ui/Views/Controls/OperationIndicator.axaml.cs +++ b/src/ClaudeDo.Ui/Views/Controls/OperationIndicator.axaml.cs @@ -15,12 +15,21 @@ public partial class OperationIndicator : UserControl set => SetValue(StatusProperty, value); } - public OperationIndicator() => InitializeComponent(); + public OperationIndicator() + { + InitializeComponent(); + IsVisible = false; + } + /// A null `Status` leaves the inner panel's DataContext null, so its `IsVisible="{Binding + /// ShowIndicator}"` can't resolve and falls back to the property default — *visible*. That + /// renders a label-less spinner plus the stalled hint forever. Collapse the whole control + /// instead. Deliberately a direct set, not a binding: no call site binds `IsVisible` on the + /// indicator, and a local value would win over one anyway. protected override void OnPropertyChanged(AvaloniaPropertyChangedEventArgs change) { base.OnPropertyChanged(change); if (change.Property == StatusProperty) - DataContext = change.GetNewValue(); + IsVisible = change.GetNewValue() is not null; } } diff --git a/src/ClaudeDo.Ui/Views/Islands/DetailsIslandView.axaml b/src/ClaudeDo.Ui/Views/Islands/DetailsIslandView.axaml index 7d20b634..12d003f1 100644 --- a/src/ClaudeDo.Ui/Views/Islands/DetailsIslandView.axaml +++ b/src/ClaudeDo.Ui/Views/Islands/DetailsIslandView.axaml @@ -160,7 +160,7 @@ Command="{Binding Prep.PlanDayCommand}" IsEnabled="{Binding Prep.IsPlanDayEnabled}" Content="{loc:Tr details.planDay}"/> - + diff --git a/src/ClaudeDo.Ui/Views/Islands/TaskRowView.axaml.cs b/src/ClaudeDo.Ui/Views/Islands/TaskRowView.axaml.cs index e7f2524c..d1cc05ea 100644 --- a/src/ClaudeDo.Ui/Views/Islands/TaskRowView.axaml.cs +++ b/src/ClaudeDo.Ui/Views/Islands/TaskRowView.axaml.cs @@ -31,7 +31,7 @@ public partial class TaskRowView : UserControl // OnTunnelPointerPressed (TasksIslandView) only selects on the left button, so // right-click needs its own explicit selection before the menu opens. - vm.SelectedTask = row; + vm.SelectFrom(row, "context-menu"); var menu = new ContextMenu { DataContext = row }; diff --git a/src/ClaudeDo.Ui/Views/Islands/TasksIslandView.axaml b/src/ClaudeDo.Ui/Views/Islands/TasksIslandView.axaml index d6932a2c..2d6040a5 100644 --- a/src/ClaudeDo.Ui/Views/Islands/TasksIslandView.axaml +++ b/src/ClaudeDo.Ui/Views/Islands/TasksIslandView.axaml @@ -21,7 +21,7 @@ Foreground="{DynamicResource TextMuteBrush}" Text="{Binding Subtitle}" TextTrimming="CharacterEllipsis"/> - + r.Id == taskId); - if (existing is not null) { s.Tasks.SelectedTask = existing; return; } + if (existing is not null) { s.Tasks.SelectFrom(existing, "jump-to-task"); return; } var tcs = new TaskCompletionSource(); void OnChanged(object? _, NotifyCollectionChangedEventArgs __) @@ -32,7 +32,7 @@ internal static class JumpToTaskHelper { await Task.WhenAny(tcs.Task, Task.Delay(5000)); var row = s.Tasks.Items.FirstOrDefault(r => r.Id == taskId); - if (row is not null) s.Tasks.SelectedTask = row; + if (row is not null) s.Tasks.SelectFrom(row, "jump-to-task"); } finally { diff --git a/src/ClaudeDo.Ui/Views/MainWindow.axaml b/src/ClaudeDo.Ui/Views/MainWindow.axaml index 6e844f2a..b366b6b3 100644 --- a/src/ClaudeDo.Ui/Views/MainWindow.axaml +++ b/src/ClaudeDo.Ui/Views/MainWindow.axaml @@ -83,7 +83,7 @@ - + diff --git a/src/ClaudeDo.Ui/Views/Modals/RepoImportModalView.axaml b/src/ClaudeDo.Ui/Views/Modals/RepoImportModalView.axaml index e607f6e7..61840114 100644 --- a/src/ClaudeDo.Ui/Views/Modals/RepoImportModalView.axaml +++ b/src/ClaudeDo.Ui/Views/Modals/RepoImportModalView.axaml @@ -39,7 +39,7 @@ Command="{Binding ForgetFoldersCommand}" IsVisible="{Binding HasFolders}"/> - + diff --git a/src/ClaudeDo.Ui/Views/Modals/SettingsModalView.axaml b/src/ClaudeDo.Ui/Views/Modals/SettingsModalView.axaml index 01d21c60..3deacbe5 100644 --- a/src/ClaudeDo.Ui/Views/Modals/SettingsModalView.axaml +++ b/src/ClaudeDo.Ui/Views/Modals/SettingsModalView.axaml @@ -281,7 +281,7 @@