mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-31 04:07:52 +00:00
test: regression coverage for edge source_id stamping, timer placement, recipe order
- import-code-edges-source-id: scoped import stamps edges + scoped getCallersOf/getCalleesOf match (verified failing pre-fix), plus the unscoped-import case asserting 'default' stamping. - cli-force-exit-teardown-arming: structural pin — the hard-deadline timer arms inside the finally (teardown entry), never before the op body; daemon guard, unref, clearTimeout intact. - embedding-dim-check: recipe order pinned — UPDATE precedes ALTER so the printed SQL can't drift from docs/embedding-migrations.md again.
This commit is contained in:
jbarol
@@ -0,0 +1,56 @@
|
||||
/**
|
||||
* Structural regression — the DISCONNECT_HARD_DEADLINE_MS force-exit timer in
|
||||
* cli.ts main() must be armed at TEARDOWN ENTRY (inside the finally, before
|
||||
* the drain + disconnect), never before the op-dispatch try block.
|
||||
*
|
||||
* Pre-fix bug: the 10s unref'd setTimeout was armed BEFORE the try, so any op
|
||||
* whose handler ran past 10s wall-clock was killed mid-flight with
|
||||
* process.exit(0) and ZERO stdout — an empty "success" indistinguishable from
|
||||
* no results (a healthy `gbrain search` on a slow Postgres pooler hit this on
|
||||
* every run). Armed in the finally, the timer still bounds a hung
|
||||
* drain/disconnect (the C13 contract) but can no longer kill a
|
||||
* slow-but-progressing op body.
|
||||
*
|
||||
* Source-grep is the right tool here (same rationale as
|
||||
* fix-wave-structural.test.ts): the rule is "this arming must stay at this
|
||||
* location". A behavioral test would need >10s of real wall-clock plus a
|
||||
* deliberately slow op handler in a spawned CLI — slow and flaky by
|
||||
* construction.
|
||||
*/
|
||||
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import { readFileSync } from 'fs';
|
||||
|
||||
describe('cli.ts — disconnect hard-deadline armed at teardown entry, not before the op body', () => {
|
||||
test('forceExitTimer setTimeout lives inside the finally, gated on the daemon guard, before the drain', () => {
|
||||
const src = readFileSync('src/cli.ts', 'utf8');
|
||||
|
||||
const decl = src.indexOf('const DISCONNECT_HARD_DEADLINE_MS');
|
||||
expect(decl).toBeGreaterThan(-1);
|
||||
const tryIdx = src.indexOf('try {', decl);
|
||||
expect(tryIdx).toBeGreaterThan(-1);
|
||||
const finallyIdx = src.indexOf('} finally {', tryIdx);
|
||||
expect(finallyIdx).toBeGreaterThan(-1);
|
||||
const armIdx = src.indexOf('forceExitTimer = setTimeout', decl);
|
||||
expect(armIdx).toBeGreaterThan(-1);
|
||||
const drainIdx = src.indexOf('drainAllBackgroundWorkForCliExit', finallyIdx);
|
||||
expect(drainIdx).toBeGreaterThan(-1);
|
||||
|
||||
// NO arming between the deadline declaration and the op-body try — a
|
||||
// pre-try timer kills slow-but-progressing op handlers mid-flight with
|
||||
// exit 0 and empty stdout. (`setTimeout(` matches only a call site; the
|
||||
// `ReturnType<typeof setTimeout>` type annotation stays allowed.)
|
||||
expect(src.slice(decl, tryIdx)).not.toContain('setTimeout(');
|
||||
|
||||
// The arming sits AFTER the finally opens (teardown entry) and BEFORE the
|
||||
// drain + disconnect it exists to bound.
|
||||
expect(armIdx).toBeGreaterThan(finallyIdx);
|
||||
expect(armIdx).toBeLessThan(drainIdx);
|
||||
|
||||
// Still gated on the daemon-survival guard so `serve` stays alive, and
|
||||
// still unref'd + cleared on clean teardown.
|
||||
expect(src.slice(finallyIdx, drainIdx)).toMatch(/if \(shouldForceExitAfterMain\(\)\)/);
|
||||
expect(src.slice(finallyIdx, drainIdx)).toContain('forceExitTimer.unref?.()');
|
||||
expect(src.slice(drainIdx)).toContain('if (forceExitTimer) clearTimeout(forceExitTimer)');
|
||||
});
|
||||
});
|
||||
@@ -103,6 +103,26 @@ describe('embeddingMismatchMessage', () => {
|
||||
expect(msg).toContain('docs/embedding-migrations.md');
|
||||
});
|
||||
|
||||
test('Postgres recipe NULLs embeddings BEFORE the column alter (pgvector refuses cross-dim casts)', () => {
|
||||
// pgvector aborts `ALTER COLUMN TYPE vector(N)` with "expected N
|
||||
// dimensions, not M" while rows still hold old-width vectors — which is
|
||||
// every brain running this recipe. The UPDATE must precede the ALTER
|
||||
// (NULLs cast fine). Order pinned so the printed recipe can't drift from
|
||||
// the corrected docs/embedding-migrations.md again.
|
||||
const msg = embeddingMismatchMessage({
|
||||
currentDims: 1536,
|
||||
requestedDims: 768,
|
||||
requestedModel: 'nomic-embed-text',
|
||||
source: 'init',
|
||||
engineKind: 'postgres',
|
||||
});
|
||||
const nullIdx = msg.indexOf('UPDATE content_chunks SET embedding = NULL');
|
||||
const alterIdx = msg.indexOf('ALTER TABLE content_chunks ALTER COLUMN embedding TYPE vector(768)');
|
||||
expect(nullIdx).toBeGreaterThan(-1);
|
||||
expect(alterIdx).toBeGreaterThan(-1);
|
||||
expect(nullIdx).toBeLessThan(alterIdx);
|
||||
});
|
||||
|
||||
test('Postgres branch skips HNSW recreate when requested dims exceed pgvector cap', () => {
|
||||
// Codex finding #8: 2048d (Voyage 4 Large) cannot be HNSW-indexed in pgvector.
|
||||
// The recipe must NOT instruct a CREATE INDEX HNSW for that dim.
|
||||
|
||||
@@ -0,0 +1,112 @@
|
||||
/**
|
||||
* Regression — importCodeFile stamps source_id on extracted call-graph edges.
|
||||
*
|
||||
* Pre-fix bug: importCodeFile built CodeEdgeInput rows WITHOUT source_id, so
|
||||
* every extracted edge landed NULL in code_edges_symbol. getCallersOf /
|
||||
* getCalleesOf add `AND source_id = <scoped>` whenever a worktree pin or
|
||||
* --source is in play — NULL never matches that filter, so scoped call-graph
|
||||
* queries silently returned 0 rows on multi-source brains even though the
|
||||
* edges existed. Pre-existing coverage (cathedral-ii-brainbench.test.ts,
|
||||
* code-edges.test.ts) only ever queried with { allSources: true }, which
|
||||
* bypasses the filter — exactly why the NULL never surfaced.
|
||||
*
|
||||
* This test imports a caller/callee pair under a non-default source and
|
||||
* asserts (a) the persisted code_edges_symbol rows carry the source_id, and
|
||||
* (b) the SCOPED getCallersOf/getCalleesOf — the user-visible path that
|
||||
* returned 0 — now find the edge, while a different source scope does not.
|
||||
*/
|
||||
|
||||
import { describe, test, expect, beforeAll, afterAll } from 'bun:test';
|
||||
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
|
||||
import { importCodeFile } from '../src/core/import-file.ts';
|
||||
import { runSources } from '../src/commands/sources.ts';
|
||||
|
||||
let engine: PGLiteEngine;
|
||||
|
||||
beforeAll(async () => {
|
||||
engine = new PGLiteEngine();
|
||||
await engine.connect({});
|
||||
await engine.initSchema();
|
||||
await runSources(engine, ['add', 'testsrc', '--no-federated']);
|
||||
|
||||
// Same fixture shape as cathedral-ii-brainbench: runner() calls helper(),
|
||||
// Layer 5 edge extraction captures the unresolved 'calls' edge — but here
|
||||
// the import is pinned to a non-default source.
|
||||
await importCodeFile(
|
||||
engine,
|
||||
'src/a.ts',
|
||||
'export function runner() { return helper(); }\n',
|
||||
{ noEmbed: true, sourceId: 'testsrc' },
|
||||
);
|
||||
await importCodeFile(
|
||||
engine,
|
||||
'src/b.ts',
|
||||
'export function helper() { return 42; }\n',
|
||||
{ noEmbed: true, sourceId: 'testsrc' },
|
||||
);
|
||||
}, 60_000);
|
||||
|
||||
afterAll(async () => {
|
||||
if (engine) await engine.disconnect();
|
||||
}, 30_000);
|
||||
|
||||
describe('importCodeFile — source_id stamped on extracted call-graph edges', () => {
|
||||
test('edges land with the import source_id and scoped caller/callee queries match', async () => {
|
||||
// (a) The persisted unresolved edge rows carry the source, not NULL.
|
||||
const rows = await engine.executeRaw<{ source_id: string | null }>(
|
||||
`SELECT source_id FROM code_edges_symbol WHERE from_symbol_qualified = $1`,
|
||||
['runner'],
|
||||
);
|
||||
expect(rows.length).toBeGreaterThanOrEqual(1);
|
||||
for (const r of rows) {
|
||||
expect(r.source_id).toBe('testsrc');
|
||||
}
|
||||
|
||||
// (b) The scoped queries — the path that silently returned 0 pre-fix.
|
||||
const callers = await engine.getCallersOf('helper', { sourceId: 'testsrc' });
|
||||
const fromRunner = callers.find(r => r.from_symbol_qualified === 'runner');
|
||||
expect(fromRunner).toBeDefined();
|
||||
expect(fromRunner!.edge_type).toBe('calls');
|
||||
expect(fromRunner!.source_id).toBe('testsrc');
|
||||
|
||||
const callees = await engine.getCalleesOf('runner', { sourceId: 'testsrc' });
|
||||
expect(callees.some(r => r.to_symbol_qualified === 'helper')).toBe(true);
|
||||
|
||||
// Source isolation still holds: a different scope must NOT see the edge.
|
||||
const otherScope = await engine.getCallersOf('helper', { sourceId: 'default' });
|
||||
expect(otherScope.find(r => r.from_symbol_qualified === 'runner')).toBeUndefined();
|
||||
});
|
||||
|
||||
test('UNSCOPED import stamps edges with the schema-default source, not NULL', async () => {
|
||||
// The other door of the same bug: an import WITHOUT opts.sourceId (legacy
|
||||
// unscoped callers — `gbrain reindex --code` with no --source) lands its
|
||||
// pages under the schema default (pages.source_id DEFAULT 'default').
|
||||
// If its edges were stamped NULL, the matching scoped query
|
||||
// getCallersOf(sym, { sourceId: 'default' }) — a worktree pinned to
|
||||
// default, --source default, GBRAIN_SOURCE=default — would miss them.
|
||||
await importCodeFile(
|
||||
engine,
|
||||
'src/c.ts',
|
||||
'export function unscopedRunner() { return unscopedHelper(); }\n',
|
||||
{ noEmbed: true },
|
||||
);
|
||||
await importCodeFile(
|
||||
engine,
|
||||
'src/d.ts',
|
||||
'export function unscopedHelper() { return 7; }\n',
|
||||
{ noEmbed: true },
|
||||
);
|
||||
|
||||
const rows = await engine.executeRaw<{ source_id: string | null }>(
|
||||
`SELECT source_id FROM code_edges_symbol WHERE from_symbol_qualified = $1`,
|
||||
['unscopedRunner'],
|
||||
);
|
||||
expect(rows.length).toBeGreaterThanOrEqual(1);
|
||||
for (const r of rows) {
|
||||
expect(r.source_id).toBe('default');
|
||||
}
|
||||
|
||||
const callers = await engine.getCallersOf('unscopedHelper', { sourceId: 'default' });
|
||||
expect(callers.some(r => r.from_symbol_qualified === 'unscopedRunner')).toBe(true);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user