diff --git a/gitnexus/src/core/search/bm25-index.ts b/gitnexus/src/core/search/bm25-index.ts index 040440999..92d8ec120 100644 --- a/gitnexus/src/core/search/bm25-index.ts +++ b/gitnexus/src/core/search/bm25-index.ts @@ -17,6 +17,7 @@ export interface BM25SearchResult { filePath: string; score: number; rank: number; + nodeIds?: string[]; } /** @@ -79,7 +80,7 @@ async function queryFTSViaExecutor( indexName: string, query: string, limit: number, -): Promise> { +): Promise> { // Escape single quotes and backslashes to prevent Cypher injection const escapedQuery = query.replace(/\\/g, '\\\\').replace(/'/g, "''"); const cypher = ` @@ -96,6 +97,7 @@ async function queryFTSViaExecutor( return { filePath: node.filePath || '', score: typeof score === 'number' ? score : parseFloat(score) || 0, + nodeId: node.nodeId || node.id || '', }; }); } catch { @@ -167,17 +169,13 @@ export const searchFTSFromLbug = async ( ); } - // Merge results by filePath, summing scores for same file - const merged = new Map(); + // Collect all node scores per filePath to track which nodes actually matched + const fileNodeScores = new Map>(); const addResults = (results: any[]) => { for (const r of results) { - const existing = merged.get(r.filePath); - if (existing) { - existing.score += r.score; - } else { - merged.set(r.filePath, { filePath: r.filePath, score: r.score }); - } + if (!fileNodeScores.has(r.filePath)) fileNodeScores.set(r.filePath, []); + fileNodeScores.get(r.filePath)!.push({ score: r.score, nodeId: r.nodeId }); } }; @@ -187,6 +185,19 @@ export const searchFTSFromLbug = async ( addResults(methodResults); addResults(interfaceResults); + // Sum the top-3 highest-scoring nodes per file and collect their nodeIds. + // Summing all nodes naively inflates scores for files with many mediocre + // matches (e.g. test files) over files with a single highly-relevant symbol. + const merged = new Map(); + for (const [filePath, entries] of fileNodeScores) { + const top3 = [...entries].sort((a, b) => b.score - a.score).slice(0, 3); + merged.set(filePath, { + filePath, + score: top3.reduce((acc, e) => acc + e.score, 0), + nodeIds: top3.map((e) => e.nodeId).filter((id) => id), + }); + } + // Sort by score descending and add rank const sorted = Array.from(merged.values()) .sort((a, b) => b.score - a.score) @@ -196,5 +207,6 @@ export const searchFTSFromLbug = async ( filePath: r.filePath, score: r.score, rank: index + 1, + nodeIds: r.nodeIds, })); }; diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index 129cda678..3414f5f5b 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -851,16 +851,30 @@ export class LocalBackend { for (const bm25Result of bm25Results) { const fullPath = bm25Result.filePath; try { - const symbols = await executeParameterized( - repo.id, - ` - MATCH (n) - WHERE n.filePath = $filePath - RETURN n.id AS id, n.name AS name, labels(n)[0] AS type, n.filePath AS filePath, n.startLine AS startLine, n.endLine AS endLine - LIMIT 3 - `, - { filePath: fullPath }, - ); + // Prefer direct nodeId lookup (exact FTS-matched nodes) over filePath fallback. + // Without this, LIMIT 3 on filePath returns arbitrary symbols rather than + // the nodes that actually scored highest in the BM25 index. + const nodeIds = bm25Result.nodeIds?.length ? bm25Result.nodeIds : null; + const symbols = nodeIds + ? await executeParameterized( + repo.id, + ` + MATCH (n) + WHERE n.id IN $nodeIds + RETURN n.id AS id, n.name AS name, labels(n)[0] AS type, n.filePath AS filePath, n.startLine AS startLine, n.endLine AS endLine + `, + { nodeIds }, + ) + : await executeParameterized( + repo.id, + ` + MATCH (n) + WHERE n.filePath = $filePath + RETURN n.id AS id, n.name AS name, labels(n)[0] AS type, n.filePath AS filePath, n.startLine AS startLine, n.endLine AS endLine + LIMIT 3 + `, + { filePath: fullPath }, + ); if (symbols.length > 0) { for (const sym of symbols) { diff --git a/gitnexus/test/unit/bm25-search.test.ts b/gitnexus/test/unit/bm25-search.test.ts index 1d8be757f..466df3395 100644 --- a/gitnexus/test/unit/bm25-search.test.ts +++ b/gitnexus/test/unit/bm25-search.test.ts @@ -1,6 +1,14 @@ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; import { searchFTSFromLbug, type BM25SearchResult } from '../../src/core/search/bm25-index.js'; +vi.mock('../../src/core/lbug/lbug-adapter.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + queryFTS: vi.fn().mockResolvedValue([]), + }; +}); + describe('BM25 search', () => { describe('searchFTSFromLbug', () => { it('returns empty array when LadybugDB is not initialized', async () => { @@ -32,5 +40,133 @@ describe('BM25 search', () => { expect(result.score).toBe(1.5); expect(result.rank).toBe(1); }); + + it('accepts optional nodeIds field', () => { + const result: BM25SearchResult = { + filePath: 'src/index.ts', + score: 1.5, + rank: 1, + nodeIds: ['func:id1', 'func:id2'], + }; + expect(result.nodeIds).toEqual(['func:id1', 'func:id2']); + }); + }); + + describe('score aggregation', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('sums only top-3 scoring nodes per file when more than 3 match', async () => { + const { queryFTS } = await import('../../src/core/lbug/lbug-adapter.js'); + // File table: empty; Function table: 5 hits for the same file; rest: empty + vi.mocked(queryFTS) + .mockResolvedValueOnce([]) // File + .mockResolvedValueOnce([ + // Function — 5 hits, scores 10/9/8/7/6 + { filePath: 'src/views.py', score: 10, nodeId: 'func:node1', name: 'get_queryset' }, + { filePath: 'src/views.py', score: 9, nodeId: 'func:node2', name: 'post' }, + { filePath: 'src/views.py', score: 8, nodeId: 'func:node3', name: 'delete' }, + { filePath: 'src/views.py', score: 7, nodeId: 'func:node4', name: 'patch' }, + { filePath: 'src/views.py', score: 6, nodeId: 'func:node5', name: 'put' }, + ]) + .mockResolvedValueOnce([]) // Class + .mockResolvedValueOnce([]) // Method + .mockResolvedValueOnce([]); // Interface + + const results = await searchFTSFromLbug('queryset'); + + expect(results).toHaveLength(1); + expect(results[0].filePath).toBe('src/views.py'); + // Only top-3 scores (10+9+8=27), not naive sum of all 5 (10+9+8+7+6=40) + expect(results[0].score).toBe(27); + expect(results[0].nodeIds).toEqual(['func:node1', 'func:node2', 'func:node3']); + }); + + it('propagates nodeIds for files with fewer than 3 matching nodes', async () => { + const { queryFTS } = await import('../../src/core/lbug/lbug-adapter.js'); + vi.mocked(queryFTS) + .mockResolvedValueOnce([]) // File + .mockResolvedValueOnce([ + // Function — 2 hits + { filePath: 'src/models.py', score: 5, nodeId: 'func:m1', name: 'save' }, + { filePath: 'src/models.py', score: 3, nodeId: 'func:m2', name: 'delete' }, + ]) + .mockResolvedValueOnce([]) // Class + .mockResolvedValueOnce([]) // Method + .mockResolvedValueOnce([]); // Interface + + const results = await searchFTSFromLbug('model'); + + expect(results).toHaveLength(1); + expect(results[0].score).toBe(8); // 5+3 + expect(results[0].nodeIds).toEqual(['func:m1', 'func:m2']); + }); + + it('filters out empty nodeIds', async () => { + const { queryFTS } = await import('../../src/core/lbug/lbug-adapter.js'); + vi.mocked(queryFTS) + .mockResolvedValueOnce([]) // File + .mockResolvedValueOnce([ + // Function — nodes with no id + { filePath: 'src/utils.py', score: 5, nodeId: '', name: 'helper' }, + { filePath: 'src/utils.py', score: 3, nodeId: '', name: 'util' }, + ]) + .mockResolvedValueOnce([]) // Class + .mockResolvedValueOnce([]) // Method + .mockResolvedValueOnce([]); // Interface + + const results = await searchFTSFromLbug('util'); + + expect(results).toHaveLength(1); + expect(results[0].nodeIds).toEqual([]); + }); + + it('merges hits across multiple index tables for the same file', async () => { + const { queryFTS } = await import('../../src/core/lbug/lbug-adapter.js'); + vi.mocked(queryFTS) + .mockResolvedValueOnce([ + // File table + { filePath: 'src/auth.py', score: 4, nodeId: 'file:auth', name: 'auth.py' }, + ]) + .mockResolvedValueOnce([ + // Function table + { filePath: 'src/auth.py', score: 9, nodeId: 'func:login', name: 'login' }, + ]) + .mockResolvedValueOnce([ + // Class table + { filePath: 'src/auth.py', score: 7, nodeId: 'cls:User', name: 'User' }, + ]) + .mockResolvedValueOnce([]) // Method + .mockResolvedValueOnce([]); // Interface + + const results = await searchFTSFromLbug('auth'); + + expect(results).toHaveLength(1); + // All 3 hits (scores 9+7+4=20) — each from a different table, all top-3 + expect(results[0].score).toBe(20); + expect(results[0].nodeIds).toEqual(['func:login', 'cls:User', 'file:auth']); + }); + + it('ranks files by aggregated score descending', async () => { + const { queryFTS } = await import('../../src/core/lbug/lbug-adapter.js'); + vi.mocked(queryFTS) + .mockResolvedValueOnce([]) // File + .mockResolvedValueOnce([ + // Function — hits across two files + { filePath: 'src/low.py', score: 2, nodeId: 'func:a', name: 'a' }, + { filePath: 'src/high.py', score: 9, nodeId: 'func:b', name: 'b' }, + ]) + .mockResolvedValueOnce([]) // Class + .mockResolvedValueOnce([]) // Method + .mockResolvedValueOnce([]); // Interface + + const results = await searchFTSFromLbug('fn'); + + expect(results[0].filePath).toBe('src/high.py'); + expect(results[1].filePath).toBe('src/low.py'); + expect(results[0].rank).toBe(1); + expect(results[1].rank).toBe(2); + }); }); });