mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-27 22:15:33 +00:00
fix(schema-pack): narrow stats catch-all so masked errors surface, not fake 0 pages (#2466)
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: javieraldape <javieraldape@users.noreply.github.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
javieraldape
Claude Fable 5
parent
0612b0daa8
commit
97ddee0548
@@ -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;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -222,6 +222,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 () => {
|
||||
|
||||
Reference in New Issue
Block a user