From 64814d141152fff37eda25498facb9e032bf0129 Mon Sep 17 00:00:00 2001 From: James Maes Date: Fri, 9 Oct 2026 06:32:04 -0500 Subject: [PATCH 01/10] Workspace: error surfaces, stale-table guard, visible-row selection, O(1) progress updates (#30) Verified UX findings: - Load and organization-lookup failures now open a red, dismissable InfoBar with a Retry button (LoadErrorTitle/Message/Open), outside both page states so a failed first load and a failed refresh look the same. StatusText still carries the message for the live region. - Editing the organization after a load marks the table stale (LoadedOrganization/IsTableStale/StaleTableMessage): a warning bar with "Reload repositories" appears and SyncCommand is blocked until a reload, so the chip can no longer promise one org while the rows (and the clone) are another's. - A load that returns nothing shows an empty state naming the organization instead of a blank table with headers. - The header checkbox acts on the VISIBLE rows and reflects them, so "filter by name, then select what you see" is the subset workflow; hidden rows keep their selection and still sync. - Saving Settings no longer resets Quick Sync's parallelism unless the default itself changed (ApplySettings takes an explicit flag). Verified performance findings: - FilteredRepos is maintained incrementally on status transitions: one Insert or RemoveAt at the row's table position (binary search over a row->index map) instead of an O(n) rebuild with a Reset per report. Full rebuilds remain for filter, load, sort, and run start/end. - Per-repo failure logging moved off the UI thread into a progress wrapper that logs on the engine's worker thread before marshaling. - Header select-all and Retry-failed batch their selection changes: one recount and one CanExecute re-query per batch instead of per row. - Sort drops the O(n^2) IndexOf/Move choreography (nothing binds Repos) for a plain O(n) reorder plus the one rebuild of the bound list. Tests: 9 new WorkspaceViewModel tests (error surface open/close, lookup failure, stale/unstale by edit and by reload, visible-row header semantics, one CanExecute per batch, in-place membership updates with no Reset, sorted insertion after a sort, worker-thread failure logging). 353 Engine tests, coverage 100%, FlaUI e2e 8/8. Co-Authored-By: Claude Fable 5.1 --- gclo.Engine.Tests/WorkspaceViewModelTests.cs | 234 +++++++++++++++++ gclo.ViewModels/WorkspaceViewModel.cs | 259 ++++++++++++++++--- gclo/MainWindow.xaml.cs | 23 +- gclo/WorkspacePage.xaml | 67 ++++- gclo/WorkspacePage.xaml.cs | 10 +- 5 files changed, 543 insertions(+), 50 deletions(-) diff --git a/gclo.Engine.Tests/WorkspaceViewModelTests.cs b/gclo.Engine.Tests/WorkspaceViewModelTests.cs index 48623bd..f07627c 100644 --- a/gclo.Engine.Tests/WorkspaceViewModelTests.cs +++ b/gclo.Engine.Tests/WorkspaceViewModelTests.cs @@ -138,6 +138,11 @@ public sealed class WorkspaceViewModelTests Assert.False(vm.IsLoadingRepos); Assert.True(vm.LoadReposCommand.CanExecute(null)); Assert.False(vm.SyncCommand.CanExecute(null)); + // The failure gets a real error surface, not just the caption live region (#30). + Assert.True(vm.LoadErrorOpen); + Assert.Equal("Could not load repositories", vm.LoadErrorTitle); + Assert.Equal("org not found", vm.LoadErrorMessage); + Assert.Null(vm.LoadedOrganization); } // ---------------------------------------------------------------- connect card state @@ -778,8 +783,130 @@ public sealed class WorkspaceViewModelTests Directory.Delete(vm.TargetFolder, recursive: true); } + // ---------------------------------------------------------------- load errors and stale tables (#30) + + [Fact] + public async Task LoadRepos_StartingALoad_ClosesThePreviousLoadError() + { + _lister.ExceptionToThrow = new InvalidOperationException("boom"); + var vm = CreateViewModel(); + vm.Organization = "acme"; + vm.Token = "token-1234567890"; + await WaitUntilAsync(() => vm.StatusText.Length > 0, "org lookup to settle"); + await vm.LoadReposCommand.ExecuteAsync(null); + Assert.True(vm.LoadErrorOpen); + + _lister.ExceptionToThrow = null; + _lister.Repositories = Repos("alpha"); + await vm.LoadReposCommand.ExecuteAsync(null); + + Assert.False(vm.LoadErrorOpen); + Assert.Equal("acme", vm.LoadedOrganization); + Assert.True(vm.HasLoadedRepos); + } + + [Fact] + public async Task OrgLookupFailure_OpensTheErrorSurface_AndASuccessfulLookupClosesIt() + { + _orgs.Handler = (_, _) => Task.FromException>( + new InvalidOperationException("GitHub rejected the token (401). Check the PAT.")); + var vm = CreateViewModel(); + vm.Token = "token-1234567890"; + await WaitUntilAsync(() => vm.LoadErrorOpen, "lookup failure to surface"); + + Assert.Equal("Could not list organizations", vm.LoadErrorTitle); + Assert.Contains("401", vm.LoadErrorMessage); + Assert.Equal(vm.LoadErrorMessage, vm.StatusText); + + _orgs.Handler = (_, _) => Task.FromResult>(["me", "acme"]); + vm.Token = "token-1234567890-fixed"; + await WaitUntilAsync(() => vm.Organizations.Count == 2, "lookup to succeed"); + + Assert.False(vm.LoadErrorOpen); + } + + [Fact] + public async Task EditingTheOrganizationAfterALoad_MarksTheTableStale_AndBlocksSync() + { + var vm = await CreateLoadedViewModelAsync(Repo("alpha")); + Assert.False(vm.IsTableStale); + Assert.Equal("", vm.StaleTableMessage); + Assert.True(vm.SyncCommand.CanExecute(null)); + var raised = new List(); + vm.PropertyChanged += (_, e) => raised.Add(e.PropertyName); + + vm.Organization = "acme-labs"; + + Assert.True(vm.IsTableStale); + Assert.Contains("'acme'", vm.StaleTableMessage); + Assert.Contains("'acme-labs'", vm.StaleTableMessage); + Assert.False(vm.SyncCommand.CanExecute(null), "syncing a stale table would clone the wrong org"); + Assert.Contains(nameof(WorkspaceViewModel.IsTableStale), raised); + Assert.Contains(nameof(WorkspaceViewModel.StaleTableMessage), raised); + + // Putting it back (case and whitespace are not a change) clears the state... + vm.Organization = " ACME "; + Assert.False(vm.IsTableStale); + Assert.True(vm.SyncCommand.CanExecute(null)); + + // ...and so does reloading under the new name. + vm.Organization = "acme-labs"; + await vm.LoadReposCommand.ExecuteAsync(null); + Assert.Equal("acme-labs", vm.LoadedOrganization); + Assert.False(vm.IsTableStale); + Assert.True(vm.SyncCommand.CanExecute(null)); + } + + [Fact] + public void BeforeAnyLoad_TheTableIsNeverStale() + { + var vm = CreateViewModel(); + vm.Organization = "anything"; + Assert.False(vm.IsTableStale); + } + // ---------------------------------------------------------------- selection + [Fact] + public async Task AllSelected_ActsOnVisibleRowsOnly_AndReflectsThem() + { + var vm = await CreateLoadedViewModelAsync(Repo("alpha"), Repo("bravo"), Repo("charlie")); + vm.NameFilter = "al"; // alpha only + Assert.Equal(["alpha"], vm.FilteredRepos.Select(r => r.Name)); + Assert.True(vm.AllSelected); + + vm.AllSelected = false; + + // Only the visible row changed; hidden rows keep their selection and still sync. + Assert.False(vm.Repos[0].IsSelected); + Assert.True(vm.Repos[1].IsSelected); + Assert.True(vm.Repos[2].IsSelected); + Assert.Equal(2, vm.SelectedCount); + Assert.True(vm.SyncCommand.CanExecute(null)); + + // The header reflects the visible set as the filter changes. + vm.NameFilter = ""; + Assert.False(vm.AllSelected); + vm.NameFilter = "br"; + Assert.True(vm.AllSelected); + + vm.AllSelected = true; // no-op on the visible row (already selected) + Assert.False(vm.Repos[0].IsSelected); + } + + [Fact] + public async Task AllSelected_HeaderPush_ReQueriesSyncOncePerBatch() + { + var vm = await CreateLoadedViewModelAsync(Repo("alpha"), Repo("bravo"), Repo("charlie"), Repo("delta")); + int canExecuteRaised = 0; + vm.SyncCommand.CanExecuteChanged += (_, _) => canExecuteRaised++; + + vm.AllSelected = false; + + // One notification for the whole batch, not one per row (#30). + Assert.Equal(1, canExecuteRaised); + } + [Fact] public async Task AllSelected_TogglesEveryRow_AndFollowsItemChanges() { @@ -849,6 +976,26 @@ public sealed class WorkspaceViewModelTests Assert.False(vm.SortDescending); // direction resets on a new column } + [Fact] + public async Task Sort_KeepsFilteredReposInTheNewOrder_AndMembershipUpdatesStillLandInPlace() + { + var vm = await CreateLoadedViewModelAsync(Repo("charlie"), Repo("alpha"), Repo("bravo")); + vm.SortCommand.Execute("Name"); + Assert.Equal(["alpha", "bravo", "charlie"], vm.FilteredRepos.Select(r => r.Name)); + + // A progress-driven membership change after a sort must respect the sorted + // order (the position map is rebuilt by Sort). + vm.Filter = RepoFilter.Failed; + Assert.Empty(vm.FilteredRepos); + _git.CloneHandler = (_, path, _, _, _) => + Path.GetFileName(path) is "charlie" or "alpha" + ? Task.FromException(new InvalidOperationException("boom")) + : Task.CompletedTask; + await vm.SyncCommand.ExecuteAsync(null); + + Assert.Equal(["alpha", "charlie"], vm.FilteredRepos.Select(r => r.Name)); + } + [Fact] public async Task Sort_ByStatus_OrdersByLifecycle_AndIsStable() { @@ -899,6 +1046,93 @@ public sealed class WorkspaceViewModelTests Assert.Equal(["alpha", "bravo", "charlie", "delta"], vm.FilteredRepos.Select(r => r.Name)); } + [Fact] + public async Task ProgressTransitions_UpdateFilteredReposInPlace_WithoutAReset() + { + var vm = await CreateLoadedViewModelAsync(Repo("alpha"), Repo("bravo"), Repo("charlie"), Repo("delta")); + vm.Filter = RepoFilter.Pending; + Assert.Equal(4, vm.FilteredRepos.Count); + var actions = new List(); + vm.FilteredRepos.CollectionChanged += (_, e) => actions.Add(e.Action); + + var gate = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + _git.CloneHandler = async (_, path, _, _, _) => + { + if (Path.GetFileName(path) == "bravo") + { + await gate.Task; + } + }; + var run = vm.SyncCommand.ExecuteAsync(null); + await WaitUntilAsync(() => vm.Repos[1].Status == SyncStatus.Cloning, "bravo to start"); + + // Rows leave Pending one by one as they start: removals, never a Reset. + Assert.DoesNotContain(System.Collections.Specialized.NotifyCollectionChangedAction.Reset, actions); + Assert.DoesNotContain(vm.Repos[1], vm.FilteredRepos); + + gate.SetResult(); + await run; + Assert.Empty(vm.FilteredRepos); + Assert.DoesNotContain(System.Collections.Specialized.NotifyCollectionChangedAction.Reset, actions); + + // And a later transition back into the filtered set inserts in table order. + vm.Filter = RepoFilter.Failed; + _git.CloneHandler = (_, path, _, _, _) => + Path.GetFileName(path) is "delta" or "alpha" + ? Task.FromException(new InvalidOperationException("boom")) + : Task.CompletedTask; + actions.Clear(); + await vm.SyncCommand.ExecuteAsync(null); + + Assert.Equal(["alpha", "delta"], vm.FilteredRepos.Select(r => r.Name)); + Assert.DoesNotContain(System.Collections.Specialized.NotifyCollectionChangedAction.Reset, actions); + } + + [Fact] + public async Task FailedReports_AreLoggedOnTheReportingThread_NotTheUiHandler() + { + var log = new ThreadRecordingLog(); + _lister.Repositories = Repos("alpha"); + var vm = new WorkspaceViewModel(_lister, _git, _orgs, + handler => new SyncProgress(handler), TimeSpan.FromMilliseconds(1), log); + vm.Organization = "acme"; + vm.Token = "token-1234567890"; + await WaitUntilAsync(() => vm.StatusText.Length > 0, "org lookup to settle"); + vm.TargetFolder = Path.Combine(Path.GetTempPath(), "gclo-tests", Guid.NewGuid().ToString("N")); + await vm.LoadReposCommand.ExecuteAsync(null); + int reportingThread = -1; + _git.CloneHandler = (_, _, _, _, _) => + { + reportingThread = Environment.CurrentManagedThreadId; + return Task.FromException(new InvalidOperationException("boom")); + }; + + await vm.SyncCommand.ExecuteAsync(null); + + var entry = Assert.Single(log.Errors, e => e.Message.StartsWith("alpha failed", StringComparison.Ordinal)); + Assert.Contains("boom", entry.Message); + // The engine reports from the thread that ran the git operation; the fake's + // synchronous progress means the log call happened right there, before any + // UI-marshaled handler could have run. + Assert.Equal(reportingThread, entry.Thread); + } + + private sealed class ThreadRecordingLog : IActivityLog + { + public List<(string Message, int Thread)> Errors { get; } = []; + + public void Info(string message) + { + } + + public void Error(string message, Exception? exception = null) + => Errors.Add((message, Environment.CurrentManagedThreadId)); + + public string LogDirectory => ""; + + public string CurrentLogFilePath => ""; + } + [Fact] public async Task Filter_WithNoMatchingRows_LeavesFilteredReposEmpty() { diff --git a/gclo.ViewModels/WorkspaceViewModel.cs b/gclo.ViewModels/WorkspaceViewModel.cs index cd43058..a526ba4 100644 --- a/gclo.ViewModels/WorkspaceViewModel.cs +++ b/gclo.ViewModels/WorkspaceViewModel.cs @@ -27,6 +27,16 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable private readonly Dictionary _itemsByName = new(StringComparer.OrdinalIgnoreCase); private CancellationTokenSource? _orgLoadCts; + /// + /// Position of every row in ; rebuilt on load and sort. Lets + /// progress-driven membership changes find a row's slot in + /// by binary search instead of rebuilding the whole bound list (#30). + /// + private readonly Dictionary _reposIndex = new(); + + /// Mirror of for O(1) membership checks. + private readonly HashSet _filteredSet = new(); + /// Canceled on dispose so an in-flight path recovery dies with the workspace. private readonly CancellationTokenSource _lifetimeCts = new(); @@ -91,6 +101,8 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable MaxConcurrency = AppSettings.DefaultConcurrency; StatusText = ""; ResultMessage = ""; + LoadErrorTitle = ""; + LoadErrorMessage = ""; AllSelected = true; CanEditInputs = true; @@ -187,10 +199,56 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable [ObservableProperty] public partial bool CreateOrgSubfolder { get; set; } - /// Header checkbox state: setting it checks or unchecks every row. + /// + /// Header checkbox state: setting it checks or unchecks every VISIBLE row (the rows + /// in ), and it reads true when every visible row is + /// selected. With no filter active that is every row; with a filter it makes + /// "filter, then select what you see" the subset workflow (#30). Hidden rows keep + /// their selection and still sync. + /// [ObservableProperty] public partial bool AllSelected { get; set; } + /// + /// Organization whose repositories the table currently holds (trimmed, as loaded); + /// null before the first successful load. + /// + [ObservableProperty] + [NotifyPropertyChangedFor(nameof(IsTableStale))] + [NotifyPropertyChangedFor(nameof(StaleTableMessage))] + public partial string? LoadedOrganization { get; set; } + + /// + /// True when was edited after a load: the table still + /// shows the previous organization's rows while the chip and target path describe + /// the new one. Sync is blocked until a reload replaces the rows (#30). + /// + public bool IsTableStale + => LoadedOrganization is not null + && !string.Equals(Organization.Trim(), LoadedOrganization, StringComparison.OrdinalIgnoreCase); + + /// One-line explanation shown while ; empty otherwise. + public string StaleTableMessage + => IsTableStale + ? $"The table still shows '{LoadedOrganization}'. Reload to list '{Organization.Trim()}' before syncing." + : ""; + + /// Title of the error surface for a failed load or organization lookup. + [ObservableProperty] + public partial string LoadErrorTitle { get; set; } + + /// Message of the most recent failed load or organization lookup; empty when none. + [ObservableProperty] + public partial string LoadErrorMessage { get; set; } + + /// + /// True while the load-error surface is showing. Set by a failed load or + /// organization lookup, cleared when the next load starts or a lookup succeeds; the + /// page two-way binds it so the user can dismiss the bar (#30). + /// + [ObservableProperty] + public partial bool LoadErrorOpen { get; set; } + /// True once a sync run has finished (successfully, canceled, or faulted). [ObservableProperty] public partial bool HasCompletedRun { get; set; } @@ -314,7 +372,22 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable partial void OnTargetFolderChanged(string value) => NotifyTargetPathChanged(); - partial void OnOrganizationChanged(string value) => NotifyTargetPathChanged(); + partial void OnOrganizationChanged(string value) + { + NotifyTargetPathChanged(); + OnPropertyChanged(nameof(IsTableStale)); + OnPropertyChanged(nameof(StaleTableMessage)); + SyncCommand.NotifyCanExecuteChanged(); + } + + /// Records a failed load or lookup on the error surface (and the live region). + private void ShowLoadError(string title, string message) + { + StatusText = message; + LoadErrorTitle = title; + LoadErrorMessage = message; + LoadErrorOpen = true; + } partial void OnCreateOrgSubfolderChanged(bool value) => NotifyTargetPathChanged(); @@ -377,6 +450,7 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable } Organization = organizationBeforeRefresh; OnPropertyChanged(nameof(Organization)); + LoadErrorOpen = false; // a lookup that works clears a stale lookup failure // The production lister always lists the token's own account first, so a // single entry means no organizations were visible. StatusText = orgs.Count <= 1 @@ -391,7 +465,7 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable { if (_orgLoadCts == cts) { - StatusText = ex.Message; + ShowLoadError("Could not list organizations", ex.Message); } } finally @@ -442,7 +516,7 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable _syncingSelection = true; try { - foreach (RepoItemViewModel item in Repos) + foreach (RepoItemViewModel item in FilteredRepos) { item.IsSelected = value; } @@ -469,18 +543,21 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable } if (!_syncingSelection) { - // A header push recomputes once after its loop instead of per row. + // A header push (or any other batch) recomputes once after its loop + // instead of per row — including the command re-query, whose CanSync + // scans every row (#30). RecomputeSelectedCount(); UpdateAllSelectedFromItems(); + SyncCommand.NotifyCanExecuteChanged(); } - SyncCommand.NotifyCanExecuteChanged(); } private void RecomputeSelectedCount() => SelectedCount = Repos.Count(r => r.IsSelected); + /// Header state follows the VISIBLE rows; see . private void UpdateAllSelectedFromItems() { - bool all = Repos.All(r => r.IsSelected); + bool all = FilteredRepos.All(r => r.IsSelected); if (AllSelected != all) { _syncingSelection = true; @@ -510,6 +587,7 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable { IsLoadingRepos = true; ResetRunResult(); + LoadErrorOpen = false; try { string organization = Organization.Trim(); @@ -544,14 +622,18 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable SortColumn = null; SortDescending = false; _sampleRepoName = Repos.Count > 0 ? Repos[0].Name : null; - UpdateAllSelectedFromItems(); + RebuildReposIndex(); + LoadedOrganization = organization; HasLoadedRepos = true; StatusText = $"{Repos.Count} repositories loaded."; _log.Info($"Loaded {Repos.Count} repositories for organization '{organization}'."); } catch (Exception ex) { - StatusText = ex.Message; + // The message is good (the listers translate 401/404/rate limits); the + // surface must be too — an InfoBar with Retry, not caption text at the + // bottom of the window (#30). + ShowLoadError("Could not load repositories", ex.Message); _log.Error($"Loading repositories failed: {ex.Message}", ex); } finally @@ -559,7 +641,7 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable IsLoadingRepos = false; NotifyTargetPathChanged(); RecomputeSelectedCount(); - RebuildFilteredRepos(); + RebuildFilteredRepos(); // also re-derives AllSelected from the visible rows SyncCommand.NotifyCanExecuteChanged(); RetryFailedCommand.NotifyCanExecuteChanged(); RecomputeHasFailedRepos(); @@ -572,6 +654,7 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable !IsRunning && !IsLoadingRepos && !IsResolvingPaths + && !IsTableStale && !string.IsNullOrWhiteSpace(TargetFolder) && Repos.Any(r => r.IsSelected); @@ -597,6 +680,7 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable _runSet = selected; TotalCount = selected.Count; RecomputeCompletedCount(); + RebuildFilteredRepos(); // every selected row just went back to Queued string targetRoot = EffectiveTargetRoot; StatusText = $"Syncing {selected.Count} repositories..."; @@ -604,7 +688,9 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable // Constructed on the UI thread: Progress captures the WinUI // SynchronizationContext, so HandleProgress always runs on the UI thread. - var progress = _progressFactory(HandleProgress); + // Failure logging is layered in front of it so the per-entry file I/O + // happens on the engine's worker thread, never on the dispatcher (#30). + var progress = new FailureLoggingProgress(_progressFactory(HandleProgress), _log); var request = new SyncRequest( Organization.Trim(), Token.Trim(), targetRoot, MaxConcurrency); @@ -676,10 +762,23 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable [RelayCommand(CanExecute = nameof(CanRetryFailed))] private async Task RetryFailedAsync() { - foreach (RepoItemViewModel item in Repos) + // One batch: per-row recounts and command re-queries would be O(n^2) (#30). + _syncingSelection = true; + try { - item.IsSelected = item.Status == SyncStatus.Failed; + foreach (RepoItemViewModel item in Repos) + { + item.IsSelected = item.Status == SyncStatus.Failed; + } } + finally + { + _syncingSelection = false; + } + RecomputeSelectedCount(); + UpdateAllSelectedFromItems(); + SyncCommand.NotifyCanExecuteChanged(); + // Executing SyncCommand itself keeps SyncCancelCommand working for retry runs. await SyncCommand.ExecuteAsync(null); } @@ -744,6 +843,7 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable IsResolvingPaths = false; } RecomputeCompletedCount(); + UpdateFilteredMembership(item); RetryFailedCommand.NotifyCanExecuteChanged(); } @@ -779,15 +879,15 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable _ => OrderRepos(r => r.IsArchived, Comparer.Default), }; - // Move (rather than clear + re-add) so list controls keep row containers stable. - for (int target = 0; target < sorted.Count; target++) + // Nothing binds Repos (the table binds FilteredRepos), so a plain reorder is + // enough: O(n) adds instead of the O(n^2) IndexOf/Move choreography that + // used to run here, and the one rebuild below refreshes the bound list (#30). + Repos.Clear(); + foreach (RepoItemViewModel item in sorted) { - int current = Repos.IndexOf(sorted[target]); - if (current != target) - { - Repos.Move(current, target); - } + Repos.Add(item); } + RebuildReposIndex(); RebuildFilteredRepos(); // the table binds FilteredRepos, which mirrors Repos' order } @@ -842,8 +942,8 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable if (report.Status == SyncStatus.Failed) { - _log.Error($"{report.RepoName} failed: {report.Error}"); - // Failures never reach StatusText (which has a live region), so assistive + // Logged already by FailureLoggingProgress on the worker thread. Failures + // never reach StatusText (which has a live region), so assistive // technology hears them only through this explicit channel. AnnouncementRequested?.Invoke($"{report.RepoName} failed. {report.Error}"); } @@ -852,20 +952,39 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable { RecomputeCompletedCount(); } - else if (statusBefore != item.Status && Filter is RepoFilter.Active or RepoFilter.Pending) + + if (statusBefore != item.Status) { - // Status-dependent filters must follow NON-terminal transitions too (a row - // entering Cloning leaves Pending and joins Active immediately); terminal - // ones already rebuild via RecomputeCompletedCount, and the rebuild's - // sequence check keeps redundant calls cheap and scroll-stable. - RebuildFilteredRepos(); + // Status-dependent filters follow every transition (a row entering Cloning + // leaves Pending and joins Active immediately). Only this one row can have + // changed membership, so it is inserted or removed in place — no O(n) + // rebuild, no Reset notification, no scroll jump (#30). + UpdateFilteredMembership(item); + } + } + + /// + /// Logs failed reports on the thread that produced them (the engine's worker), then + /// forwards every report to the UI-marshaled progress. FileActivityLog opens, + /// appends, and closes the file per entry; a mass failure (revoked PAT, dropped + /// network) used to put a thousand of those on the UI thread (#30). + /// + private sealed class FailureLoggingProgress(IProgress inner, IActivityLog log) + : IProgress + { + public void Report(RepoProgress value) + { + if (value.Status == SyncStatus.Failed) + { + log.Error($"{value.RepoName} failed: {value.Error}"); + } + inner.Report(value); } } /// /// Recounts terminal rows within the captured run set (#22) — before the first run - /// the set is empty and the count stays 0. Also the hook for everything that follows - /// a terminal transition: the failed-rows flag and the filtered table. + /// the set is empty and the count stays 0. Also refreshes the failed-rows flag. /// private void RecomputeCompletedCount() { @@ -879,7 +998,6 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable } CompletedCount = completed; RecomputeHasFailedRepos(); - RebuildFilteredRepos(); } private void RecomputeHasFailedRepos() @@ -887,24 +1005,91 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable // ---------------------------------------------------------------- filtering + /// Refreshes the row -> position map after was reloaded or reordered. + private void RebuildReposIndex() + { + _reposIndex.Clear(); + for (int i = 0; i < Repos.Count; i++) + { + _reposIndex[Repos[i]] = i; + } + } + /// /// Rebuilds from , preserving the - /// table's current sort order. A no-op when the visible set is already correct, so - /// list controls keep their scroll position across progress ticks. + /// table's current sort order, then re-derives the header checkbox from the + /// visible rows. Used when the filter, the table contents, or the sort order + /// change; single-row status transitions go through + /// instead. A no-op when the visible set + /// is already correct, so list controls keep their scroll position. /// private void RebuildFilteredRepos() { List desired = Repos.Where(MatchesFilter).ToList(); - if (desired.SequenceEqual(FilteredRepos)) + if (!desired.SequenceEqual(FilteredRepos)) + { + FilteredRepos.Clear(); + _filteredSet.Clear(); + foreach (RepoItemViewModel repo in desired) + { + FilteredRepos.Add(repo); + _filteredSet.Add(repo); + } + } + UpdateAllSelectedFromItems(); + } + + /// + /// Applies one row's current filter match to : inserts it + /// at its table position (binary search on ) when it became + /// visible, removes it when it became hidden, and does nothing when its membership + /// did not change. O(log n) instead of the former O(n) rebuild per progress report. + /// + private void UpdateFilteredMembership(RepoItemViewModel item) + { + bool visible = MatchesFilter(item); + bool wasVisible = _filteredSet.Contains(item); + if (visible == wasVisible) { return; } - FilteredRepos.Clear(); - foreach (RepoItemViewModel repo in desired) + int slot = FindFilteredSlot(item); + if (visible) { - FilteredRepos.Add(repo); + FilteredRepos.Insert(slot, item); + _filteredSet.Add(item); } + else + { + FilteredRepos.RemoveAt(slot); + _filteredSet.Remove(item); + } + UpdateAllSelectedFromItems(); + } + + /// + /// Index in where sits (when + /// present) or belongs (when absent), by its position in . + /// + private int FindFilteredSlot(RepoItemViewModel item) + { + int position = _reposIndex[item]; + int low = 0; + int high = FilteredRepos.Count; + while (low < high) + { + int mid = (low + high) / 2; + if (_reposIndex[FilteredRepos[mid]] < position) + { + low = mid + 1; + } + else + { + high = mid; + } + } + return low; } private bool MatchesFilter(RepoItemViewModel repo) diff --git a/gclo/MainWindow.xaml.cs b/gclo/MainWindow.xaml.cs index 5770fdc..24d5bc7 100644 --- a/gclo/MainWindow.xaml.cs +++ b/gclo/MainWindow.xaml.cs @@ -90,7 +90,7 @@ namespace gclo // Create and show Quick Sync before selecting it, so NavigationView.Content // is never empty regardless of when SelectionChanged fires. ShowWorkspace(Guid.Empty); - ApplySettings(); + ApplySettings(seedQuickSyncConcurrency: true); WorkspaceNav.SelectedItem = QuickSyncNavItem; // AppWindow.Resize takes physical pixels; scale by the monitor DPI so the @@ -134,15 +134,22 @@ namespace gclo } } - private void ApplySettings() + /// + /// Applies the theme and seeds the ad-hoc Quick Sync workspace from the settings + /// defaults (account workspaces carry their own configuration). The target + /// folder is only filled when blank; the parallelism default is pushed only + /// when — at startup, and after a + /// Settings save that actually changed it — so saving an unrelated setting + /// (theme, splash) cannot silently reset a value the user tuned in the + /// workspace's Options flyout (#30). + /// + private void ApplySettings(bool seedQuickSyncConcurrency) { if (Content is FrameworkElement root) { root.RequestedTheme = StatusFormat.ToElementTheme(_settings.Theme); } - // Settings defaults only seed the ad-hoc Quick Sync workspace; account - // workspaces carry their own configuration. if (_workspaces.TryGetValue(Guid.Empty, out var quickSync)) { WorkspaceViewModel viewModel = quickSync.ViewModel; @@ -151,7 +158,10 @@ namespace gclo { viewModel.TargetFolder = _settings.DefaultTargetFolder; } - viewModel.MaxConcurrency = _settings.DefaultMaxConcurrency; + if (seedQuickSyncConcurrency) + { + viewModel.MaxConcurrency = _settings.DefaultMaxConcurrency; + } } } @@ -738,10 +748,11 @@ namespace gclo { XamlRoot = Content.XamlRoot, }; + int concurrencyBefore = _settings.DefaultMaxConcurrency; if (await DialogGuard.ShowAsync(dialog) == ContentDialogResult.Primary) { dialog.ApplyAndSave(); - ApplySettings(); + ApplySettings(seedQuickSyncConcurrency: _settings.DefaultMaxConcurrency != concurrencyBefore); } } diff --git a/gclo/WorkspacePage.xaml b/gclo/WorkspacePage.xaml index e1d3675..591b88c 100644 --- a/gclo/WorkspacePage.xaml +++ b/gclo/WorkspacePage.xaml @@ -31,6 +31,7 @@ + @@ -57,10 +58,29 @@ + + + +