* feat(mcp): rank context/impact disambiguation candidates and expose kind/file_path hints
The `context` MCP tool already returned `{ status: 'ambiguous', candidates }`
when a name hit multiple symbols, but the candidates were returned in
arbitrary DB order and the only hint it accepted was file_path. The
`impact` tool was worse: when its name resolver found multiple viable
matches it silently picked the first one from a priority UNION, with no
signal back to the caller that a different symbol might have been
intended.
Both failure modes were flagged in issue #470 and reconfirmed in the
comments by a second user who described impact as returning "incorrect
parsing results and meaningless tool calls" in the multi-match case.
Changes:
* Add `resolveSymbolCandidates(repo, query, hints)` private helper on
LocalBackend. Single place that:
- Short-circuits on direct uid (zero-ambiguity)
- Runs the same name-or-qualified-id match as before, with LIMIT 20
(was 10) so the ranker has headroom instead of arbitrary truncation
- Preserves the #480 Class/Constructor preference -- when the only
ambiguity is a Class and its own Constructor, the Class wins
silently
- Scores each candidate (pure TS, no extra DB round-trip): base 0.50,
+0.40 for file_path match, +0.20 for kind match, plus a small
kind-priority tiebreaker (Class > Interface > Function > Method >
Constructor) when no explicit kind hint is given
- Sorts desc by score with stable tiebreakers (shorter filePath,
then lex uid)
- Promotes to a single confident resolve when the top score is
>= 0.95 AND beats the runner-up by >= 0.10 -- lets a strong hint
cut through without forcing the caller through a disambiguation
round-trip
* Rewire `context()` to use the shared helper. Response shape is a
strict superset of today's: candidates gain a `score` field, the
existing `{ uid, name, kind, filePath, line }` keys are preserved so
every downstream consumer (rename, eval-server formatter, etc.) keeps
working. New `kind` input hint accepted.
* Rewire `impact()` to use the shared helper. Now emits the same
`{ status: 'ambiguous', candidates, impactedCount: 0, risk: 'UNKNOWN' }`
shape instead of silent first-pick. New inputs accepted:
`target_uid`, `file_path`, `kind`.
* Update tool schemas in mcp/tools.ts to advertise the new inputs and
describe ranked disambiguation.
Backward compatibility:
The #480 Class/Constructor collapse is preserved and covered by the
existing java-class-impact integration test (still green). The
ambiguous response shape is a strict superset -- `eval-formatters`
unit test that parses the old shape is unchanged and still passes.
`impact` going from silent-first-pick to structured ambiguous is a
semantic improvement that is the entire point of the issue; callers
relying on silent first-pick now get an actionable response.
Scope declined for v1:
module/community hint -- the issue lists it as one of several hints,
but kind + file_path cover the vast majority of disambiguation needs
in practice, and a community-label filter requires an extra graph
query per candidate. Natural v2 follow-up.
Tests: calltool-dispatch.test.ts gains 5 new cases covering file_path
boost, kind hint boost, impact ambiguous shape, impact target_uid
short-circuit, and score field presence on the existing ambiguous
test. Plus the extended assertions on the existing
`context tool returns disambiguation for multiple matches`.
Verification:
npx vitest run test/unit/calltool-dispatch.test.ts -> 64 pass
npx vitest run test/integration/java-class-impact.test.ts -> pass
npm run test:unit -> 3642 pass
(4 pre-existing env failures unchanged: skip-git-cli needs built
dist/, git-utils tmpdir on Windows worktree -- same on main)
npx tsc --noEmit -> clean
Closes#470
* 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
