fix(mcp): enrich labels from UNION when labels(n)[0] is empty; address review findings

CI on PR #888 caught 13 integration-test failures I did not cover locally:
my resolver refactor collected candidates via `labels(n)[0] AS type`, but
LadybugDB returns an empty string for that projection on certain node
types (most importantly Class). With an empty `type`, impact's downstream
`_runImpactBFS` no longer recognised `symType === 'Class' | 'Interface'`
and stopped seeding Constructor + File nodes into the frontier, so the
"impact(upstream) surfaces the file importer" assertion broke across 11
language fixtures plus 2 OVERRIDES filter tests.

The original impact resolver worked around this by running a prioritised
UNION across Class/Interface/Function/Method/Constructor and picking the
first hit. My refactor dropped that. Fix: keep the simple candidate MATCH
but enrich types afterward via a single scoped UNION query, so every
candidate carries an accurate label for both scoring and downstream
BFS seeding. The UID direct-lookup path is patched the same way.

Also addresses the findings from the senior reviewer on PR #888:

* MIGRATION.md: document the `impact` behavioural change (silent first-
  pick → structured `{ status: 'ambiguous', candidates }`) so downstream
  callers know to branch on `result.status` before reading byDepth/
  summary. `context` is unchanged shape-wise (strict superset).

* New test: `context tool promotes top candidate via scoring when
  multiple rows survive DB pre-filter`. The review flagged that the
  existing file_path test works only because the mock ignores WHERE
  parameters -- the scored-promotion path (top ≥ 0.95 AND gap > 0.09)
  wasn't directly exercised. The new test uses two candidates both in
  App.tsx-containing paths plus a kind hint so promotion is decided by
  scoring, not DB pre-filtering. Also tightened the comment on the
  earlier file_path test to describe the mock vs production divergence
  honestly.

* NIT: added a paragraph explaining why `scored.length >= 2` is kept as
  a defensive guard even though the `normalized.length === 1` early
  return already covers the single-candidate path.

* Integration: two tests in `local-backend-calltool.test.ts` targeted
  `'authenticate'`, which now correctly resolves as ambiguous (two
  Method nodes: AuthService.authenticate and BaseService.authenticate).
  Updated both to pass `file_path: 'src/auth.ts'` so they exercise the
  new disambiguation API and still assert the METHOD_OVERRIDES filtering
  they were originally about.

Edge case fix in the promotion gap check: IEEE754 makes 0.50 + 0.40 +
0.20 - 0.90 = 0.09999999999999998 instead of exactly 0.10, which would
otherwise break the "winner clearly dominates" intent for legitimate
1.00 vs 0.90 cases. Changed `>= 0.10` to `> 0.09`; same user-facing
intent, no floating-point sensitivity.

Verification (all from gitnexus/):
  npx vitest run test/integration/class-impact-all-languages.test.ts
    -> 52 pass (was 11 FAIL on CI before this fix)
  npx vitest run test/integration/local-backend-calltool.test.ts
    -> 18 pass (was 2 FAIL on CI before this fix)
  npx vitest run test/integration/java-class-impact.test.ts
    -> 10 pass (regression guard for #480 preserved)
  npx vitest run test/unit/calltool-dispatch.test.ts
    -> 65 pass (1 new test + 4 from original #470 PR)
  npm run test:unit
    -> 3626 pass, 4 pre-existing env failures unchanged
  npx tsc --noEmit
    -> clean

MIGRATION.md

@@ -1,5 +1,49 @@
 # Migration Guide
 
+## `impact` tool may now return `{ status: 'ambiguous' }` (PR #888, issue #470)
+
+Before this change the `impact` MCP tool silently picked the first match
+when the `target` name hit multiple symbols (Class → Interface → Function
+→ Method → Constructor priority UNION). This often produced analysis for
+the wrong symbol with no signal back to the caller.
+
+After this change, when the resolver finds more than one viable match
+and the caller supplied none of `target_uid` / `file_path` / `kind`,
+`impact` returns a disambiguation response shaped like:
+
+```json
+{
+  "status": "ambiguous",
+  "message": "Found N symbols matching '<target>'. Use target_uid, file_path, or kind to disambiguate.",
+  "target": { "name": "<target>" },
+  "direction": "upstream",
+  "impactedCount": 0,
+  "risk": "UNKNOWN",
+  "candidates": [
+    { "uid": "...", "name": "...", "kind": "Function", "filePath": "...", "line": 42, "score": 0.76 }
+  ]
+}
+```
+
+### Do I need to migrate?
+
+**Probably not, but check for assumptions.** Callers that unconditionally
+read `result.byDepth` / `result.summary` / `result.affected_processes`
+without first checking `result.status` will now see `undefined` in the
+ambiguous case. The fix is to branch on `result.status === 'ambiguous'`
+first and follow up with `target_uid` (preferred) or `file_path` / `kind`.
+
+The `context` tool's ambiguous response is a strict superset of the
+existing shape — every candidate gains a `score` field, no existing field
+has changed. No migration required for `context` callers.
+
+### What happens on re-index?
+
+Nothing — this is an MCP-surface change only. The graph schema, indexer,
+and stored data are untouched.
+
+---
+
 ## OVERRIDES → METHOD_OVERRIDES (PR #642)
 
 The `OVERRIDES` relationship type has been renamed to `METHOD_OVERRIDES` for

gitnexus/src/mcp/local/local-backend.ts

@@ -1070,6 +1070,55 @@ export class LocalBackend {
     return result;
   }
 
+  /**
+   * Patch the `type` field on candidates whose `labels(n)[0]` projection
+   * came back empty — a known LadybugDB behaviour for several node types.
+   *
+   * Uses one scoped UNION query across the five priority labels rather
+   * than per-candidate round-trips, so cost is a single DB call regardless
+   * of how many candidates need enrichment. No-op when every candidate
+   * already has a non-empty type.
+   *
+   * Failures are swallowed: label enrichment is an optimisation for
+   * downstream scoring and #480 Class/Interface BFS seeding; if it fails
+   * the symbol still resolves, just without the kind-priority bonus.
+   */
+  private async enrichCandidateLabels(
+    repo: RepoHandle,
+    candidates: Array<{ id: string; type: string }>,
+  ): Promise<void> {
+    const ids = candidates.filter((c) => c.type === '' && c.id).map((c) => c.id);
+    if (ids.length === 0) return;
+    try {
+      const rows = await executeParameterized(
+        repo.id,
+        `
+        MATCH (n:\`Class\`) WHERE n.id IN $ids RETURN n.id AS id, 'Class' AS label
+        UNION ALL
+        MATCH (n:\`Interface\`) WHERE n.id IN $ids RETURN n.id AS id, 'Interface' AS label
+        UNION ALL
+        MATCH (n:\`Function\`) WHERE n.id IN $ids RETURN n.id AS id, 'Function' AS label
+        UNION ALL
+        MATCH (n:\`Method\`) WHERE n.id IN $ids RETURN n.id AS id, 'Method' AS label
+        UNION ALL
+        MATCH (n:\`Constructor\`) WHERE n.id IN $ids RETURN n.id AS id, 'Constructor' AS label
+        `,
+        { ids },
+      );
+      const labelById = new Map<string, string>();
+      for (const r of rows as any[]) {
+        const id = (r.id ?? r[0]) as string;
+        const label = (r.label ?? r[1]) as string;
+        if (id && label && !labelById.has(id)) labelById.set(id, label);
+      }
+      for (const c of candidates) {
+        if (c.type === '' && labelById.has(c.id)) c.type = labelById.get(c.id) as string;
+      }
+    } catch {
+      /* best-effort — downstream resolvers still work without the label */
+    }
+  }
+
   /**
    * Score a symbol candidate for disambiguation ranking.
    *
@@ -1171,19 +1220,20 @@ export class LocalBackend {
       );
       if (rows.length === 0) return { kind: 'not_found' };
       const r = rows[0] as any;
-      return {
-        kind: 'ok',
-        symbol: {
-          id: r.id ?? r[0],
-          name: r.name ?? r[1],
-          type: r.type ?? r[2] ?? '',
-          filePath: r.filePath ?? r[3],
-          startLine: r.startLine ?? r[4],
-          endLine: r.endLine ?? r[5],
-          ...(include_content ? { content: r.content ?? r[6] } : {}),
-        },
-        resolvedLabel: '',
+      const symbol = {
+        id: (r.id ?? r[0]) as string,
+        name: (r.name ?? r[1]) as string,
+        type: (r.type ?? r[2] ?? '') as string,
+        filePath: (r.filePath ?? r[3]) as string,
+        startLine: (r.startLine ?? r[4]) as number,
+        endLine: (r.endLine ?? r[5]) as number,
+        ...(include_content ? { content: (r.content ?? r[6]) as string | undefined } : {}),
       };
+      // Same LadybugDB label-enrichment as the name-based path: a UID
+      // pointing at a Class must still surface `type: 'Class'` so impact's
+      // Class/Interface BFS seed fires. No-op when type is already set.
+      await this.enrichCandidateLabels(repo, [symbol]);
+      return { kind: 'ok', symbol, resolvedLabel: symbol.type };
     }
 
     if (!name) return { kind: 'not_found' };
@@ -1221,12 +1271,22 @@ export class LocalBackend {
       ...(include_content ? { content: (r.content ?? r[6]) as string | undefined } : {}),
     }));
 
+    // Enrich labels for any candidates where `labels(n)[0]` came back empty.
+    // LadybugDB returns an empty string for that projection on certain node
+    // types (notably Class), which left downstream consumers (impact's
+    // Class/Interface BFS seed, the kind-priority scoring bonus) unable to
+    // distinguish a Class target from "unknown kind". One scoped UNION
+    // across the five priority labels patches the type in-place without
+    // per-candidate round-trips.
+    await this.enrichCandidateLabels(repo, normalized);
+
     // Preserve #480 Class/Constructor collapse: if we have exactly one
     // Class (or Interface) candidate and one Constructor sharing name +
     // filePath, fold into the Class. This used to require a follow-up
     // label query because LadybugDB sometimes returns an empty labels()[0]
-    // for Class nodes — we still fall back to that check when type is
-    // blank on at least one candidate.
+    // for Class nodes — enrichment above handles the empty-type case, but
+    // the `type === 'Constructor'` gate still correctly triggers when a
+    // Class and its Constructor share the name.
     if (!hints.kind && normalized.length > 1) {
       const ambiguousType = normalized.some((s) => s.type === '' || s.type === 'Constructor');
       if (ambiguousType) {
@@ -1273,10 +1333,24 @@ export class LocalBackend {
       return String(a.id).localeCompare(String(b.id));
     });
 
-    // Confident single-result: top score ≥ 0.95 AND beats runner-up by ≥ 0.10.
-    // This lets a very strong file_path/kind hint resolve cleanly instead of
-    // forcing the caller through a disambiguation round-trip.
-    if (scored.length >= 2 && scored[0].score >= 0.95 && scored[0].score - scored[1].score >= 0.1) {
+    // Confident single-result: top score ≥ 0.95 AND beats runner-up by a
+    // clear margin. This lets a very strong file_path/kind hint resolve
+    // cleanly instead of forcing the caller through a disambiguation
+    // round-trip.
+    //
+    // The gap threshold uses `> 0.09` rather than `>= 0.10` on purpose:
+    // IEEE754 addition of the scoring terms (0.50 + 0.40 + 0.20 - 0.90
+    // yields 0.09999999999999998, not exactly 0.10) would otherwise break
+    // the comparison for legitimate "top is 1.00, runner is 0.90" cases.
+    // The intent is a clearly-dominant winner; 0.09 is a large enough
+    // margin to mean that unambiguously.
+    //
+    // The `scored.length >= 2` guard is defensive. The `normalized.length === 1`
+    // early return above already handles the single-candidate path, so in
+    // practice `scored` always has at least two elements by the time we get
+    // here — keeping the guard means changes to the upstream early-return
+    // logic cannot accidentally index out of bounds at `scored[1]`.
+    if (scored.length >= 2 && scored[0].score >= 0.95 && scored[0].score - scored[1].score > 0.09) {
       return { kind: 'ok', symbol: scored[0], resolvedLabel: scored[0].type };
     }
 

gitnexus/test/integration/local-backend-calltool.test.ts

@@ -141,12 +141,20 @@ withTestLbugDB(
       });
 
       it('filters by OVERRIDES only', async () => {
+        // The seed has two Method nodes named 'authenticate' (AuthService's
+        // override and BaseService's base). Per #470, `impact` now returns
+        // a ranked-ambiguous response when the target name hits multiple
+        // symbols, so we must disambiguate with file_path to get the
+        // AuthService override (the one with the outgoing METHOD_OVERRIDES
+        // edge we want to follow downstream).
         const result = await backend.callTool('impact', {
           target: 'authenticate',
+          file_path: 'src/auth.ts',
           direction: 'downstream',
           relationTypes: ['METHOD_OVERRIDES'],
         });
         expect(result).not.toHaveProperty('error');
+        expect(result.status).not.toBe('ambiguous');
         // AuthService.authenticate overrides BaseService.authenticate
         expect(result.impactedCount).toBeGreaterThanOrEqual(1);
         const d1 = result.byDepth[1] || result.byDepth['1'] || [];
@@ -158,12 +166,15 @@ withTestLbugDB(
         // Pass the LEGACY alias 'OVERRIDES' — impactByUid should flatMap-expand
         // it to ['OVERRIDES', 'METHOD_OVERRIDES'] so the METHOD_OVERRIDES edge
         // between BaseService.authenticate and AuthService.authenticate is found.
+        // file_path hint disambiguates the two 'authenticate' methods per #470.
         const result = await backend.callTool('impact', {
           target: 'authenticate',
+          file_path: 'src/auth.ts',
           direction: 'downstream',
           relationTypes: ['OVERRIDES'],
         });
         expect(result).not.toHaveProperty('error');
+        expect(result.status).not.toBe('ambiguous');
         expect(result.impactedCount).toBeGreaterThanOrEqual(1);
         const d1 = result.byDepth[1] || result.byDepth['1'] || [];
         const names = d1.map((d: any) => d.name);

gitnexus/test/unit/calltool-dispatch.test.ts

@@ -282,13 +282,56 @@ describe('LocalBackend.callTool', () => {
       name: 'handleConnect',
       file_path: 'App.tsx',
     });
-    // Single confident match expected (App.tsx hit gets 0.50 base + 0.40
-    // file_path bonus + 0.06 Function priority = 0.96 ≥ 0.95 threshold and
-    // beats the other candidate by > 0.10).
+    // In production, `WHERE n.filePath CONTAINS $filePath` would pre-filter
+    // at the DB layer and only `src/App.tsx` would come back — resolving
+    // via the single-candidate early return rather than via scoring. The
+    // `executeParameterized` mock here returns both rows regardless of the
+    // WHERE clause parameters, so this asserts that the resolver ends up
+    // picking the App.tsx candidate in either case (via mock-relaxed DB
+    // pre-filter or via scoring promotion). The dedicated scoring-promotion
+    // path is covered by the next `it()` block below.
     expect(result.status).toBe('found');
     expect(result.symbol.filePath).toBe('src/App.tsx');
   });
 
+  it('context tool promotes top candidate via scoring when multiple rows survive DB pre-filter (#470)', async () => {
+    // This test explicitly exercises the scored-promotion path (#470
+    // review): both candidates satisfy the file_path hint (so DB
+    // pre-filter would return both in production), and promotion is
+    // determined purely by the combined file_path + kind score.
+    (executeParameterized as any).mockResolvedValue([
+      {
+        id: 'fn:App:1',
+        name: 'render',
+        type: 'Function',
+        filePath: 'src/components/App.tsx',
+        startLine: 10,
+        endLine: 20,
+      },
+      {
+        id: 'method:App:1',
+        name: 'render',
+        type: 'Method',
+        filePath: 'src/pages/App.tsx',
+        startLine: 5,
+        endLine: 15,
+      },
+    ]);
+    const result = await backend.callTool('context', {
+      name: 'render',
+      file_path: 'App.tsx',
+      kind: 'Function',
+    });
+    // Expected scoring:
+    //   Function candidate: 0.50 base + 0.40 file_path + 0.20 kind = 1.10 → cap 1.00
+    //   Method candidate:   0.50 base + 0.40 file_path + 0.00 kind = 0.90
+    // Top score ≥ 0.95 and beats runner-up by 0.10 → confident promotion
+    // to `{ status: 'found' }` with the Function.
+    expect(result.status).toBe('found');
+    expect(result.symbol.filePath).toBe('src/components/App.tsx');
+    expect(result.symbol.kind).toBe('Function');
+  });
+
   it('context tool returns ranked candidates when file_path only partially narrows (#470)', async () => {
     (executeParameterized as any).mockResolvedValue([
       {