fix: diff-review findings (symbol pairing, noise, FTS scope, atomic save, rebuild guard)

CHANGELOG.md

@@ -20,6 +20,10 @@ finalized in place with a date — no renaming/migration step needed.
 
 - **File metadata is persisted once per index batch instead of once per pass.** The vector and FTS stores already commit per batch, but the file-meta cache was only written at the very end of a refresh pass — a serve restart (or a failed FTS commit) mid-pass left the meta empty while chunks existed, and the next open then wiped the whole store and re-embedded everything again. The meta now follows the stores' own commit granularity, and `save()` writes atomically (tmp + rename) so a crash mid-write can never truncate the file.
 
+### Changed
+
+- **find_impact position queries that resolve to no symbol now return an error with a hint.** Previously they returned an empty references list, which looked like "no callers".
+
 ## [1.5.29]
 
 ### Fixed

helpers/csharp/ReferenceResolver.cs

@@ -136,6 +136,36 @@ public async Task<BatchFindRefsOutput> BatchFindRefsAsync(Solution solution, IRe
         return null;
     }
 
+    /// <summary>
+    /// Picks the workspace counterpart of <paramref name="target"/> among the
+    /// similar-symbol candidates: the exact signature match wins (the first
+    /// candidate may be another overload), a lone candidate is accepted, and
+    /// anything else returns <paramref name="target"/> with
+    /// <paramref name="fellBack"/> set.
+    /// </summary>
+    internal static ISymbol PickWorkspaceSymbol(ISymbol target, IEnumerable<ISymbol> similar, out bool fellBack)
+    {
+        var candidates = similar.ToList();
+        var signature = target.ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat);
+
+        var exact = candidates.FirstOrDefault(c =>
+            c.ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat) == signature);
+        if (exact is not null)
+        {
+            fellBack = false;
+            return exact;
+        }
+
+        if (candidates.Count == 1)
+        {
+            fellBack = false;
+            return candidates[0];
+        }
+
+        fellBack = true;
+        return target;
+    }
+
     /// <summary>
     /// Resolves references for one symbol. Returns the occurrences plus a
     /// warning per survived failure — a caught FindReferencesAsync exception
@@ -157,20 +187,36 @@ public async Task<BatchFindRefsOutput> BatchFindRefsAsync(Solution solution, IRe
             // while the calls were visible to text search). Re-resolve the
             // symbol inside the containing project's workspace compilation
             // first; fall back to the foreign symbol when no counterpart
-            // exists.
+            // exists. Multi-target projects share an AssemblyName, so every
+            // matching project is tried until one yields a counterpart.
             var workspaceSymbol = targetSymbol;
+            var resolved = false;
             var assemblyName = targetSymbol.ContainingAssembly?.Name;
-            var project = solution.Projects.FirstOrDefault(p =>
-                string.Equals(p.AssemblyName, assemblyName, StringComparison.OrdinalIgnoreCase));
-            if (project is not null)
+            foreach (var project in solution.Projects)
             {
+                if (!string.Equals(project.AssemblyName, assemblyName, StringComparison.OrdinalIgnoreCase))
+                    continue;
+
                 var compilation = await project.GetCompilationAsync().ConfigureAwait(false);
-                if (compilation is not null)
-                {
-                    var similar = SymbolFinder
-                        .FindSimilarSymbols(targetSymbol, compilation);
-                    workspaceSymbol = similar.FirstOrDefault() ?? targetSymbol;
-                }
+                if (compilation is null)
+                    continue;
+
+                var picked = PickWorkspaceSymbol(
+                    targetSymbol,
+                    SymbolFinder.FindSimilarSymbols(targetSymbol, compilation),
+                    out var fellBack);
+                if (fellBack)
+                    continue;
+
+                workspaceSymbol = picked;
+                resolved = true;
+                break;
+            }
+
+            if (!resolved)
+            {
+                Console.Error.WriteLine(
+                    $"[INFO] No workspace counterpart for {targetSymbol.Name}; resolving references with the detached symbol");
             }
 
             var refResults = await SymbolFinder

helpers/csharp/WorkspaceNoise.cs

@@ -89,9 +89,10 @@ public static bool IsBuildHostShutdownChatter(string? message)
     /// <summary>
     /// True when a restore or load diagnostic is a NuGet source that cannot
     /// be reached or authenticated ("Unable to load the service index for
-    /// source ...", "Unable to find package X ... in source(s)"). A host
-    /// environment problem, not a code defect; the restore fallback already
-    /// reports the affected projects. Pure and null-tolerant.
+    /// source ..."). A host environment problem, not a code defect; the
+    /// restore fallback already reports the affected projects. "Unable to
+    /// find package" is deliberately not matched: it means a package is
+    /// really missing and the project loads degraded. Pure and null-tolerant.
     /// </summary>
     public static bool IsNuGetSourceFailure(string? message)
     {
@@ -100,7 +101,6 @@ public static bool IsNuGetSourceFailure(string? message)
             return false;
         }
         return message.Contains("Unable to load the service index for source", StringComparison.OrdinalIgnoreCase)
-            || message.Contains("Unable to find package", StringComparison.OrdinalIgnoreCase)
             || message.Contains("Error occurred while getting package vulnerability data", StringComparison.OrdinalIgnoreCase);
     }
 

helpers/csharp/tests/ScipCsharp.Tests.csproj

@@ -44,6 +44,7 @@
     <Compile Include="..\GeneratorSupport.cs" Link="GeneratorSupport.cs" />
     <Compile Include="..\WorkspaceLoad.cs" Link="WorkspaceLoad.cs" />
     <Compile Include="..\WorkspaceNoise.cs" Link="WorkspaceNoise.cs" />
+    <Compile Include="..\ReferenceResolver.cs" Link="ReferenceResolver.cs" />
     <Compile Include="..\RestorePolicy.cs" Link="RestorePolicy.cs" />
     <Compile Include="..\RestoreGate.cs" Link="RestoreGate.cs" />
   </ItemGroup>

helpers/csharp/tests/WorkspaceNoiseTests.cs

@@ -21,11 +21,15 @@ public void NuGetAuditAdvisoryIsFiltered()
 
     [Theory]
     [InlineData("Acme.Core.csproj: Unable to load the service index for source https://pkgs.example/index.json.")]
-    [InlineData("Acme.Core.csproj: Unable to find package Acme.Shared. No packages exist with this id in source(s)")]
     [InlineData("Acme.Core.csproj: Error occurred while getting package vulnerability data: Unable to load the service index")]
     public void NuGetSourceFailureIsDemoted(string message) =>
         Assert.True(WorkspaceNoise.IsNuGetSourceFailure(message));
 
+    [Fact]
+    public void MissingPackageIsNotNuGetSourceFailure() =>
+        Assert.False(WorkspaceNoise.IsNuGetSourceFailure(
+            "Acme.Core.csproj: Unable to find package Acme.Shared. No packages exist with this id in source(s)"));
+
     [Fact]
     public void GenuineLoadFailureIsNotNuGetSourceFailure() =>
         Assert.False(WorkspaceNoise.IsNuGetSourceFailure("The type or namespace name 'Catalog' does not exist"));

helpers/csharp/tests/WorkspaceSymbolPickTests.cs

@@ -0,0 +1,99 @@
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.FindSymbols;
+using ScipCsharp;
+using Xunit;
+
+namespace ScipCsharp.Tests;
+
+/// <summary>
+/// Pins how the detached map symbol is paired with its workspace counterpart:
+/// the exact overload must win over whichever similar symbol comes first, and
+/// an unresolvable pairing must be reported as a fallback, never passed off
+/// as a match.
+/// </summary>
+public class WorkspaceSymbolPickTests
+{
+    private const string Overloads =
+        "class C { public int Add(int a, int b) => a + b; public int Add(int a, int b, int c) => a + b + c; }";
+
+    private const string OtherOverloads =
+        "class C { public int Add(int a) => a; public int Add(long a) => 0; }";
+
+    private static CSharpCompilation Compile(string source) =>
+        CSharpCompilation.Create(
+            "Acme.Core",
+            [CSharpSyntaxTree.ParseText(source)],
+            references: [MetadataReference.CreateFromFile(typeof(object).Assembly.Location)],
+            options: new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary));
+
+    private static List<IMethodSymbol> AddOverloads(Compilation compilation) =>
+        compilation.GetTypeByMetadataName("C")!
+            .GetMembers("Add")
+            .OfType<IMethodSymbol>()
+            .OrderBy(m => m.Parameters.Length)
+            .ToList();
+
+    [Fact]
+    public void PicksTheMatchingOverloadFromTheWorkspaceCompilation()
+    {
+        var detached = Compile(Overloads);
+        var workspace = Compile(Overloads);
+        var target = AddOverloads(detached)[1];
+
+        var similar = SymbolFinder.FindSimilarSymbols(target, workspace);
+        var picked = ReferenceResolver.PickWorkspaceSymbol(target, similar, out var fellBack);
+
+        Assert.False(fellBack);
+        Assert.NotSame(target, picked);
+        Assert.Same(workspace.Assembly, picked.ContainingAssembly);
+        Assert.Equal(3, Assert.IsAssignableFrom<IMethodSymbol>(picked).Parameters.Length);
+    }
+
+    [Fact]
+    public void ExactSignatureWinsOverAnEarlierCandidate()
+    {
+        var target = AddOverloads(Compile(Overloads))[1];
+        var workspaceOverloads = AddOverloads(Compile(Overloads));
+
+        var picked = ReferenceResolver.PickWorkspaceSymbol(target, workspaceOverloads, out var fellBack);
+
+        Assert.False(fellBack);
+        Assert.Same(workspaceOverloads[1], picked);
+    }
+
+    [Fact]
+    public void NoCandidatesFallsBackToTheTarget()
+    {
+        var target = AddOverloads(Compile(Overloads))[1];
+
+        var picked = ReferenceResolver.PickWorkspaceSymbol(target, [], out var fellBack);
+
+        Assert.True(fellBack);
+        Assert.Same(target, picked);
+    }
+
+    [Fact]
+    public void LoneNonExactCandidateIsAccepted()
+    {
+        var target = AddOverloads(Compile(Overloads))[1];
+        var lone = AddOverloads(Compile(OtherOverloads))[0];
+
+        var picked = ReferenceResolver.PickWorkspaceSymbol(target, [lone], out var fellBack);
+
+        Assert.False(fellBack);
+        Assert.Same(lone, picked);
+    }
+
+    [Fact]
+    public void SeveralNonExactCandidatesFallBackToTheTarget()
+    {
+        var target = AddOverloads(Compile(Overloads))[1];
+        var others = AddOverloads(Compile(OtherOverloads));
+
+        var picked = ReferenceResolver.PickWorkspaceSymbol(target, others, out var fellBack);
+
+        Assert.True(fellBack);
+        Assert.Same(target, picked);
+    }
+}

src/cache/file_meta.rs

@@ -6,7 +6,9 @@ use std::fs;
 use std::path::{Path, PathBuf};
 use std::time::SystemTime;
 
-use crate::constants::FILE_META_DB_NAME;
+use crate::constants::{
+    FILE_META_DB_NAME, FILE_META_SAVE_RENAME_ATTEMPTS, FILE_META_SAVE_RENAME_DELAY_MS,
+};
 
 // ─── CANONICAL PATH POLICY ────────────────────────────────────────────────────
 //
@@ -355,8 +357,23 @@ impl FileMetaStore {
         // would trip the empty-meta wipe guard.
         let tmp_path = db_path.join(format!("{}.tmp", Self::FILENAME));
         fs::write(&tmp_path, content)?;
-        fs::rename(&tmp_path, &meta_path)?;
-        Ok(())
+        // Windows: an indexer can briefly hold the target (os error 5/32).
+        let mut attempt = 1;
+        loop {
+            match fs::rename(&tmp_path, &meta_path) {
+                Ok(()) => return Ok(()),
+                Err(e) if attempt >= FILE_META_SAVE_RENAME_ATTEMPTS => {
+                    let _ = fs::remove_file(&tmp_path);
+                    return Err(e.into());
+                }
+                Err(_) => {
+                    attempt += 1;
+                    std::thread::sleep(std::time::Duration::from_millis(
+                        FILE_META_SAVE_RENAME_DELAY_MS,
+                    ));
+                }
+            }
+        }
     }
 
     /// Migrate stored paths to normalized format.

src/chunker/semantic.rs

@@ -1230,6 +1230,72 @@ main "$@"
         );
     }
 
+    #[test]
+    fn test_chunk_php_fallback() {
+        // PHP has a tree-sitter grammar but no extractor, so it falls back
+        // to fallback_chunk (sliding window) like Shell does.
+        let mut chunker = SemanticChunker::new(100, 2000, 10);
+
+        let php_source = r#"<?php
+
+namespace App\Services;
+
+class OrderService
+{
+    public function createOrder(array $items): int
+    {
+        return count($items);
+    }
+
+    public function cancelOrder(int $orderId): bool
+    {
+        return $orderId > 0;
+    }
+}
+
+function formatPrice(float $amount): string
+{
+    return number_format($amount, 2);
+}
+"#;
+
+        let path = Path::new("Service.php");
+
+        // Verify language detection
+        assert_eq!(
+            crate::file::Language::from_path(path),
+            crate::file::Language::Php,
+            ".php files should be detected as PHP language"
+        );
+
+        let chunks = chunker
+            .chunk_semantic(crate::file::Language::Php, path, php_source)
+            .unwrap();
+
+        // Should produce chunks (via fallback)
+        assert!(
+            !chunks.is_empty(),
+            "PHP files should produce chunks via fallback, got 0"
+        );
+
+        // All chunks should be Block kind (fallback chunking)
+        assert!(
+            chunks.iter().all(|c| c.kind == ChunkKind::Block),
+            "Fallback chunks should all be Block kind"
+        );
+
+        // Chunks should cover the PHP content
+        let all_content: String = chunks.iter().map(|c| c.content.as_str()).collect();
+        assert!(
+            all_content.contains("createOrder"),
+            "Chunks should contain createOrder method"
+        );
+        assert!(
+            all_content.contains("cancelOrder"),
+            "Chunks should contain cancelOrder method"
+        );
+    }
+
     #[test]
     fn test_context_breadcrumbs() {
         let mut chunker = SemanticChunker::new(100, 2000, 10);

src/constants.rs

@@ -45,6 +45,12 @@ pub const CONFIG_DIR_NAME: &str = ".codesearch";
 /// Name of the file metadata database
 pub const FILE_META_DB_NAME: &str = "file_meta.json";
 
+/// Rename attempts when publishing the file metadata (AV/Search indexer can hold the file)
+pub const FILE_META_SAVE_RENAME_ATTEMPTS: u32 = 5;
+
+/// Delay between file metadata rename attempts, in milliseconds
+pub const FILE_META_SAVE_RENAME_DELAY_MS: u64 = 100;
+
 /// Subdirectory name for embedding models within the global config dir
 const MODELS_SUBDIR: &str = "models";