mirror of
https://github.com/garrytan/gbrain.git
synced 2026-08-14 17:02:19 +00:00
Compare commits
1
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
97ddee0548 |
@@ -197,7 +197,7 @@ export interface SyncResult {
|
||||
/** Pages re-embedded during this sync's auto-embed step. 0 if --no-embed or skipped. */
|
||||
embedded: number;
|
||||
pagesAffected: string[];
|
||||
failedFiles?: number; // count of per-file import/sync failures (Bug 9)
|
||||
failedFiles?: number; // count of parse failures (Bug 9)
|
||||
/**
|
||||
* v0.41.13.0 partial-sync fields (only set when status === 'partial').
|
||||
*
|
||||
@@ -3183,7 +3183,7 @@ async function performSyncInner(engine: BrainEngine, opts: SyncOpts): Promise<Sy
|
||||
await clearOpCheckpoint(engine, ckpt.target);
|
||||
};
|
||||
|
||||
// issue #1939 adversarial finding #1: a file that failed to import (open ledger
|
||||
// issue #1939 adversarial finding #1: a file that failed to parse (open ledger
|
||||
// row) and is then deleted/renamed-away never re-enters failedFiles and never
|
||||
// imports, so its row would never clear and would age doctor to a permanent
|
||||
// FAIL. Treat removed paths as resolved so the ledger self-heals.
|
||||
@@ -3215,9 +3215,9 @@ async function performSyncInner(engine: BrainEngine, opts: SyncOpts): Promise<Sy
|
||||
} else {
|
||||
const fileFailCount = failedFiles.filter(f => isSkippablePath(f.path)).length;
|
||||
serr(
|
||||
`\nSync blocked: ${fileFailCount} file(s) failed to import:\n` +
|
||||
`\nSync blocked: ${fileFailCount} file(s) failed to parse:\n` +
|
||||
`${codeBreakdown}\n\n` +
|
||||
`Fix the listed file errors and re-run, or use 'gbrain sync --skip-failed' to ` +
|
||||
`Fix the frontmatter and re-run, or use 'gbrain sync --skip-failed' to ` +
|
||||
`acknowledge and move on. A file that keeps failing auto-skips after ` +
|
||||
`${resolveAutoSkipThreshold()} consecutive syncs.`,
|
||||
);
|
||||
@@ -5355,7 +5355,7 @@ function printSyncResult(result: SyncResult, sink: NodeJS.WriteStream = process.
|
||||
case 'dry_run':
|
||||
break; // already printed in performSync
|
||||
case 'blocked_by_failures':
|
||||
write(`Sync BLOCKED at ${result.toCommit.slice(0, 8)}: ${result.failedFiles ?? 0} file(s) failed to import.`);
|
||||
write(`Sync BLOCKED at ${result.toCommit.slice(0, 8)}: ${result.failedFiles ?? 0} file(s) failed to parse.`);
|
||||
write(` See ~/.gbrain/sync-failures.jsonl for details, or run 'gbrain doctor'.`);
|
||||
write(` Fix the files then re-run 'gbrain sync', or 'gbrain sync --skip-failed' to move on.`);
|
||||
break;
|
||||
|
||||
@@ -1058,16 +1058,6 @@ export class PGLiteEngine implements BrainEngine {
|
||||
RETURNING id, source_id, slug, type, title, compiled_truth, timeline, frontmatter, content_hash, created_at, updated_at, effective_date, effective_date_source, import_filename, source_kind, source_uri, ingested_via, ingested_at`,
|
||||
[sourceId, slug, page.type, pageKind, page.title, page.compiled_truth, page.timeline || '', JSON.stringify(frontmatter), hash, effectiveDate, effectiveDateSource, importFilename, chunkerVersion, sourcePath, sourceKind, sourceUri, ingestedVia, ingestedAt]
|
||||
);
|
||||
// #2189: an INSERT … ON CONFLICT DO UPDATE … RETURNING that yields 0 rows
|
||||
// (e.g. a BEFORE trigger suppressing the write) previously crashed in
|
||||
// rowToPage with an opaque "undefined is not an object (row.deleted_at)".
|
||||
// Throw a diagnosable error naming the row instead. Mirrors postgres-engine.ts.
|
||||
if (!rows[0]) {
|
||||
throw new Error(
|
||||
`putPage: INSERT … RETURNING produced no row for slug='${slug}' source_id='${sourceId}'. ` +
|
||||
`A trigger or rule on the pages table may be suppressing the write.`
|
||||
);
|
||||
}
|
||||
return rowToPage(rows[0] as Record<string, unknown>);
|
||||
}
|
||||
|
||||
|
||||
@@ -1119,16 +1119,6 @@ export class PostgresEngine implements BrainEngine {
|
||||
ingested_at = COALESCE(EXCLUDED.ingested_at, pages.ingested_at)
|
||||
RETURNING id, source_id, slug, type, title, compiled_truth, timeline, frontmatter, content_hash, created_at, updated_at, effective_date, effective_date_source, import_filename, source_kind, source_uri, ingested_via, ingested_at
|
||||
`;
|
||||
// #2189: an INSERT … ON CONFLICT DO UPDATE … RETURNING that yields 0 rows
|
||||
// (e.g. a BEFORE trigger suppressing the write) previously crashed in
|
||||
// rowToPage with an opaque "undefined is not an object (row.deleted_at)".
|
||||
// Throw a diagnosable error naming the row instead. Mirrors pglite-engine.ts.
|
||||
if (!rows[0]) {
|
||||
throw new Error(
|
||||
`putPage: INSERT … RETURNING produced no row for slug='${slug}' source_id='${sourceId}'. ` +
|
||||
`A trigger or rule on the pages table may be suppressing the write.`
|
||||
);
|
||||
}
|
||||
return rowToPage(rows[0]);
|
||||
}
|
||||
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,74 +0,0 @@
|
||||
// #2189 regression guard: putPage's INSERT … ON CONFLICT DO UPDATE … RETURNING
|
||||
// can yield 0 rows when brain-local DB state (e.g. a BEFORE INSERT trigger)
|
||||
// suppresses the write. Pre-fix, rowToPage(rows[0]) crashed with the opaque
|
||||
// "undefined is not an object (evaluating 'row.deleted_at')" that failed
|
||||
// ~all files of a code sync. Post-fix, putPage throws a descriptive error
|
||||
// naming the slug + source_id so the failure is diagnosable per-file.
|
||||
//
|
||||
// Same guard lands in postgres-engine.ts (engine-parity invariant); this test
|
||||
// exercises the PGLite side, where the issue was reported.
|
||||
|
||||
import { describe, expect, test, beforeAll, afterAll } from 'bun:test';
|
||||
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
|
||||
|
||||
let engine: PGLiteEngine;
|
||||
|
||||
beforeAll(async () => {
|
||||
engine = new PGLiteEngine();
|
||||
await engine.connect({});
|
||||
await engine.initSchema();
|
||||
// Simulate the reporter's state-dependent failure: a trigger that
|
||||
// suppresses inserts for one slug, making RETURNING produce no row.
|
||||
await engine.executeRaw(`
|
||||
CREATE OR REPLACE FUNCTION suppress_pages_insert() RETURNS trigger AS $$
|
||||
BEGIN
|
||||
IF NEW.slug = 'suppressed-page' THEN RETURN NULL; END IF;
|
||||
RETURN NEW;
|
||||
END;
|
||||
$$ LANGUAGE plpgsql;
|
||||
`);
|
||||
await engine.executeRaw(`
|
||||
CREATE TRIGGER suppress_pages_insert_trg
|
||||
BEFORE INSERT ON pages
|
||||
FOR EACH ROW EXECUTE FUNCTION suppress_pages_insert();
|
||||
`);
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await engine.executeRaw('DROP TRIGGER IF EXISTS suppress_pages_insert_trg ON pages');
|
||||
await engine.executeRaw('DROP FUNCTION IF EXISTS suppress_pages_insert');
|
||||
await engine.disconnect();
|
||||
});
|
||||
|
||||
describe('putPage RETURNING guard (#2189)', () => {
|
||||
test('0-row RETURNING throws a descriptive error, not row.deleted_at TypeError', async () => {
|
||||
let err: Error | undefined;
|
||||
try {
|
||||
await engine.putPage('suppressed-page', {
|
||||
type: 'code',
|
||||
title: 'Suppressed',
|
||||
compiled_truth: 'x',
|
||||
timeline: '',
|
||||
});
|
||||
} catch (e) {
|
||||
err = e as Error;
|
||||
}
|
||||
expect(err).toBeDefined();
|
||||
expect(err!.message).toContain('putPage');
|
||||
expect(err!.message).toContain("slug='suppressed-page'");
|
||||
expect(err!.message).toContain("source_id='default'");
|
||||
// The pre-fix crash signature must be gone.
|
||||
expect(err!.message).not.toContain('deleted_at');
|
||||
});
|
||||
|
||||
test('unsuppressed slugs still upsert normally with the trigger installed', async () => {
|
||||
const page = await engine.putPage('normal-page', {
|
||||
type: 'concept',
|
||||
title: 'Normal',
|
||||
compiled_truth: 'y',
|
||||
timeline: '',
|
||||
});
|
||||
expect(page.slug).toBe('normal-page');
|
||||
expect(page.source_id).toBe('default');
|
||||
});
|
||||
});
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -375,31 +375,6 @@ describe('performSync dry-run never writes', () => {
|
||||
expect(messages.some(m => m.includes('git pull failed'))).toBe(false);
|
||||
});
|
||||
|
||||
test('first PGLite code sync imports code files without runtime failures', async () => {
|
||||
const { performSync } = await import('../src/commands/sync.ts');
|
||||
mkdirSync(join(repoPath, 'src'), { recursive: true });
|
||||
writeFileSync(
|
||||
join(repoPath, 'src/example.ts'),
|
||||
'export function add(left: number, right: number) { return left + right; }\n',
|
||||
);
|
||||
execSync('git add -A && git commit -m "add code file"', { cwd: repoPath, stdio: 'pipe' });
|
||||
|
||||
const result = await performSync(engine, {
|
||||
repoPath,
|
||||
noPull: true,
|
||||
noEmbed: true,
|
||||
noExtract: true,
|
||||
strategy: 'code',
|
||||
});
|
||||
|
||||
expect(result.status).toBe('first_sync');
|
||||
expect(result.added).toBe(1);
|
||||
expect(result.failedFiles ?? 0).toBe(0);
|
||||
const page = await engine.getPage('src-example-ts');
|
||||
expect(page?.type).toBe('code');
|
||||
expect(page?.frontmatter).toMatchObject({ file: 'src/example.ts', language: 'typescript' });
|
||||
});
|
||||
|
||||
test('incremental dry-run does NOT write to DB or advance the bookmark', async () => {
|
||||
const { performSync } = await import('../src/commands/sync.ts');
|
||||
// First do a real sync to seed the bookmark.
|
||||
|
||||
Reference in New Issue
Block a user