Merge branch 'main' into fix/esm-path-alias-js-extension

This commit is contained in:
Gergő Magyar 2026-05-13 12:34:45 +01:00 committed by GitHub
commit 837df7ac71
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 137 additions and 16 deletions

View file

@ -50,9 +50,15 @@ export const mergeWithRRF = (
): HybridSearchResult[] => {
const merged = new Map<string, HybridSearchResult>();
// Guard against undefined/null inputs (#1489) — when FTS is unavailable
// in the MCP process, bm25Results can arrive as undefined and the
// for-loop would throw "bm25Results is not iterable".
const safeBm25 = bm25Results ?? [];
const safeSemantic = semanticResults ?? [];
// Process BM25 results
for (let i = 0; i < bm25Results.length; i++) {
const r = bm25Results[i];
for (let i = 0; i < safeBm25.length; i++) {
const r = safeBm25[i];
const rrfScore = 1 / (RRF_K + i + 1); // i+1 because rank starts at 1
merged.set(r.filePath, {
@ -65,8 +71,8 @@ export const mergeWithRRF = (
}
// Process semantic results and merge
for (let i = 0; i < semanticResults.length; i++) {
const r = semanticResults[i];
for (let i = 0; i < safeSemantic.length; i++) {
const r = safeSemantic[i];
const rrfScore = 1 / (RRF_K + i + 1);
const existing = merged.get(r.filePath);
@ -149,6 +155,9 @@ export const formatHybridResults = (results: HybridSearchResult[]): string => {
* Execute BM25 + semantic search and merge with RRF.
* Uses LadybugDB FTS for always-fresh BM25 results (no cached data).
* The semanticSearch function is injected to keep this module environment-agnostic.
*
* When FTS is unavailable (e.g. read-only MCP connection, missing indexes),
* falls back to semantic-only results instead of crashing (#1489).
*/
export const hybridSearch = async (
query: string,
@ -160,8 +169,16 @@ export const hybridSearch = async (
k?: number,
) => Promise<SemanticSearchResult[]>,
): Promise<HybridSearchResult[]> => {
// Use LadybugDB FTS for always-fresh BM25 results
const { results: bm25Results } = await searchFTSFromLbug(query, limit);
// Use LadybugDB FTS for always-fresh BM25 results.
// If FTS fails (e.g. extension not loaded in MCP process), fall back to
// semantic-only search instead of crashing with "bm25Results is not iterable".
let bm25Results: BM25SearchResult[] = [];
try {
const ftsResponse = await searchFTSFromLbug(query, limit);
bm25Results = ftsResponse?.results ?? [];
} catch {
// FTS unavailable — continue with semantic-only search
}
const semanticResults = await semanticSearch(executeQuery, query, limit);
return mergeWithRRF(bm25Results, semanticResults, limit);
};

View file

@ -755,8 +755,10 @@ export class LocalBackend {
timer.time('vector', this.semanticSearch(repo, searchQuery, searchLimit)),
]);
const bm25Results = bm25SearchResult.results;
const ftsUsed = bm25SearchResult.ftsUsed;
// Guard against undefined results (#1489) — when FTS is entirely
// unavailable the search helper may return an unexpected shape.
const bm25Results = bm25SearchResult?.results ?? [];
const ftsUsed = bm25SearchResult?.ftsUsed ?? false;
// Merge via reciprocal rank fusion
timer.start('merge');
@ -774,8 +776,9 @@ export class LocalBackend {
}
}
for (let i = 0; i < semanticResults.length; i++) {
const result = semanticResults[i];
const safeSemanticResults = semanticResults ?? [];
for (let i = 0; i < safeSemanticResults.length; i++) {
const result = safeSemanticResults[i];
const key = result.nodeId || result.filePath;
const rrfScore = 1 / (60 + i);
const existing = scoreMap.get(key);
@ -992,7 +995,17 @@ export class LocalBackend {
query: string,
limit: number,
): Promise<{ results: any[]; ftsUsed: boolean }> {
const { searchFTSFromLbug } = await import('../../core/search/bm25-index.js');
let searchFTSFromLbug;
try {
({ searchFTSFromLbug } = await import('../../core/search/bm25-index.js'));
} catch (err: any) {
// Module import can fail in sandboxed MCP contexts (#1489)
logger.warn(
{ err: err?.message },
'GitNexus: bm25-index.js import failed — falling back to semantic-only',
);
return { results: [], ftsUsed: false };
}
let ftsResponse;
try {
ftsResponse = await searchFTSFromLbug(query, limit, repo.id);
@ -1004,8 +1017,10 @@ export class LocalBackend {
return { results: [], ftsUsed: false };
}
const bm25Results = ftsResponse.results;
const ftsUsed = ftsResponse.ftsAvailable;
// Guard against unexpected response shape (#1489) — ftsResponse.results
// could be undefined when the FTS extension is unavailable in the MCP process.
const bm25Results = ftsResponse?.results ?? [];
const ftsUsed = ftsResponse?.ftsAvailable ?? false;
const results: any[] = [];

View file

@ -216,6 +216,19 @@ describe('LocalBackend.callTool', () => {
expect(result).not.toHaveProperty('warning');
});
it('does not crash when searchFTSFromLbug throws (#1489)', async () => {
const { searchFTSFromLbug } = await import('../../src/core/search/bm25-index.js');
vi.mocked(searchFTSFromLbug).mockRejectedValueOnce(new Error('bm25Results is not iterable'));
(executeParameterized as any).mockResolvedValue([]);
const result = await backend.callTool('query', { query: 'auth' });
// Should still return a valid result shape (semantic-only fallback)
expect(result).toHaveProperty('processes');
expect(result).toHaveProperty('definitions');
expect(result).not.toHaveProperty('error');
});
it('skips vector index query when VECTOR is unsupported by the platform', async () => {
const cap = _captureLogger();
platformMocks.isVectorExtensionSupportedByPlatform.mockReturnValue(false);

View file

@ -1,5 +1,5 @@
/**
* P1 Unit Tests: Hybrid Search (mergeWithRRF)
* P1 Unit Tests: Hybrid Search (mergeWithRRF + hybridSearch)
*
* Tests: mergeWithRRF from hybrid-search.ts
* - BM25-only merge
@ -7,12 +7,20 @@
* - Combined ranking
* - Limit parameter
* - Empty inputs
* - Undefined/null inputs (#1489)
*
* Tests: hybridSearch fallback when FTS unavailable (#1489)
*/
import { describe, it, expect } from 'vitest';
import { mergeWithRRF } from '../../src/core/search/hybrid-search.js';
import { describe, it, expect, vi } from 'vitest';
import { mergeWithRRF, hybridSearch } from '../../src/core/search/hybrid-search.js';
import type { BM25SearchResult } from '../../src/core/search/bm25-index.js';
import type { SemanticSearchResult } from '../../src/core/embeddings/types.js';
vi.mock('../../src/core/search/bm25-index.js', async (importOriginal) => {
const actual = (await importOriginal()) as any;
return { ...actual, searchFTSFromLbug: vi.fn() };
});
let bm25Rank = 0;
function makeBM25(filePath: string, score: number): BM25SearchResult {
return { filePath, score, rank: ++bm25Rank };
@ -123,4 +131,72 @@ describe('mergeWithRRF', () => {
expect(result[0].bm25Score).toBe(15);
expect(result[0].semanticScore).toBeCloseTo(0.7); // 1 - distance
});
// Regression: #1489 — bm25Results is not iterable when FTS unavailable
it('does not crash when bm25Results is undefined (#1489)', () => {
const semantic: SemanticSearchResult[] = [makeSemantic('src/a.ts', 0.1)];
// Force undefined to simulate the crash path where FTS returns unexpected shape
const result = mergeWithRRF(undefined as any, semantic);
expect(result).toHaveLength(1);
expect(result[0].filePath).toBe('src/a.ts');
expect(result[0].sources).toEqual(['semantic']);
});
it('does not crash when semanticResults is undefined (#1489)', () => {
const bm25: BM25SearchResult[] = [makeBM25('src/a.ts', 10)];
const result = mergeWithRRF(bm25, undefined as any);
expect(result).toHaveLength(1);
expect(result[0].filePath).toBe('src/a.ts');
expect(result[0].sources).toEqual(['bm25']);
});
it('does not crash when both inputs are undefined (#1489)', () => {
const result = mergeWithRRF(undefined as any, undefined as any);
expect(result).toHaveLength(0);
});
});
// Regression: #1489 — hybridSearch must not crash when FTS is unavailable
describe('hybridSearch — FTS failure fallback (#1489)', () => {
it('falls back to semantic-only when searchFTSFromLbug throws', async () => {
const { searchFTSFromLbug } = await import('../../src/core/search/bm25-index.js');
vi.mocked(searchFTSFromLbug).mockRejectedValueOnce(new Error('bm25Results is not iterable'));
const mockExecuteQuery = vi.fn().mockResolvedValue([]);
const mockSemanticSearch = vi
.fn()
.mockResolvedValue([makeSemantic('src/semantic-hit.ts', 0.15)]);
const results = await hybridSearch('test query', 10, mockExecuteQuery, mockSemanticSearch);
expect(results).toHaveLength(1);
expect(results[0].filePath).toBe('src/semantic-hit.ts');
expect(results[0].sources).toEqual(['semantic']);
});
it('returns empty when both FTS and semantic return nothing', async () => {
const { searchFTSFromLbug } = await import('../../src/core/search/bm25-index.js');
vi.mocked(searchFTSFromLbug).mockRejectedValueOnce(new Error('FTS unavailable'));
const mockExecuteQuery = vi.fn().mockResolvedValue([]);
const mockSemanticSearch = vi.fn().mockResolvedValue([]);
const results = await hybridSearch('test query', 10, mockExecuteQuery, mockSemanticSearch);
expect(results).toHaveLength(0);
});
it('works normally when FTS succeeds', async () => {
const { searchFTSFromLbug } = await import('../../src/core/search/bm25-index.js');
vi.mocked(searchFTSFromLbug).mockResolvedValueOnce({
results: [{ filePath: 'src/fts-hit.ts', score: 5, rank: 1 }],
ftsAvailable: true,
});
const mockExecuteQuery = vi.fn().mockResolvedValue([]);
const mockSemanticSearch = vi.fn().mockResolvedValue([]);
const results = await hybridSearch('test query', 10, mockExecuteQuery, mockSemanticSearch);
expect(results).toHaveLength(1);
expect(results[0].filePath).toBe('src/fts-hit.ts');
expect(results[0].sources).toEqual(['bm25']);
});
});