fix(schema-pack): narrow stats catch-all so masked errors surface, not fake 0 pages (#2466) (#3133)

fetchCountRows and detectDeadPrefixes in src/core/schema-pack/stats.ts
swallowed EVERY engine error into empty results, so any real failure
printed 'Total pages: 0' + a vacuous 100% coverage on a populated brain.
Both catches now swallow only isUndefinedTableError (pre-init brain,
missing pages table) and rethrow everything else. Four regression tests:
real non-zero count on a populated PGLite brain, rethrow on non-missing-
table errors in both catch sites, and the missing-table degrade path.

Takeover of #2493.

Co-authored-by: Garry Tan <garrytan@gmail.com>
Co-authored-by: javieraldape <javieraldape@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Time Attakc
2026-07-23 12:55:00 -07:00
committed by GitHub
co-authored by Garry Tan javieraldape Claude Fable 5
parent 2283269932
commit 0853491eb2
2 changed files with 100 additions and 6 deletions
+17 -6
View File
@@ -18,6 +18,7 @@
import type { BrainEngine } from '../engine.ts';
import { loadActivePackBestEffort } from './best-effort.ts';
import type { OperationContext } from '../operations.ts';
import { isUndefinedTableError } from '../utils.ts';
export interface StatsOpts {
/** Single source scope. Omit + omit sourceIds for whole-brain aggregate. */
@@ -164,9 +165,17 @@ async function fetchCountRows(engine: BrainEngine, opts: StatsOpts): Promise<Raw
`;
try {
return await engine.executeRaw<RawCountRow>(sql, params);
} catch {
// Empty / pre-init brain: pages table may not exist yet.
return [];
} catch (err) {
// ONLY swallow the genuine "pages table doesn't exist yet" case
// (empty / pre-init brain). #2466: the old bare `catch {}` masked
// EVERY error — so any engine-level failure (connection, version
// skew, a query incompatibility) was silently converted to 0 rows,
// printing "Total pages: 0" on a populated brain and cascading into
// false "100% coverage" + a starved `schema suggest`. Surface
// everything that is not a missing-table error so the real failure
// is visible instead of hidden behind a fake zero.
if (isUndefinedTableError(err)) return [];
throw err;
}
}
@@ -204,9 +213,11 @@ async function detectDeadPrefixes(
if (cnt === 0) {
hints.push({ type: t.name, prefix });
}
} catch {
// Skip on engine error (no pages table yet, etc.).
continue;
} catch (err) {
// #2466: only skip on the genuine "no pages table yet" case;
// rethrow any other engine error so it isn't silently masked.
if (isUndefinedTableError(err)) continue;
throw err;
}
}
}
+83
View File
@@ -231,6 +231,89 @@ describe('runStatsCore — JSON envelope shape', () => {
});
});
describe('runStatsCore — #2466 catch-narrowing (real count + error surfacing)', () => {
// #2466: `gbrain schema stats` reported "Total pages: 0" on a populated
// PGLite brain. The bug was a bare `catch {}` in fetchCountRows (and a
// sibling in detectDeadPrefixes) that converted ANY engine error into 0
// rows. The COUNT query itself is valid on PGLite (proven below), so the
// regression pins two things: (a) a populated brain reports the real,
// non-zero count through the full runStatsCore path; (b) a non-missing-
// table engine error is rethrown, not masked into a fake zero.
it('reports the real non-zero count on a populated PGLite brain (no false 0)', async () => {
await withEnv({ GBRAIN_SCHEMA_PACK: undefined }, async () => {
// Seed a realistic mix: typed, untyped, multiple types — like the
// 169-page brain in the bug report (scaled down).
for (let i = 0; i < 12; i++) {
const type = i % 3 === 0 ? '' : (i % 3 === 1 ? 'person' : 'company');
await seedPage(`notes/p${i}`, { type, sourcePath: `notes/p${i}.md` });
}
const result = await runStatsCore(ctxOf());
// The core regression: NOT zero.
expect(result.aggregate.total_pages).toBe(12);
expect(result.aggregate.typed_pages).toBe(8);
expect(result.aggregate.untyped_pages).toBe(4);
// And coverage is the honest ratio, not the vacuous 1.0 a 0/0 prints.
expect(result.aggregate.coverage).not.toBe(1.0);
});
});
it('fetchCountRows rethrows a non-missing-table engine error instead of masking it as 0 pages', async () => {
await withEnv({ GBRAIN_SCHEMA_PACK: undefined }, async () => {
// No pack → detectDeadPrefixes is skipped, isolating the throw to the
// fetchCountRows catch we narrowed. The count query (the GROUP BY one)
// throws a column-level error (SQLSTATE 42703) — the exact class the
// old bare `catch {}` swallowed into 0 rows; everything else succeeds.
__setPackLocatorForTests(() => null);
const boom = Object.assign(new Error('column "type" does not exist'), { code: '42703' });
const stubEngine = {
executeRaw: async (sql: string) => {
if (/GROUP BY source_id/.test(sql)) throw boom; // the fetchCountRows query
return [];
},
} as unknown as PGLiteEngine;
const ctx = { ...ctxOf(), engine: stubEngine } as unknown as OperationContext;
await expect(runStatsCore(ctx)).rejects.toThrow('column "type" does not exist');
});
});
it('fetchCountRows still degrades to empty (no throw) on a genuine missing pages table', async () => {
await withEnv({ GBRAIN_SCHEMA_PACK: undefined }, async () => {
// Pre-init brain shape: the count query hits a missing pages table
// (SQLSTATE 42P01). This is the ONLY case the narrowed catch swallows.
__setPackLocatorForTests(() => null);
const missing = Object.assign(new Error('relation "pages" does not exist'), { code: '42P01' });
const stubEngine = {
executeRaw: async (sql: string) => {
if (/GROUP BY source_id/.test(sql)) throw missing;
return [];
},
} as unknown as PGLiteEngine;
const ctx = { ...ctxOf(), engine: stubEngine } as unknown as OperationContext;
const result = await runStatsCore(ctx);
expect(result.aggregate.total_pages).toBe(0);
expect(result.per_source).toEqual([]);
});
});
it('detectDeadPrefixes rethrows a non-missing-table error (sibling catch)', async () => {
await withEnv({ GBRAIN_HOME: tmpDir, GBRAIN_SCHEMA_PACK: 'tiny' }, async () => {
seedTinyPack('tiny', [{ name: 'person', prefix: 'people/' }]);
// fetchCountRows (the GROUP BY query) succeeds → []; the per-prefix
// dead-prefix LIKE query then throws a non-missing-table error, which
// must surface through the narrowed sibling catch.
const stubEngine = {
executeRaw: async (sql: string) => {
if (/GROUP BY source_id/.test(sql)) return []; // count query: empty brain, fine
throw Object.assign(new Error('division by zero'), { code: '22012' }); // the LIKE query
},
} as unknown as PGLiteEngine;
const ctx = { ...ctxOf(), engine: stubEngine } as unknown as OperationContext;
await expect(runStatsCore(ctx)).rejects.toThrow('division by zero');
});
});
});
describe('runStatsCore — type/untyped split', () => {
it('treats empty-string type as untyped (not its own bucket)', async () => {
await withEnv({ GBRAIN_SCHEMA_PACK: undefined }, async () => {