fix(ui): show ParseOp indicator on the planning-mode diff parse too
ParseOp only covered LoadFilesAsync; OnDisplayedDiffChanged's parse (driven by ToggleCombinedAsync and OnSelectedSubtaskChanged) had no visible feedback even though the offload was already correct. Both call sites now go through one ParseOffUiThreadAsync helper (Begin on the UI thread before Task.Run, End via `using` regardless of a stale generation), so the already-visible toolbar indicator covers the planning path without duplicating the begin/offload/end ceremony.
This commit is contained in:
@@ -153,6 +153,16 @@ public sealed partial class DiffViewerViewModel : ViewModelBase
|
||||
public Task LoadAsync(CancellationToken ct = default) =>
|
||||
Mode == DiffViewerMode.Planning ? LoadPlanningAsync() : LoadFilesAsync(ct);
|
||||
|
||||
// Shared by both parse call sites (LoadFilesAsync, ApplyParsedPlanningFilesAsync) so the
|
||||
// ParseOp begin/offload/end ceremony isn't duplicated. Begin runs synchronously on the UI
|
||||
// thread (the caller hasn't awaited anything yet), and the `using` guarantees End runs when
|
||||
// the offloaded parse finishes — even if the caller's own result turns out to be stale.
|
||||
private async Task<T> ParseOffUiThreadAsync<T>(Func<T> parse, CancellationToken ct = default)
|
||||
{
|
||||
using (ParseOp.Begin(Loc.T("ops.diff.parsing")))
|
||||
return await Task.Run(parse, ct);
|
||||
}
|
||||
|
||||
private async Task LoadFilesAsync(CancellationToken ct)
|
||||
{
|
||||
FileTree.Clear();
|
||||
@@ -187,16 +197,11 @@ public sealed partial class DiffViewerViewModel : ViewModelBase
|
||||
return;
|
||||
}
|
||||
|
||||
List<DiffFileViewModel> files;
|
||||
List<DiffTreeNodeViewModel> tree;
|
||||
using (ParseOp.Begin(Loc.T("ops.diff.parsing")))
|
||||
{
|
||||
(files, tree) = await Task.Run(() =>
|
||||
var (files, tree) = await ParseOffUiThreadAsync(() =>
|
||||
{
|
||||
var parsed = UnifiedDiffParser.Parse(raw);
|
||||
return (parsed, DiffTree.Build(parsed));
|
||||
}, ct);
|
||||
}
|
||||
|
||||
foreach (var node in tree)
|
||||
FileTree.Add(node);
|
||||
@@ -285,7 +290,7 @@ public sealed partial class DiffViewerViewModel : ViewModelBase
|
||||
|
||||
private async Task ApplyParsedPlanningFilesAsync(string diff, int generation)
|
||||
{
|
||||
var files = await Task.Run(() => UnifiedDiffParser.Parse(diff));
|
||||
var files = await ParseOffUiThreadAsync(() => UnifiedDiffParser.Parse(diff));
|
||||
if (generation != _planningFilesGeneration) return;
|
||||
|
||||
PlanningFiles.Clear();
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
using System.ComponentModel;
|
||||
using System.IO;
|
||||
using ClaudeDo.Localization;
|
||||
using ClaudeDo.Ui.Localization;
|
||||
@@ -167,6 +168,88 @@ public class DiffViewerViewModelTests
|
||||
Assert.Equal("foo.cs", file.Path);
|
||||
}
|
||||
|
||||
private static string DiffFor(string path) =>
|
||||
$"diff --git a/{path} b/{path}\n--- a/{path}\n+++ b/{path}\n@@ -1,1 +1,1 @@\n-old\n+new\n";
|
||||
|
||||
[Fact]
|
||||
public async Task Planning_SelectSubtask_RunsAndEndsParseOp()
|
||||
{
|
||||
var fake = new FakePlanningWorker
|
||||
{
|
||||
AggregateResult = new[]
|
||||
{
|
||||
new SubtaskDiffDto("s1", "First", "b1", "base1", "head1", null, DiffFor("a.cs")),
|
||||
new SubtaskDiffDto("s2", "Second", "b2", "base2", "head2", null, DiffFor("b.cs")),
|
||||
}
|
||||
};
|
||||
var vm = new DiffViewerViewModel(null!, fake, TestSettings());
|
||||
vm.ConfigurePlanning("plan-1", "main");
|
||||
await vm.LoadAsync();
|
||||
|
||||
// Let the auto-selected first subtask's parse settle before starting the assertion.
|
||||
var settleDeadline = DateTime.UtcNow.AddSeconds(5);
|
||||
while (DateTime.UtcNow < settleDeadline && vm.ParseOp.IsRunning) await Task.Delay(10);
|
||||
|
||||
// ParseOp.Begin runs synchronously (before the offloaded parse's first await point can
|
||||
// return), so this handler is guaranteed to observe IsRunning=true regardless of how
|
||||
// fast the background parse itself completes — no race with the poll loop below.
|
||||
var sawRunning = false;
|
||||
vm.ParseOp.PropertyChanged += (_, e) =>
|
||||
{
|
||||
if (e.PropertyName == nameof(OperationStatus.IsRunning) && vm.ParseOp.IsRunning)
|
||||
sawRunning = true;
|
||||
};
|
||||
|
||||
vm.SelectedSubtask = vm.Subtasks[1];
|
||||
|
||||
var deadline = DateTime.UtcNow.AddSeconds(5);
|
||||
while (DateTime.UtcNow < deadline && vm.ParseOp.IsRunning) await Task.Delay(10);
|
||||
|
||||
Assert.True(sawRunning);
|
||||
Assert.False(vm.ParseOp.IsRunning);
|
||||
var file = Assert.Single(vm.PlanningFiles);
|
||||
Assert.Equal("b.cs", file.Path);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Planning_RapidSubtaskSwitches_ParseOpEndsAndOnlyLatestWins()
|
||||
{
|
||||
var fake = new FakePlanningWorker
|
||||
{
|
||||
AggregateResult = new[]
|
||||
{
|
||||
new SubtaskDiffDto("s1", "First", "b1", "base1", "head1", null, DiffFor("a.cs")),
|
||||
new SubtaskDiffDto("s2", "Second", "b2", "base2", "head2", null, DiffFor("b.cs")),
|
||||
}
|
||||
};
|
||||
var vm = new DiffViewerViewModel(null!, fake, TestSettings());
|
||||
vm.ConfigurePlanning("plan-1", "main");
|
||||
await vm.LoadAsync();
|
||||
|
||||
var settleDeadline = DateTime.UtcNow.AddSeconds(5);
|
||||
while (DateTime.UtcNow < settleDeadline && vm.ParseOp.IsRunning) await Task.Delay(10);
|
||||
|
||||
var sawRunning = false;
|
||||
vm.ParseOp.PropertyChanged += (_, e) =>
|
||||
{
|
||||
if (e.PropertyName == nameof(OperationStatus.IsRunning) && vm.ParseOp.IsRunning)
|
||||
sawRunning = true;
|
||||
};
|
||||
|
||||
// Two changes fired back-to-back without awaiting in between: the first parse is
|
||||
// overtaken by the second before it can write PlanningFiles.
|
||||
vm.SelectedSubtask = vm.Subtasks[1];
|
||||
vm.SelectedSubtask = vm.Subtasks[0];
|
||||
|
||||
var deadline = DateTime.UtcNow.AddSeconds(5);
|
||||
while (DateTime.UtcNow < deadline && vm.ParseOp.IsRunning) await Task.Delay(10);
|
||||
|
||||
Assert.True(sawRunning);
|
||||
Assert.False(vm.ParseOp.IsRunning);
|
||||
var file = Assert.Single(vm.PlanningFiles);
|
||||
Assert.Equal("a.cs", file.Path);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Planning_ToggleCombined_Success_DisplaysUnifiedDiff()
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user