diff --git a/gitnexus/src/core/index-freshness.ts b/gitnexus/src/core/index-freshness.ts index f063e446c..d8ba97841 100644 --- a/gitnexus/src/core/index-freshness.ts +++ b/gitnexus/src/core/index-freshness.ts @@ -38,11 +38,33 @@ export const GRAPH_WRITE_COLLAPSE_MIN_EDGES = 100; */ export function detectGraphWriteCollapse( expected: number, - persisted: number, + /** + * Relationships readable from the DB, or `undefined` when the count could + * not be READ at all (no connection, a query that threw). + * + * The distinction is load-bearing and was got wrong once: `getLbugStats` + * reports `edges: 0` for "no connection", "query threw" AND "empty table" + * alike, so passing it straight in made every run without a readable DB look + * like a total collapse. An unmeasurable count is not a measured zero — + * accepting `undefined` here is what keeps this check from committing the + * same confident-zero error it exists to catch. + */ + persisted: number | undefined, ): { expected: number; persisted: number } | undefined { - if (expected < GRAPH_WRITE_COLLAPSE_MIN_EDGES) return undefined; - if (persisted >= expected * GRAPH_WRITE_COLLAPSE_RATIO) return undefined; - return { expected, persisted }; + // Both sides must be REAL NUMBERS before any comparison. A non-numeric + // `expected` (a graph implementation that reports no total, a lightweight + // pipeline result) does not merely skip the guards — it INVERTS them: + // `undefined < 100` is false, so the min-edges exemption never fires, and + // `0 >= undefined * 0.5` is `0 >= NaN`, also false, so the ratio check + // "passes" too and a healthy run is reported as a total collapse. Comparing + // against a non-number is the one way this check can manufacture the exact + // false certainty it was written to prevent. + if (!Number.isFinite(expected) || !Number.isFinite(persisted as number)) return undefined; + const expectedCount = expected as number; + const persistedCount = persisted as number; + if (expectedCount < GRAPH_WRITE_COLLAPSE_MIN_EDGES) return undefined; + if (persistedCount >= expectedCount * GRAPH_WRITE_COLLAPSE_RATIO) return undefined; + return { expected: expectedCount, persisted: persistedCount }; } /** Stable machine-readable reasons an index cannot be certified complete. */ diff --git a/gitnexus/src/core/run-analyze.ts b/gitnexus/src/core/run-analyze.ts index 96c939304..45f117283 100644 --- a/gitnexus/src/core/run-analyze.ts +++ b/gitnexus/src/core/run-analyze.ts @@ -2539,11 +2539,20 @@ async function runFullAnalysisInner( // relationships out of memory may no longer be able to report a total, and // a false "your index is broken" is worse than a missed one. const expectedRelationships = pipelineResult.graph.relationshipCount; - const graphWriteCollapsed = detectGraphWriteCollapse(expectedRelationships, stats.edges); + // `getLbugStats` flattens "no connection", "query threw" and "empty table" + // into `edges: 0`, so a bare `stats.edges` would make every run without a + // readable DB look like a total collapse — the confident-zero error this + // check exists to catch. `nodes > 0` is independent evidence the DB was + // readable at all; without it the count is UNKNOWN, not zero. + const persistedRelationships = stats.nodes > 0 ? stats.edges : undefined; + const graphWriteCollapsed = detectGraphWriteCollapse( + expectedRelationships, + persistedRelationships, + ); if (graphWriteCollapsed) { log( `Warning: graph write incomplete — the pipeline produced ${expectedRelationships} ` + - `relationships but only ${stats.edges} are readable from the index. Recording the ` + + `relationships but only ${persistedRelationships} are readable from the index. Recording the ` + `index as INCOMPLETE (graph-write-collapsed) rather than fresh; re-run ` + `\`gitnexus analyze --force\`.`, ); diff --git a/gitnexus/test/unit/index-freshness-graph-collapse.test.ts b/gitnexus/test/unit/index-freshness-graph-collapse.test.ts index bcff4d732..1ff79ee61 100644 --- a/gitnexus/test/unit/index-freshness-graph-collapse.test.ts +++ b/gitnexus/test/unit/index-freshness-graph-collapse.test.ts @@ -40,6 +40,24 @@ describe('detectGraphWriteCollapse (B2 detection)', () => { expect(detectGraphWriteCollapse(1000, 4000)).toBeUndefined(); }); + // REGRESSION. A non-numeric `expected` does not merely skip the guards, it + // INVERTS them: `undefined < 100` is false so the small-repo exemption never + // fires, and `0 >= undefined * 0.5` is `0 >= NaN`, also false, so the ratio + // check "passes" too. Shipped briefly and reported healthy runs as total + // collapses — the exact false certainty this check exists to prevent. + it('never fires when the expected count is not a number', () => { + expect(detectGraphWriteCollapse(undefined as unknown as number, 0)).toBeUndefined(); + expect(detectGraphWriteCollapse(NaN, 0)).toBeUndefined(); + expect(detectGraphWriteCollapse(Infinity, 0)).toBeUndefined(); + }); + + it('never fires when the persisted count is not a number', () => { + // `getLbugStats` returns `{}` under some mocks/degraded paths, so + // `stats.edges` arrives as undefined rather than a measured zero. + expect(detectGraphWriteCollapse(23009, undefined)).toBeUndefined(); + expect(detectGraphWriteCollapse(23009, NaN)).toBeUndefined(); + }); + it('is fail-safe when the expected count is unavailable', () => { // An implementation that offloads relationships out of memory may report 0; // a false "your index is broken" is worse than a missed one.