From 2dc1b09e6619a598eebe4c5920c799f336be44aa Mon Sep 17 00:00:00 2001 From: jbarol Date: Wed, 10 Jun 2026 14:49:24 -0700 Subject: [PATCH] test: regression coverage for edge source_id stamping, timer placement, recipe order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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. --- test/cli-force-exit-teardown-arming.test.ts | 56 ++++++++++ test/embedding-dim-check.test.ts | 20 ++++ test/import-code-edges-source-id.test.ts | 112 ++++++++++++++++++++ 3 files changed, 188 insertions(+) create mode 100644 test/cli-force-exit-teardown-arming.test.ts create mode 100644 test/import-code-edges-source-id.test.ts diff --git a/test/cli-force-exit-teardown-arming.test.ts b/test/cli-force-exit-teardown-arming.test.ts new file mode 100644 index 000000000..ff9743e30 --- /dev/null +++ b/test/cli-force-exit-teardown-arming.test.ts @@ -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` 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)'); + }); +}); diff --git a/test/embedding-dim-check.test.ts b/test/embedding-dim-check.test.ts index f74ebc750..6a945b98c 100644 --- a/test/embedding-dim-check.test.ts +++ b/test/embedding-dim-check.test.ts @@ -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. diff --git a/test/import-code-edges-source-id.test.ts b/test/import-code-edges-source-id.test.ts new file mode 100644 index 000000000..69c46be0f --- /dev/null +++ b/test/import-code-edges-source-id.test.ts @@ -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 = ` 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); + }); +});