diff --git a/src/ClaudeDo.Worker/External/MergeMcpTools.cs b/src/ClaudeDo.Worker/External/MergeMcpTools.cs index 56a48574..9ff74e1d 100644 --- a/src/ClaudeDo.Worker/External/MergeMcpTools.cs +++ b/src/ClaudeDo.Worker/External/MergeMcpTools.cs @@ -744,7 +744,7 @@ public sealed class MergeMcpTools // One line of a unified diff, tagged with the file it belongs to ('-'/'+' only; context lines // are dropped). File is set from the `diff --git a/ b/` header, which (unlike // `+++ b/...`) is present and names the real path even for a deleted file (`+++ /dev/null`). - private readonly record struct DiffLine(string? File, char Sign, string Text); + internal readonly record struct DiffLine(string? File, char Sign, string Text); private static List ParseDiffLines(string diff) { @@ -783,22 +783,64 @@ public sealed class MergeMcpTools @"|[\w<>\[\],.?]+\??\s+(?[A-Za-z_]\w*)\s*(?:\(|\{|=>|;|=))", RegexOptions.Compiled); - private static List ExtractRemovedSymbols(List diffLines) + private static bool IsScannedFile(string? file) + => file is not null && + !file.EndsWith(".axaml", StringComparison.OrdinalIgnoreCase) && + !file.EndsWith(".json", StringComparison.OrdinalIgnoreCase); + + // Names this diff declares on '+' lines, per file. A [ObservableProperty] field `_camelName` / + // `camelName` generates `CamelName`, so it counts as declaring that; the attribute may sit on its + // own '+' line(s) directly above the field. + private static HashSet<(string Name, string File)> ExtractAddedDeclarations(List diffLines) { + var added = new HashSet<(string Name, string File)>(); + var observablePending = false; + foreach (var line in diffLines) + { + if (line.Sign != '+' || !IsScannedFile(line.File)) { observablePending = false; continue; } + + var trimmed = line.Text.Trim(); + var isObservable = trimmed.Contains("[ObservableProperty", StringComparison.Ordinal); + if (trimmed.StartsWith('[') && trimmed.EndsWith(']')) + { + observablePending |= isObservable; + continue; + } + + var match = RemovedDeclarationRegex.Match(line.Text); + if (match.Success) + { + var name = match.Groups["name"].Value; + added.Add((name, line.File!)); + if (isObservable || observablePending) + { + var bare = name.TrimStart('_'); + if (bare.Length > 0) + added.Add((char.ToUpperInvariant(bare[0]) + bare[1..], line.File!)); + } + } + observablePending = false; + } + return added; + } + + internal static List ExtractRemovedSymbols(List diffLines) + { + var added = ExtractAddedDeclarations(diffLines); var seen = new HashSet<(string Name, string File)>(); var result = new List(); foreach (var line in diffLines) { - if (line.Sign != '-' || line.File is null) continue; - if (line.File.EndsWith(".axaml", StringComparison.OrdinalIgnoreCase) || - line.File.EndsWith(".json", StringComparison.OrdinalIgnoreCase)) - continue; + if (line.Sign != '-' || !IsScannedFile(line.File)) continue; var match = RemovedDeclarationRegex.Match(line.Text); if (!match.Success) continue; - if (seen.Add((match.Groups["name"].Value, line.File))) - result.Add(new RemovedSymbolDto(match.Groups["name"].Value, line.File)); + var key = (match.Groups["name"].Value, line.File!); + if (added.Contains(key)) continue; + + if (seen.Add(key)) + result.Add(new RemovedSymbolDto(key.Item1, key.Item2)); } return result; } diff --git a/tests/ClaudeDo.Worker.Tests/External/MergeMcpToolsTests.cs b/tests/ClaudeDo.Worker.Tests/External/MergeMcpToolsTests.cs index 6838b065..d8d10a82 100644 --- a/tests/ClaudeDo.Worker.Tests/External/MergeMcpToolsTests.cs +++ b/tests/ClaudeDo.Worker.Tests/External/MergeMcpToolsTests.cs @@ -1359,6 +1359,47 @@ public sealed class MergeMcpToolsTests : IDisposable Assert.Equal("Bar.cs", warning.ReferencedInFile); } + private static List Diff(string file, params string[] lines) + => lines.Select(l => new MergeMcpTools.DiffLine(file, l[0], l[1..])).ToList(); + + [Fact] + public void ExtractRemovedSymbols_AutoPropertyBecomesObservableField_NotReported() + { + var onOwnLine = Diff("Vm.cs", + "- public bool IsListHandler { get; set; }", + "+ [ObservableProperty]", + "+ private bool _isListHandler;"); + var inline = Diff("Vm.cs", + "- public bool IsListHandler { get; set; }", + "+ [ObservableProperty] private bool isListHandler;"); + + Assert.Empty(MergeMcpTools.ExtractRemovedSymbols(onOwnLine)); + Assert.Empty(MergeMcpTools.ExtractRemovedSymbols(inline)); + } + + [Fact] + public void ExtractRemovedSymbols_ChangedMethodSignatureInSameFile_NotReported() + { + var diff = Diff("Foo.cs", + "- public void Run(int a)", + "+ public void Run(int a, string b)"); + + Assert.Empty(MergeMcpTools.ExtractRemovedSymbols(diff)); + } + + [Fact] + public void ExtractRemovedSymbols_RealRemovalOrCrossFileMove_StillReported() + { + var diff = Diff("Foo.cs", "- private void Gone()", "- public void Moved()") + .Concat(Diff("Other.cs", "+ public void Moved()")).ToList(); + + var removed = MergeMcpTools.ExtractRemovedSymbols(diff); + + Assert.Equal(2, removed.Count); + Assert.Contains(removed, s => s.Name == "Gone" && s.File == "Foo.cs"); + Assert.Contains(removed, s => s.Name == "Moved" && s.File == "Foo.cs"); + } + [Fact] public async Task PreviewMergeSet_TwoTasksAddDifferentJsonKeys_ReportsNoOverlap() {