fix: address review findings — narrow catch, truthful capabilities, progress events

Addresses all 5 blocking findings from production-readiness review:

1. meta.json capabilities: Track ftsIndexed flag and write
   status='degraded' (not 'available') when FTS indexes are missing.
   Prevents false-positive capability reporting (#1715 regression).

2. Progress event gap: Emit progress('fts', 90, 'Search indexes
   degraded (BM25 unavailable)') in the verification-failure branch
   so progress never stalls at 85%.

3. Narrow catch scope: Only suppress errors containing
   'FTS extension unavailable'. DB connection failures, schema errors,
   and programming errors (e.g. safeIdentifier) now propagate as before.

4. (Finding 4 resolved by Finding 3): Non-FTS errors still throw in
   all analyze modes, so only the intended FTS-unavailable case degrades.

5. Stronger test assertions: Verify warning log content, verify
   progress does not report 'Search indexes ready', verify degraded
   progress message is emitted.
This commit is contained in:
henry 2026-05-23 09:26:23 +08:00
parent c7ce4d5c1f
commit c434acbcdb
2 changed files with 25 additions and 2 deletions

View file

@ -669,6 +669,7 @@ export async function runFullAnalysis(
// ── Phase 3: FTS (85–90%) ─────────────────────────────────────────
progress('fts', 85, 'Creating search indexes...');
let ftsIndexed = false;
try {
await createSearchFTSIndexes({
onIndexStart: options.verbose
@ -684,11 +685,17 @@ export async function runFullAnalysis(
`⚠️ FTS verification warning - missing indexes: ${missingIndexNames.join(', ')}. ` +
'BM25 keyword search will be degraded. Upgrade macOS or run on a compatible platform to enable FTS.',
);
progress('fts', 90, 'Search indexes degraded (BM25 unavailable)');
} else {
ftsIndexed = true;
progress('fts', 90, 'Search indexes ready');
}
} catch (ftsErr: unknown) {
const ftsMsg = ftsErr instanceof Error ? ftsErr.message : String(ftsErr);
// Only suppress known FTS-extension-unavailable errors; rethrow DB/schema/programming errors.
if (!ftsMsg.includes('FTS extension unavailable')) {
throw ftsErr;
}
log(`⚠️ FTS creation skipped (non-fatal): ${ftsMsg}`);
progress('fts', 90, 'Search indexes skipped (FTS unavailable)');
}
@ -880,7 +887,7 @@ export async function runFullAnalysis(
},
capabilities: {
graph: { provider: 'ladybugdb', status: runtimeCapabilities.graph },
fts: { provider: 'ladybugdb-fts', status: runtimeCapabilities.fts },
fts: { provider: 'ladybugdb-fts', status: ftsIndexed ? runtimeCapabilities.fts : 'degraded' },
vectorSearch: {
provider: effectiveSemanticMode === 'vector-index' ? 'ladybugdb-vector' : 'exact-scan',
status: embeddingCount > 0 ? effectiveSemanticMode : 'unavailable',

View file

@ -227,17 +227,33 @@ describe('runFullAnalysis FTS repair and verification failure paths', () => {
const tmpRepo = await createTempDir('gitnexus-run-analyze-full-verify-fail-');
try {
const { runFullAnalysis } = await import('../../src/core/run-analyze.js');
const logMessages: string[] = [];
const progressMessages: string[] = [];
// FTS verification failure is now non-fatal — analyze completes with a warning
// instead of throwing, so embedding generation can proceed.
const result = await runFullAnalysis(
tmpRepo.dbPath,
{ force: true },
{
onProgress: () => {},
onProgress: (_phase, _pct, msg) => {
if (msg) progressMessages.push(msg);
},
onLog: (msg: string) => {
logMessages.push(msg);
},
},
);
expect(result).toBeDefined();
expect(result.repoPath).toContain('gitnexus-run-analyze-full-verify-fail-');
// Verify warning was logged about missing FTS indexes
expect(logMessages.some((m) => /FTS verification warning/i.test(m))).toBe(true);
// Verify progress did NOT report "Search indexes ready" (it should report degraded)
expect(progressMessages).not.toContain('Search indexes ready');
expect(progressMessages.some((m) => /degraded|unavailable/i.test(m))).toBe(true);
} finally {
await tmpRepo.cleanup();
}