mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-28 06:22:22 +00:00
fix(analyze): degrade FTS search instead of aborting analyze on index-build failure (#2548)
* fix(analyze): degrade FTS search instead of aborting analyze on index-build failure createSearchFTSIndexes re-tokenizes every stored row on every analyze run (full or incremental). A native LadybugDB tokenizer error on a single pre-existing row (e.g. "Failed calling LOWER: Invalid UTF-8") previously propagated uncaught out of run-analyze.ts's main FTS phase, discarding an otherwise-successful run's graph/embeddings work every time analyze ran thereafter. Add buildSearchIndexesOrDegrade(), which catches build/verify failures and lets analyze finish with keyword search degraded for that run instead — mirroring the existing sibling degrade path for a missing FTS extension. The dedicated --repair-fts path is untouched and still fails loudly. Fixes #2544, #2546. * fix(analyze): keep capabilities.fts/ftsSkipped honest when index build degrades ftsSkipped and capabilities.fts.status were keyed only on ftsAvailable (extension loaded), which the new degrade path leaves true even when the index build itself failed. Track that outcome in ftsReady and use it for both, and update run-analyze-fts-repair.test.ts's coverage of this path from asserting the old throw to asserting the new degrade contract (ftsSkipped, log message, meta.json capabilities.fts.status).
This commit is contained in:
parent
8fc4d4d663
commit
fc7ec7790f
4 changed files with 120 additions and 25 deletions
|
|
@ -36,6 +36,7 @@ import {
|
|||
} from './lbug/lbug-adapter.js';
|
||||
import { escapeCypherString } from './lbug/cypher-escape.js';
|
||||
import {
|
||||
buildSearchIndexesOrDegrade,
|
||||
createSearchFTSIndexes,
|
||||
initialiseSearchFTSStemmer,
|
||||
verifySearchFTSIndexes,
|
||||
|
|
@ -1623,8 +1624,17 @@ export async function runFullAnalysis(
|
|||
const ftsAvailable = await loadFTSExtension(undefined, {
|
||||
policy: resolveAnalyzeInstallPolicy(),
|
||||
});
|
||||
// Tracks whether search indexes actually ended up usable this run — starts
|
||||
// as ftsAvailable (extension loaded) but flips to false below when the
|
||||
// build/verify step itself fails, so capabilities.fts.status / ftsSkipped
|
||||
// stay honest even though that failure no longer aborts the whole analyze.
|
||||
let ftsReady = ftsAvailable;
|
||||
if (ftsAvailable) {
|
||||
await createSearchFTSIndexes({
|
||||
// Degrade rather than throw: createSearchFTSIndexes re-tokenizes every
|
||||
// stored row on every run, so a native tokenizer error on a single
|
||||
// pre-existing row (#2544/#2546) must not discard this run's otherwise-
|
||||
// successful graph/embeddings work — only keyword search degrades.
|
||||
const ftsResult = await buildSearchIndexesOrDegrade(executeQuery, {
|
||||
onIndexStart: options.verbose
|
||||
? (table, indexName) => log(`FTS: creating ${table}.${indexName}`)
|
||||
: undefined,
|
||||
|
|
@ -1632,14 +1642,16 @@ export async function runFullAnalysis(
|
|||
? (table, indexName) => log(`FTS: ready ${table}.${indexName}`)
|
||||
: undefined,
|
||||
});
|
||||
const missingIndexNames = await verifySearchFTSIndexes(executeQuery);
|
||||
if (missingIndexNames.length > 0) {
|
||||
throw new Error(
|
||||
`FTS verification failed - missing indexes after analyze: ${missingIndexNames.join(', ')}. ` +
|
||||
'Check FTS extension availability, then retry `gitnexus analyze --force` for a full rebuild.',
|
||||
if (ftsResult.ok) {
|
||||
progress('fts', 90, 'Search indexes ready');
|
||||
} else {
|
||||
ftsReady = false;
|
||||
log(
|
||||
`FTS index build failed (${ftsResult.error}) — keyword search degraded this run. ` +
|
||||
'Graph and embeddings analysis completed successfully. Run `gitnexus analyze --repair-fts` to retry.',
|
||||
);
|
||||
progress('fts', 90, 'Search indexes skipped (build failed)');
|
||||
}
|
||||
progress('fts', 90, 'Search indexes ready');
|
||||
} else {
|
||||
// For a missing runtime dependency (#2374) the file is present, so the
|
||||
// generic "install it with network access" tail in FTS_UNAVAILABLE_MESSAGE
|
||||
|
|
@ -2036,7 +2048,7 @@ export async function runFullAnalysis(
|
|||
// meta.json / `gitnexus doctor` honest about degraded search.
|
||||
fts: {
|
||||
provider: 'ladybugdb-fts',
|
||||
status: ftsAvailable ? runtimeCapabilities.fts : 'unavailable',
|
||||
status: ftsReady ? runtimeCapabilities.fts : 'unavailable',
|
||||
},
|
||||
vectorSearch: {
|
||||
provider: effectiveSemanticMode === 'vector-index' ? 'ladybugdb-vector' : 'exact-scan',
|
||||
|
|
@ -2216,7 +2228,7 @@ export async function runFullAnalysis(
|
|||
repoPath,
|
||||
stats: meta.stats,
|
||||
pipelineResult,
|
||||
ftsSkipped: !ftsAvailable,
|
||||
ftsSkipped: !ftsReady,
|
||||
isPrimaryBranch: !placement.branch,
|
||||
};
|
||||
} catch (err) {
|
||||
|
|
|
|||
|
|
@ -178,3 +178,34 @@ export async function verifySearchFTSIndexes(
|
|||
}
|
||||
return missing;
|
||||
}
|
||||
|
||||
export interface BuildSearchIndexesResult {
|
||||
ok: boolean;
|
||||
error?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Build + verify FTS indexes, catching any failure instead of letting it
|
||||
* propagate. `createSearchFTSIndexes` re-tokenizes every stored row on every
|
||||
* analyze run (see the `ponytail:` comment above) — a native LadybugDB
|
||||
* tokenizer error on a single pre-existing row (e.g. a "Failed calling
|
||||
* LOWER: Invalid UTF-8", #2544/#2546) must not discard an otherwise-
|
||||
* successful analyze's graph/embeddings work. The caller degrades keyword
|
||||
* search for this run instead, mirroring the existing FTS-extension-
|
||||
* unavailable degrade path in `run-analyze.ts`.
|
||||
*/
|
||||
export async function buildSearchIndexesOrDegrade(
|
||||
executeQuery: (cypher: string) => Promise<unknown[]>,
|
||||
options?: CreateSearchFTSIndexesOptions,
|
||||
): Promise<BuildSearchIndexesResult> {
|
||||
try {
|
||||
await createSearchFTSIndexes(options);
|
||||
const missing = await verifySearchFTSIndexes(executeQuery);
|
||||
if (missing.length > 0) {
|
||||
return { ok: false, error: `missing indexes after build: ${missing.join(', ')}` };
|
||||
}
|
||||
return { ok: true };
|
||||
} catch (e) {
|
||||
return { ok: false, error: e instanceof Error ? e.message : String(e) };
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -15,9 +15,18 @@ vi.mock('../../src/core/lbug/lbug-adapter.js', () => ({
|
|||
),
|
||||
}));
|
||||
|
||||
const { createSearchFTSIndexes, getSearchFTSStemmer, initialiseSearchFTSStemmer } =
|
||||
await import('../../src/core/search/fts-indexes.js');
|
||||
const {
|
||||
buildSearchIndexesOrDegrade,
|
||||
createSearchFTSIndexes,
|
||||
getSearchFTSStemmer,
|
||||
initialiseSearchFTSStemmer,
|
||||
} = await import('../../src/core/search/fts-indexes.js');
|
||||
const { FTS_INDEXES } = await import('../../src/core/search/fts-schema.js');
|
||||
const { createFTSIndex } = await import('../../src/core/lbug/lbug-adapter.js');
|
||||
|
||||
/** SHOW_INDEXES rows covering every configured FTS index's expected properties. */
|
||||
const fullCoverageRows = () =>
|
||||
FTS_INDEXES.map((i) => ({ index_name: i.indexName, property_names: [...i.properties] }));
|
||||
|
||||
afterEach(() => {
|
||||
calls.length = 0;
|
||||
|
|
@ -65,6 +74,37 @@ describe('createSearchFTSIndexes', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('buildSearchIndexesOrDegrade', () => {
|
||||
it('returns ok:true when every index builds and verifies (#2544/#2546)', async () => {
|
||||
const executeQuery = vi.fn(async () => fullCoverageRows());
|
||||
|
||||
const result = await buildSearchIndexesOrDegrade(executeQuery);
|
||||
|
||||
expect(result).toEqual({ ok: true });
|
||||
});
|
||||
|
||||
it('returns ok:false instead of throwing when a single index build rejects (#2544/#2546)', async () => {
|
||||
vi.mocked(createFTSIndex).mockRejectedValueOnce(
|
||||
new Error('Runtime exception: Failed calling LOWER: Invalid UTF-8.'),
|
||||
);
|
||||
const executeQuery = vi.fn(async () => fullCoverageRows());
|
||||
|
||||
const result = await buildSearchIndexesOrDegrade(executeQuery);
|
||||
|
||||
expect(result.ok).toBe(false);
|
||||
expect(result.error).toContain('Invalid UTF-8');
|
||||
});
|
||||
|
||||
it('returns ok:false when verification finds a missing index, without throwing', async () => {
|
||||
const executeQuery = vi.fn(async () => fullCoverageRows().slice(1));
|
||||
|
||||
const result = await buildSearchIndexesOrDegrade(executeQuery);
|
||||
|
||||
expect(result.ok).toBe(false);
|
||||
expect(result.error).toContain('missing indexes');
|
||||
});
|
||||
});
|
||||
|
||||
describe('getSearchFTSStemmer', () => {
|
||||
it('defaults to porter when unset', () => {
|
||||
expect(getSearchFTSStemmer()).toBe('porter');
|
||||
|
|
|
|||
|
|
@ -459,7 +459,11 @@ describe('runFullAnalysis FTS repair and verification failure paths', () => {
|
|||
}
|
||||
});
|
||||
|
||||
it('fails full analyze when FTS verification reports missing indexes after creation', async () => {
|
||||
it('degrades gracefully (no throw, warns, ftsSkipped) when FTS verification reports missing indexes after creation (#2544/#2546)', async () => {
|
||||
// A native tokenizer error on one pre-existing row (the #2544/#2546
|
||||
// failure mode) must not abort an otherwise-successful full analyze —
|
||||
// it degrades keyword search for this run instead, same contract as the
|
||||
// FTS-extension-unavailable sibling test below.
|
||||
vi.doMock('../../src/core/lbug/lbug-adapter.js', () => ({
|
||||
initLbug: vi.fn(async () => undefined),
|
||||
loadGraphToLbug: vi.fn(async () => undefined),
|
||||
|
|
@ -480,34 +484,42 @@ describe('runFullAnalysis FTS repair and verification failure paths', () => {
|
|||
deleteAllCommunitiesAndProcesses: vi.fn(async () => undefined),
|
||||
queryImporters: vi.fn(async () => []),
|
||||
queryImportersBatch: vi.fn(async () => []),
|
||||
// FTS extension loads → analyze proceeds to create + verify indexes.
|
||||
// FTS extension loads → analyze proceeds to build + verify indexes.
|
||||
loadFTSExtension: vi.fn(async () => true),
|
||||
}));
|
||||
vi.doMock('../../src/core/search/fts-indexes.js', () => ({
|
||||
initialiseSearchFTSStemmer: vi.fn(() => 'porter'),
|
||||
createSearchFTSIndexes: vi.fn(async () => undefined),
|
||||
verifySearchFTSIndexes: vi.fn(async () => ['Function.function_fts']),
|
||||
buildSearchIndexesOrDegrade: vi.fn(async () => ({
|
||||
ok: false,
|
||||
error: 'missing indexes after build: Function.function_fts',
|
||||
})),
|
||||
}));
|
||||
vi.doMock('../../src/core/ingestion/pipeline.js', () => ({
|
||||
runPipelineFromRepo: vi.fn(async (repoPath: string) => ({
|
||||
repoPath,
|
||||
// Full-analyze path only needs `forEachNode` before the FTS verify guard.
|
||||
// Full-analyze path only needs `forEachNode` before the FTS phase.
|
||||
graph: { forEachNode: () => undefined },
|
||||
})),
|
||||
}));
|
||||
|
||||
const tmpRepo = await createTempDir('gitnexus-run-analyze-full-verify-fail-');
|
||||
try {
|
||||
const logs: string[] = [];
|
||||
const { runFullAnalysis } = await import('../../src/core/run-analyze.js');
|
||||
await expect(
|
||||
runFullAnalysis(
|
||||
tmpRepo.dbPath,
|
||||
{ force: true },
|
||||
{
|
||||
onProgress: () => {},
|
||||
},
|
||||
),
|
||||
).rejects.toThrow(/FTS verification failed - missing indexes after analyze/i);
|
||||
const result = await runFullAnalysis(
|
||||
tmpRepo.dbPath,
|
||||
{ force: true },
|
||||
{ onProgress: () => {}, onLog: (msg: string) => logs.push(msg) },
|
||||
);
|
||||
|
||||
expect(result.ftsSkipped).toBe(true);
|
||||
expect(logs.join('\n')).toMatch(
|
||||
/FTS index build failed.*missing indexes after build.*keyword search degraded this run/i,
|
||||
);
|
||||
|
||||
const { storagePath } = getStoragePaths(tmpRepo.dbPath);
|
||||
const meta = JSON.parse(await fs.readFile(`${storagePath}/meta.json`, 'utf-8'));
|
||||
expect(meta.capabilities.fts.status).toBe('unavailable');
|
||||
} finally {
|
||||
await tmpRepo.cleanup();
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue