diff --git a/src/ClaudeDo.Localization/locales/de.json b/src/ClaudeDo.Localization/locales/de.json index 8901dbed..9d6a4eb6 100644 --- a/src/ClaudeDo.Localization/locales/de.json +++ b/src/ClaudeDo.Localization/locales/de.json @@ -698,6 +698,9 @@ "merging": "Wird zusammengeführt…", "verifying": "Verify-Kommando der Liste läuft… ({0})" }, + "diff": { + "parsing": "Diff wird geparst…" + }, "review": { "submitting": "Wird zur Prüfung eingereicht…", "rejecting": "Wird abgelehnt…", diff --git a/src/ClaudeDo.Localization/locales/en.json b/src/ClaudeDo.Localization/locales/en.json index 1a7d6493..35ce84f3 100644 --- a/src/ClaudeDo.Localization/locales/en.json +++ b/src/ClaudeDo.Localization/locales/en.json @@ -698,6 +698,9 @@ "merging": "Merging…", "verifying": "Running the list's verify command… ({0})" }, + "diff": { + "parsing": "Parsing diff…" + }, "review": { "submitting": "Submitting for review…", "rejecting": "Rejecting…", diff --git a/src/ClaudeDo.Ui/ViewModels/Modals/DiffViewerViewModel.cs b/src/ClaudeDo.Ui/ViewModels/Modals/DiffViewerViewModel.cs index f8003bc6..dfd0e7e9 100644 --- a/src/ClaudeDo.Ui/ViewModels/Modals/DiffViewerViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/Modals/DiffViewerViewModel.cs @@ -98,6 +98,14 @@ public sealed partial class DiffViewerViewModel : ViewModelBase public Action? CloseAction { get; set; } + // Parsing a large diff (UnifiedDiffParser.Parse + DiffTree.Build, both pure CPU work) is + // offloaded to a threadpool thread so it never blocks the UI dispatcher. + public OperationStatus ParseOp { get; } = new(); + + // Guards the fire-and-forget planning parse below against a stale write: a second + // DisplayedDiff change while the first parse is still running must not clobber PlanningFiles. + private int _planningFilesGeneration; + public DiffViewerViewModel(GitService git, IWorkerClient worker, AppSettings settings) { _git = git; @@ -179,8 +187,18 @@ public sealed partial class DiffViewerViewModel : ViewModelBase return; } - var files = UnifiedDiffParser.Parse(raw).ToList(); - foreach (var node in DiffTree.Build(files)) + List files; + List tree; + using (ParseOp.Begin(Loc.T("ops.diff.parsing"))) + { + (files, tree) = await Task.Run(() => + { + var parsed = UnifiedDiffParser.Parse(raw); + return (parsed, DiffTree.Build(parsed)); + }, ct); + } + + foreach (var node in tree) FileTree.Add(node); SelectedNode = DiffTree.FirstLeaf(FileTree); @@ -254,10 +272,24 @@ public sealed partial class DiffViewerViewModel : ViewModelBase partial void OnIsCombinedModeChanged(bool value) => ToggleCombinedCommand.Execute(null); + // DisplayedDiff is set from four places (OnSelectedSubtaskChanged plus three branches of + // ToggleCombinedAsync), and OnSelectedSubtaskChanged is itself a synchronous, non-awaitable + // callback — funneling the offload through this single change handler covers all of them + // without touching every call site. Fire-and-forget is safe because the generation counter + // below discards a parse whose result is no longer the current DisplayedDiff. partial void OnDisplayedDiffChanged(string value) { + var generation = ++_planningFilesGeneration; + _ = ApplyParsedPlanningFilesAsync(value, generation); + } + + private async Task ApplyParsedPlanningFilesAsync(string diff, int generation) + { + var files = await Task.Run(() => UnifiedDiffParser.Parse(diff)); + if (generation != _planningFilesGeneration) return; + PlanningFiles.Clear(); - foreach (var file in UnifiedDiffParser.Parse(value)) + foreach (var file in files) PlanningFiles.Add(file); } diff --git a/src/ClaudeDo.Ui/Views/Modals/DiffViewerView.axaml b/src/ClaudeDo.Ui/Views/Modals/DiffViewerView.axaml index 2788b78a..a6e7e137 100644 --- a/src/ClaudeDo.Ui/Views/Modals/DiffViewerView.axaml +++ b/src/ClaudeDo.Ui/Views/Modals/DiffViewerView.axaml @@ -48,6 +48,7 @@ ToolTip.Tip="{loc:Tr modals.diff.wrapLines}"> + diff --git a/tests/ClaudeDo.Ui.Tests/ViewModels/DiffViewerViewModelTests.cs b/tests/ClaudeDo.Ui.Tests/ViewModels/DiffViewerViewModelTests.cs index 5c27c6b5..98169c56 100644 --- a/tests/ClaudeDo.Ui.Tests/ViewModels/DiffViewerViewModelTests.cs +++ b/tests/ClaudeDo.Ui.Tests/ViewModels/DiffViewerViewModelTests.cs @@ -140,6 +140,33 @@ public class DiffViewerViewModelTests Assert.Equal("DIFF-B", vm.DisplayedDiff); } + [Fact] + public async Task Planning_SelectSubtask_PopulatesPlanningFilesAsync() + { + const string diff = "diff --git a/foo.cs b/foo.cs\n" + + "--- a/foo.cs\n" + + "+++ b/foo.cs\n" + + "@@ -1,1 +1,1 @@\n" + + "-old\n" + + "+new\n"; + var fake = new FakePlanningWorker + { + AggregateResult = new[] { new SubtaskDiffDto("s1", "First", "b1", "base1", "head1", null, diff) } + }; + var vm = new DiffViewerViewModel(null!, fake, TestSettings()); + vm.ConfigurePlanning("plan-1", "main"); + + await vm.LoadAsync(); + + // OnDisplayedDiffChanged offloads the parse via Task.Run and writes PlanningFiles back + // fire-and-forget, so it may not have landed yet when LoadAsync returns. + var deadline = DateTime.UtcNow.AddSeconds(5); + while (DateTime.UtcNow < deadline && vm.PlanningFiles.Count == 0) await Task.Delay(10); + + var file = Assert.Single(vm.PlanningFiles); + Assert.Equal("foo.cs", file.Path); + } + [Fact] public async Task Planning_ToggleCombined_Success_DisplaysUnifiedDiff() {