mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-29 19:01:39 +00:00
merge: #2073 fix(cli,code-graph): search 10s force-exit + scoped call-graph 0-rows + dim-migration order
Reviewed: CLI read-timeout wraps connectEngine (line 249) per the timeout-layer learning; migration v116 (code_edges source_id backfill + from_symbol_qualified index) coexists with the authored #2038 drift repair in runMigrations; typecheck clean. CHANGELOG version collision resolved (master's 0.42.39.0 Retrieval Reflex kept; #2073's notes fold into the wave entry at ship). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -1799,11 +1799,6 @@ Three items deferred:
|
||||
self-heals via stale-reclaim). The common sync SUCCESS path already drains via
|
||||
handleCliOnly's finally. Convert for graceful drain on sync error exits.
|
||||
|
||||
- [ ] **(v0.42.20.0 follow-up) Decouple the op-dispatch force-exit timer** so it
|
||||
wraps `engine.disconnect()` only (it's armed before the handler today, doubling
|
||||
as a blanket handler watchdog) and fix its misleading "engine.disconnect() did
|
||||
not return…" message that fires even when the handler (not disconnect) was slow.
|
||||
|
||||
- [ ] **(v0.42.20.0 follow-up) Gateway idle-timeout (vs absolute) for streaming
|
||||
chat.** `withDefaultTimeout` uses an absolute `AbortSignal.timeout`; a streaming
|
||||
generation actively producing tokens past the chat default (300s) would abort.
|
||||
@@ -3573,6 +3568,18 @@ keeping both skills' triggers intact for chaining.
|
||||
|
||||
## Completed
|
||||
|
||||
### ~~(v0.42.20.0 follow-up) Decouple the op-dispatch force-exit timer~~
|
||||
**Completed:** v0.42.39.0 (2026-06-10)
|
||||
|
||||
The timer now arms at teardown entry (inside the op-dispatch finally, before
|
||||
drain + disconnect) so it bounds ONLY disconnect — no longer doubling as a
|
||||
blanket handler watchdog that killed slow-but-healthy ops at 10s with exit 0
|
||||
and empty stdout. Its "engine.disconnect() did not return…" message is now
|
||||
accurate by construction (it can only fire during teardown). Read-scope
|
||||
handlers + context build got their own explicit wallclock bound (180s default,
|
||||
`--timeout=Ns`, exit 124, hard-exit after teardown) in the same wave. Pinned by
|
||||
`test/cli-force-exit-teardown-arming.test.ts`.
|
||||
|
||||
### ~~Checks 5 + 6 for check-resolvable~~
|
||||
**Completed:** v0.19.0 (2026-04-22)
|
||||
|
||||
|
||||
@@ -43,8 +43,8 @@ genuinely has to change.
|
||||
Switching dimensions requires:
|
||||
|
||||
1. Dropping the HNSW vector index (pgvector won't survive an `ALTER COLUMN TYPE`).
|
||||
2. Altering the column type (Postgres only — PGLite cannot do this).
|
||||
3. Wiping every existing embedding (the old vectors are unusable in the new space).
|
||||
2. Wiping every existing embedding (the old vectors are unusable in the new space — and pgvector refuses to cast them across dimensions, so this must happen before the alter).
|
||||
3. Altering the column type (Postgres only — PGLite cannot do this).
|
||||
4. Re-embedding the entire corpus (can take hours on a 50K-page brain and costs $1-100 in API calls depending on model).
|
||||
5. Conditionally recreating the index (HNSW supports up to 2000 dimensions per pgvector; above that you must use exact scans).
|
||||
|
||||
@@ -115,12 +115,17 @@ BEGIN;
|
||||
-- 1. Drop the HNSW index. It can't survive the column type change.
|
||||
DROP INDEX IF EXISTS idx_chunks_embedding;
|
||||
|
||||
-- 2. Alter the column type.
|
||||
ALTER TABLE content_chunks ALTER COLUMN embedding TYPE vector(<NEW_DIMS>);
|
||||
|
||||
-- 3. Clear stale embeddings so they don't survive into the new space.
|
||||
-- 2. Clear stale embeddings FIRST. This must happen BEFORE the column
|
||||
-- alter: pgvector refuses to cast existing vectors across dimensions
|
||||
-- ("expected <NEW_DIMS> dimensions, not <OLD_DIMS>"), so altering a
|
||||
-- column that still holds old-width vectors aborts the transaction.
|
||||
-- NULLs cast fine. (The old vectors are unusable in the new space
|
||||
-- anyway — this is the wipe step from the rationale above.)
|
||||
UPDATE content_chunks SET embedding = NULL, embedded_at = NULL;
|
||||
|
||||
-- 3. Alter the column type (all rows are NULL now, so the cast succeeds).
|
||||
ALTER TABLE content_chunks ALTER COLUMN embedding TYPE vector(<NEW_DIMS>);
|
||||
|
||||
-- 4. Recreate the HNSW index ONLY IF dims <= 2000. Above that, leave it
|
||||
-- indexless and rely on exact scans (gbrain searchVector handles this
|
||||
-- automatically — search just gets slower, not broken).
|
||||
|
||||
+102
-23
@@ -360,30 +360,83 @@ async function main() {
|
||||
//
|
||||
// Defense-in-depth (adversarial-review C13): `engine.disconnect()` itself
|
||||
// can hang on PGLite (db.close() or releaseLock racing OS-level FS state).
|
||||
// Install an unref'd setTimeout hard-exit fallback BEFORE entering the
|
||||
// try/catch/finally so a hung disconnect cannot defeat the force-exit
|
||||
// contract. Daemons (`serve`) are excluded so they stay alive.
|
||||
// The unref'd hard-exit fallback is armed inside the `finally` below, so it
|
||||
// bounds ONLY the teardown phase (drain + disconnect) — the same placement
|
||||
// as the fall-through owner-disconnect site later in this file. It used to
|
||||
// be armed HERE, before the try, which silently killed any op whose BODY ran
|
||||
// past the deadline: on a slow Postgres pooler (6-10s per fresh connection)
|
||||
// a healthy `gbrain search` was force-exited mid-handler with code 0 and
|
||||
// ZERO stdout — an empty "success" indistinguishable from no results. The
|
||||
// exitCode honor (v0.42.20.0) can't help there: a mid-op kill fires before
|
||||
// any error path sets exitCode. Op-body wallclock bounds are the read-scope
|
||||
// withTimeout wrap inside the try below, not this teardown backstop.
|
||||
// Daemons (`serve`) are excluded so they stay alive.
|
||||
const DISCONNECT_HARD_DEADLINE_MS = 10_000;
|
||||
// Wallclock bound for READ-scope op handlers. With the hard-deadline timer
|
||||
// correctly scoped to teardown, a genuinely WEDGED read handler (hung pooler
|
||||
// connection mid-query) would otherwise hang the CLI forever — the #1633
|
||||
// zombie class the old (buggy) pre-try timer accidentally bounded at 10s.
|
||||
// 180s sits far above any healthy slow-pooler run (6-10s/connection);
|
||||
// --timeout=Ns overrides. Writes/admin stay unbounded: a long import/embed
|
||||
// must never be killed by a default deadline.
|
||||
const READ_OP_TIMEOUT_MS = 180_000;
|
||||
let forceExitTimer: ReturnType<typeof setTimeout> | undefined;
|
||||
if (shouldForceExitAfterMain()) {
|
||||
forceExitTimer = setTimeout(() => {
|
||||
console.warn(
|
||||
`[cli] engine.disconnect() did not return within ${DISCONNECT_HARD_DEADLINE_MS}ms — force-exiting`,
|
||||
);
|
||||
// v0.42.20.0 (codex): honor an exit code an errored op already set —
|
||||
// a bare process.exit(0) here would mask a failed op as success if the
|
||||
// drain/disconnect then hangs.
|
||||
process.exit(process.exitCode ?? 0);
|
||||
}, DISCONNECT_HARD_DEADLINE_MS);
|
||||
// unref so the timer itself doesn't keep the event loop alive — only
|
||||
// the actual pending work (PGLite WASM handle) does. Without unref,
|
||||
// we'd block a clean exit by 10s on every successful CLI run.
|
||||
forceExitTimer.unref?.();
|
||||
}
|
||||
// Set when a wallclock bound fired. The abandoned (timed-out but still
|
||||
// running) handler can hold ref'd sockets/timers that keep Bun's event loop
|
||||
// alive after main() returns — so the finally must hard-exit after teardown
|
||||
// on this path, or the timeout print is followed by an immortal process:
|
||||
// the same zombie class, resurrected through the timeout door (adversarial
|
||||
// review finding).
|
||||
let wallclockTimedOut = false;
|
||||
|
||||
try {
|
||||
const ctx = await makeContext(engine, params);
|
||||
const rawResult = await op.handler(ctx, params);
|
||||
const { withTimeout, OperationTimeoutError } = await import('./core/timeout.ts');
|
||||
const wallclockMs = getCliOptions().timeoutMs ?? READ_OP_TIMEOUT_MS;
|
||||
const onWallclockTimeout = (e: InstanceType<typeof OperationTimeoutError>) => {
|
||||
const hint = getCliOptions().timeoutMs
|
||||
? ''
|
||||
: ` (default ${e.ms}ms; pass --timeout=Ns to override)`;
|
||||
console.error(`${e.label} timed out${hint}.`);
|
||||
process.exitCode = 124;
|
||||
wallclockTimedOut = true;
|
||||
};
|
||||
|
||||
// Context build does DB I/O (resolveSourceId) and runs for EVERY op —
|
||||
// a wedged pooler connection here would otherwise hang reads, writes,
|
||||
// and admin alike with no bound at all (adversarial review finding).
|
||||
let ctx: Awaited<ReturnType<typeof makeContext>>;
|
||||
try {
|
||||
ctx = await withTimeout(
|
||||
makeContext(engine, params),
|
||||
wallclockMs,
|
||||
`gbrain ${command}: context`,
|
||||
);
|
||||
} catch (e: unknown) {
|
||||
if (e instanceof OperationTimeoutError) {
|
||||
onWallclockTimeout(e);
|
||||
return; // the finally below still drains + disconnects, then exits
|
||||
}
|
||||
throw e;
|
||||
}
|
||||
|
||||
let rawResult: unknown;
|
||||
if (op.scope === 'read') {
|
||||
try {
|
||||
rawResult = await withTimeout(
|
||||
op.handler(ctx, params),
|
||||
wallclockMs,
|
||||
`gbrain ${command}`,
|
||||
);
|
||||
} catch (e: unknown) {
|
||||
if (e instanceof OperationTimeoutError) {
|
||||
onWallclockTimeout(e);
|
||||
return; // the finally below still drains + disconnects, then exits
|
||||
}
|
||||
throw e;
|
||||
}
|
||||
} else {
|
||||
rawResult = await op.handler(ctx, params);
|
||||
}
|
||||
// ENG-2 (renderer parity by data shape): JSON-round-trip the local-engine
|
||||
// path's return value so renderers see the same shape they'd see on the
|
||||
// routed path. Date → ISO string; bigint → string (postgres.js shape);
|
||||
@@ -396,7 +449,8 @@ async function main() {
|
||||
// STILL runs (drains every background-work sink + disconnects). A bare
|
||||
// process.exit(1) here would skip the finally → skip the drain + disconnect
|
||||
// (leaves facts/cache/eval-capture writes racing teardown). The finally's
|
||||
// drain bounds teardown; the outer hard-deadline timer bounds a hung one.
|
||||
// drain bounds teardown; the hard-deadline timer armed at teardown entry
|
||||
// bounds a hung one.
|
||||
if (e instanceof OperationError) {
|
||||
console.error(`Error [${e.code}]: ${e.message}`);
|
||||
if (e.suggestion) console.error(` Fix: ${e.suggestion}`);
|
||||
@@ -412,11 +466,36 @@ async function main() {
|
||||
// DB logIngest gets the freshest live-engine window. 1s per-sink timeout:
|
||||
// read paths with no pending work pay the ~0ms fast path; capture/import
|
||||
// that DO enqueue pay up to 1s (+ facts shutdown grace) while in-flight
|
||||
// Haiku finishes. The unref'd hard-deadline timer above is the backstop if
|
||||
// disconnect or a lingering socket keeps Bun's loop alive.
|
||||
// Haiku finishes. The unref'd hard-deadline timer armed here is the
|
||||
// backstop if disconnect or a lingering socket keeps Bun's loop alive —
|
||||
// armed at teardown entry (NOT before the op body; see the C13 comment
|
||||
// above) so a slow-but-progressing op handler is never killed mid-flight.
|
||||
if (shouldForceExitAfterMain()) {
|
||||
forceExitTimer = setTimeout(() => {
|
||||
console.warn(
|
||||
`[cli] engine.disconnect() did not return within ${DISCONNECT_HARD_DEADLINE_MS}ms — force-exiting`,
|
||||
);
|
||||
// v0.42.20.0 (codex): honor an exit code an errored op already set —
|
||||
// a bare process.exit(0) here would mask a failed op as success if the
|
||||
// drain/disconnect then hangs.
|
||||
process.exit(process.exitCode ?? 0);
|
||||
}, DISCONNECT_HARD_DEADLINE_MS);
|
||||
// unref so the timer itself doesn't keep the event loop alive — only
|
||||
// the actual pending work (PGLite WASM handle) does. Without unref,
|
||||
// we'd block a clean exit by 10s on every successful CLI run.
|
||||
forceExitTimer.unref?.();
|
||||
}
|
||||
await drainAllBackgroundWorkForCliExit({ timeoutMs: 1000 });
|
||||
await engine.disconnect();
|
||||
if (forceExitTimer) clearTimeout(forceExitTimer);
|
||||
// Wallclock-timeout path: teardown is done, but the ABANDONED handler
|
||||
// (withTimeout races, it does not cancel) can still hold ref'd sockets /
|
||||
// SDK retry timers that keep Bun's event loop alive indefinitely. With
|
||||
// the hard-deadline timer just cleared, nothing else bounds that — exit
|
||||
// explicitly. Safe: drain + disconnect completed on the lines above.
|
||||
if (wallclockTimedOut) {
|
||||
process.exit(process.exitCode ?? 124);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -217,8 +217,10 @@ export function embeddingMismatchMessage(opts: EmbeddingMismatchOpts): string {
|
||||
``,
|
||||
` BEGIN;`,
|
||||
` DROP INDEX IF EXISTS idx_chunks_embedding;`,
|
||||
` ALTER TABLE content_chunks ALTER COLUMN embedding TYPE vector(${requestedDims});`,
|
||||
` -- NULL embeddings BEFORE the alter: pgvector refuses to cast existing`,
|
||||
` -- vectors across dimensions and aborts the transaction. NULLs cast fine.`,
|
||||
` UPDATE content_chunks SET embedding = NULL, embedded_at = NULL;`,
|
||||
` ALTER TABLE content_chunks ALTER COLUMN embedding TYPE vector(${requestedDims});`,
|
||||
` ${reindexLine.split('\n').join('\n ')}`,
|
||||
` COMMIT;`,
|
||||
``,
|
||||
@@ -573,11 +575,25 @@ export function buildFactsAlterRecipe(
|
||||
): string {
|
||||
const opclass = columnType === 'halfvec' ? 'halfvec_cosine_ops' : 'vector_cosine_ops';
|
||||
const targetType = columnType === 'halfvec' ? `halfvec(${configuredDims})` : `vector(${configuredDims})`;
|
||||
const dimsChanged = columnDims !== configuredDims;
|
||||
return [
|
||||
`-- ALTER ${columnType}(${columnDims}) → ${columnType}(${configuredDims}) on indexed column.`,
|
||||
`-- HOLD a maintenance window: this rewrites every row's embedding.`,
|
||||
`-- Coordinate with any active extract-conversation-facts backfill.`,
|
||||
`DROP INDEX IF EXISTS idx_facts_embedding_hnsw;`,
|
||||
// Same-dim type swaps (halfvec <-> vector) cast row data losslessly and
|
||||
// MUST keep it. Cross-dimension changes are different: pgvector refuses
|
||||
// to cast existing vectors across dimensions ("expected N dimensions,
|
||||
// not M") and aborts the transaction — and the old-space vectors are
|
||||
// unusable at the new width anyway. NULL them first; the facts pipeline
|
||||
// re-embeds on the next write.
|
||||
...(dimsChanged
|
||||
? [
|
||||
`-- Dimension change: NULL embeddings BEFORE the alter — pgvector`,
|
||||
`-- refuses cross-dimension casts and aborts the transaction.`,
|
||||
`UPDATE facts SET embedding = NULL;`,
|
||||
]
|
||||
: []),
|
||||
`ALTER TABLE facts ALTER COLUMN embedding TYPE ${targetType}`,
|
||||
` USING embedding::${targetType};`,
|
||||
`CREATE INDEX idx_facts_embedding_hnsw`,
|
||||
|
||||
+20
-1
@@ -1193,7 +1193,15 @@ export async function importCodeFile(
|
||||
// chunk IDs are stable.
|
||||
if (extractedEdges.length > 0 && chunks.length > 0) {
|
||||
try {
|
||||
const persistedChunks = await engine.getChunks(slug, sourceId ? { sourceId } : undefined);
|
||||
// Normalize ONCE: '' and undefined both mean the schema-default source
|
||||
// (pages.source_id DEFAULT 'default'). Using the normalized value for
|
||||
// BOTH the chunk lookup and the edge stamp keeps them in lockstep —
|
||||
// an unscoped getChunks here could fan out to same-slug chunks from
|
||||
// another source, and a '' stamp would FK-violate against sources(id)
|
||||
// and silently drop the file's whole call graph in the best-effort
|
||||
// catch below (adversarial review findings).
|
||||
const edgeSourceId = sourceId || 'default';
|
||||
const persistedChunks = await engine.getChunks(slug, { sourceId: edgeSourceId });
|
||||
const byIndex = new Map<number, { id?: number; symbol_name_qualified?: string | null; start_line?: number | null; end_line?: number | null }>();
|
||||
for (const pc of persistedChunks) {
|
||||
byIndex.set(pc.chunk_index, pc);
|
||||
@@ -1231,6 +1239,17 @@ export async function importCodeFile(
|
||||
from_symbol_qualified: from.symbol_name_qualified,
|
||||
to_symbol_qualified: e.toSymbol,
|
||||
edge_type: e.edgeType,
|
||||
// Stamp the source: getCallersOf/getCalleesOf add
|
||||
// `AND source_id = <scoped>` whenever a worktree pin / --source is
|
||||
// in play, and a NULL here never matches that filter — so every
|
||||
// scoped call-graph query silently returned 0 rows on
|
||||
// multi-source brains even though the edges existed. The fallback
|
||||
// is 'default', NOT null: an unscoped import lands its pages under
|
||||
// the schema default (pages.source_id DEFAULT 'default'), so a
|
||||
// NULL-stamped edge would be invisible to the matching scoped
|
||||
// query getCallersOf(sym, { sourceId: 'default' }) — the same bug
|
||||
// through the other door.
|
||||
source_id: edgeSourceId,
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -5186,6 +5186,52 @@ export const MIGRATIONS: Migration[] = [
|
||||
);
|
||||
`,
|
||||
},
|
||||
{
|
||||
version: 116,
|
||||
name: 'code_edges_source_backfill_and_callee_index',
|
||||
// Repair + index pass for the call graph:
|
||||
//
|
||||
// 1. BACKFILL: importCodeFile built CodeEdgeInput rows without source_id,
|
||||
// so every extracted edge landed NULL. getCallersOf/getCalleesOf add
|
||||
// `AND source_id = <scoped>` whenever a worktree pin / --source is in
|
||||
// play — NULL never matches, so scoped call-graph queries silently
|
||||
// returned 0 rows on multi-source brains even though the edges
|
||||
// existed. The write path now stamps `sourceId ?? 'default'`; this
|
||||
// backfill repairs rows written before the fix by deriving each
|
||||
// edge's source from its own from_chunk's page (pages.source_id is
|
||||
// NOT NULL DEFAULT 'default', so COALESCE is belt-and-braces only).
|
||||
//
|
||||
// 2. INDEXES: getCalleesOf filters BOTH edge tables on
|
||||
// from_symbol_qualified, which had no index anywhere — every callee
|
||||
// lookup was a sequential scan, amplified per-BFS-node by the
|
||||
// recursive code walk (one getCalleesOf per frontier node, up to
|
||||
// maxNodes). With NULL edges repaired, scoped walks actually expand,
|
||||
// so the latent seq-scan cost becomes real. Plain CREATE INDEX (not
|
||||
// CONCURRENTLY): edge tables are modest (mirrors the v58 resolver
|
||||
// index). Keep in sync with src/schema.sql.
|
||||
idempotent: true,
|
||||
sql: `
|
||||
UPDATE code_edges_symbol e
|
||||
SET source_id = COALESCE(p.source_id, 'default')
|
||||
FROM content_chunks c
|
||||
JOIN pages p ON p.id = c.page_id
|
||||
WHERE c.id = e.from_chunk_id
|
||||
AND e.source_id IS NULL;
|
||||
|
||||
UPDATE code_edges_chunk e
|
||||
SET source_id = COALESCE(p.source_id, 'default')
|
||||
FROM content_chunks c
|
||||
JOIN pages p ON p.id = c.page_id
|
||||
WHERE c.id = e.from_chunk_id
|
||||
AND e.source_id IS NULL;
|
||||
|
||||
CREATE INDEX IF NOT EXISTS idx_code_edges_symbol_from_symbol
|
||||
ON code_edges_symbol (from_symbol_qualified);
|
||||
|
||||
CREATE INDEX IF NOT EXISTS idx_code_edges_chunk_from_symbol
|
||||
ON code_edges_chunk (from_symbol_qualified);
|
||||
`,
|
||||
},
|
||||
];
|
||||
|
||||
export const LATEST_VERSION = MIGRATIONS.length > 0
|
||||
|
||||
@@ -4943,7 +4943,7 @@ export class PGLiteEngine implements BrainEngine {
|
||||
e.from_chunk_id, e.to_chunk_id, e.from_symbol_qualified,
|
||||
e.to_symbol_qualified, e.edge_type,
|
||||
JSON.stringify(e.edge_metadata ?? {}),
|
||||
e.source_id ?? null,
|
||||
e.source_id ?? 'default',
|
||||
);
|
||||
}
|
||||
const res = await this.db.query(
|
||||
@@ -4964,7 +4964,7 @@ export class PGLiteEngine implements BrainEngine {
|
||||
params.push(
|
||||
e.from_chunk_id, e.from_symbol_qualified, e.to_symbol_qualified, e.edge_type,
|
||||
JSON.stringify(e.edge_metadata ?? {}),
|
||||
e.source_id ?? null,
|
||||
e.source_id ?? 'default',
|
||||
);
|
||||
}
|
||||
const res = await this.db.query(
|
||||
|
||||
@@ -5169,7 +5169,7 @@ export class PostgresEngine implements BrainEngine {
|
||||
const toQual = resolved.map(e => e.to_symbol_qualified);
|
||||
const edgeTypes = resolved.map(e => e.edge_type);
|
||||
const metas = resolved.map(e => JSON.stringify(e.edge_metadata ?? {}));
|
||||
const sources = resolved.map(e => e.source_id ?? null);
|
||||
const sources = resolved.map(e => e.source_id ?? 'default');
|
||||
const res = await sql`
|
||||
INSERT INTO code_edges_chunk (from_chunk_id, to_chunk_id, from_symbol_qualified, to_symbol_qualified, edge_type, edge_metadata, source_id)
|
||||
SELECT * FROM unnest(
|
||||
@@ -5189,7 +5189,7 @@ export class PostgresEngine implements BrainEngine {
|
||||
const toQual = unresolved.map(e => e.to_symbol_qualified);
|
||||
const edgeTypes = unresolved.map(e => e.edge_type);
|
||||
const metas = unresolved.map(e => JSON.stringify(e.edge_metadata ?? {}));
|
||||
const sources = unresolved.map(e => e.source_id ?? null);
|
||||
const sources = unresolved.map(e => e.source_id ?? 'default');
|
||||
const res = await sql`
|
||||
INSERT INTO code_edges_symbol (from_chunk_id, from_symbol_qualified, to_symbol_qualified, edge_type, edge_metadata, source_id)
|
||||
SELECT * FROM unnest(
|
||||
|
||||
+41
-17
@@ -390,6 +390,11 @@ CREATE INDEX IF NOT EXISTS idx_code_edges_chunk_to
|
||||
ON code_edges_chunk(to_chunk_id, edge_type);
|
||||
CREATE INDEX IF NOT EXISTS idx_code_edges_chunk_to_symbol
|
||||
ON code_edges_chunk(to_symbol_qualified, edge_type);
|
||||
-- getCalleesOf filters on from_symbol_qualified; without this index every
|
||||
-- callee lookup is a sequential scan, amplified per-BFS-node by the
|
||||
-- recursive code walk. Mirrors migration v116.
|
||||
CREATE INDEX IF NOT EXISTS idx_code_edges_chunk_from_symbol
|
||||
ON code_edges_chunk(from_symbol_qualified);
|
||||
|
||||
CREATE TABLE IF NOT EXISTS code_edges_symbol (
|
||||
id SERIAL PRIMARY KEY,
|
||||
@@ -407,16 +412,32 @@ CREATE INDEX IF NOT EXISTS idx_code_edges_symbol_from
|
||||
ON code_edges_symbol(from_chunk_id, edge_type);
|
||||
CREATE INDEX IF NOT EXISTS idx_code_edges_symbol_to
|
||||
ON code_edges_symbol(to_symbol_qualified, edge_type);
|
||||
-- getCalleesOf companion to idx_code_edges_chunk_from_symbol above.
|
||||
-- Mirrors migration v116.
|
||||
CREATE INDEX IF NOT EXISTS idx_code_edges_symbol_from_symbol
|
||||
ON code_edges_symbol(from_symbol_qualified);
|
||||
|
||||
-- ============================================================
|
||||
-- links: cross-references between pages
|
||||
-- ============================================================
|
||||
-- Provenance model (v0.13, extended issue #972):
|
||||
-- link_source — 'markdown' | 'frontmatter' | 'manual' | 'wikilink-resolved' | NULL
|
||||
-- 'wikilink-resolved' is the opt-in
|
||||
-- (link_resolution.global_basename) basename-match
|
||||
-- provenance — see issue #972 / migration v113.
|
||||
-- (NULL = legacy row written before v0.13; unknown source)
|
||||
-- Provenance model (v0.13; opened to kebab provenance in v114 / issue #1941):
|
||||
-- link_source — open kebab-case provenance tag, NOT a closed allowlist.
|
||||
-- Format gate (CHECK): ^[a-z][a-z0-9]*(-[a-z0-9]+)*\$ and
|
||||
-- char_length <= 64. NULL = legacy row (pre-v0.13).
|
||||
-- Reconciliation-managed built-ins written internally:
|
||||
-- 'markdown' — body markdown links
|
||||
-- 'frontmatter' — YAML frontmatter edges (see origin_*)
|
||||
-- 'mentions' — auto-linked body-text mentions
|
||||
-- 'wikilink-resolved'— opt-in global-basename [[name]] (#972)
|
||||
-- User/tool-facing:
|
||||
-- 'manual' — hand- or tool-created edges (the
|
||||
-- add_link op default + CLI link-add)
|
||||
-- '<your-tag>' — external derivers, e.g. 'citation-graph',
|
||||
-- stamp their own kebab tag (no migration).
|
||||
-- The add_link OP forbids callers from passing the four
|
||||
-- managed built-ins (they imply reconciliation semantics a
|
||||
-- hand-created row can't honor); the DB CHECK still admits
|
||||
-- them because internal writers use them. See operations.ts.
|
||||
-- origin_page_id — for link_source='frontmatter', the page whose YAML
|
||||
-- frontmatter created this edge; scopes reconciliation
|
||||
-- origin_field — the frontmatter field name (e.g. 'key_people')
|
||||
@@ -424,21 +445,21 @@ CREATE INDEX IF NOT EXISTS idx_code_edges_symbol_to
|
||||
-- The unique constraint includes link_source + origin_page_id so a manual edge
|
||||
-- and a frontmatter-derived edge with the same (from, to, type) tuple coexist.
|
||||
-- Reconciliation on put_page filters by (link_source='frontmatter' AND
|
||||
-- origin_page_id = written_page) — never touches other pages' edges.
|
||||
-- origin_page_id = written_page) — never touches other pages' edges. (This is
|
||||
-- exactly why a CLI-forged 'frontmatter' row with NULL origin would be a phantom
|
||||
-- edge reconciliation never cleans — hence the op-layer guard.)
|
||||
CREATE TABLE IF NOT EXISTS links (
|
||||
id SERIAL PRIMARY KEY,
|
||||
from_page_id INTEGER NOT NULL REFERENCES pages(id) ON DELETE CASCADE,
|
||||
to_page_id INTEGER NOT NULL REFERENCES pages(id) ON DELETE CASCADE,
|
||||
link_type TEXT NOT NULL DEFAULT '',
|
||||
context TEXT NOT NULL DEFAULT '',
|
||||
-- v114 (#1941): link_source is an open kebab-case provenance, not a closed
|
||||
-- allowlist — external derivers (e.g. 'citation-graph') stamp their own tag
|
||||
-- without a gbrain migration. Format gate only: lowercase kebab, <=64 chars.
|
||||
-- The reconciliation-managed built-ins ('markdown','frontmatter','mentions',
|
||||
-- 'wikilink-resolved') still satisfy this and are written internally; the
|
||||
-- add_link op forbids CALLERS from forging them (see operations.ts). 'manual'
|
||||
-- is the user-facing default for hand/tool-created edges.
|
||||
link_source TEXT CHECK (link_source IS NULL OR (link_source ~ '^[a-z][a-z0-9]*(-[a-z0-9]+)*$' AND char_length(link_source) <= 64)),
|
||||
-- v0.41.18.0: 'mentions' added for auto-linked body-text mentions
|
||||
-- (gbrain extract links --by-mention). Filtered OUT of backlink-count
|
||||
-- for search ranking; only counts toward orphan-ratio + graph traversal.
|
||||
-- v0.40.8.2 (#972): 'wikilink-resolved' added for opt-in global-basename
|
||||
-- wikilink resolution (bare [[name]] resolved by slug tail).
|
||||
link_source TEXT CHECK (link_source IS NULL OR (link_source ~ '^[a-z][a-z0-9]*(-[a-z0-9]+)*\$' AND char_length(link_source) <= 64)),
|
||||
-- v0.41.18.0: nullable link_kind distinguishes "plain body mention" from
|
||||
-- "verb-pattern-derived typed link" within link_source='mentions'.
|
||||
-- Codex finding #12 design: keep link_source stable; add link_kind
|
||||
@@ -675,8 +696,11 @@ CREATE TABLE IF NOT EXISTS op_checkpoints (
|
||||
CREATE INDEX IF NOT EXISTS op_checkpoints_updated_at_idx
|
||||
ON op_checkpoints (updated_at);
|
||||
|
||||
-- #1794: append-only delta storage (one row per completed path). FK cascade
|
||||
-- drops children with the parent. Mirrors migration v115 + src/schema.sql.
|
||||
-- #1794: append-only delta storage. One row per completed path; sync's
|
||||
-- appendCompleted INSERTs only the delta instead of rewriting the whole
|
||||
-- completed_keys JSONB array each flush (O(N^2) -> O(delta)). FK cascade drops
|
||||
-- children with the parent (clearOpCheckpoint + 7-day purge). PK prefix
|
||||
-- (op,fingerprint) serves all reads; no separate index. Mirrors migration v115.
|
||||
CREATE TABLE IF NOT EXISTS op_checkpoint_paths (
|
||||
op TEXT NOT NULL,
|
||||
fingerprint TEXT NOT NULL,
|
||||
|
||||
@@ -386,6 +386,11 @@ CREATE INDEX IF NOT EXISTS idx_code_edges_chunk_to
|
||||
ON code_edges_chunk(to_chunk_id, edge_type);
|
||||
CREATE INDEX IF NOT EXISTS idx_code_edges_chunk_to_symbol
|
||||
ON code_edges_chunk(to_symbol_qualified, edge_type);
|
||||
-- getCalleesOf filters on from_symbol_qualified; without this index every
|
||||
-- callee lookup is a sequential scan, amplified per-BFS-node by the
|
||||
-- recursive code walk. Mirrors migration v116.
|
||||
CREATE INDEX IF NOT EXISTS idx_code_edges_chunk_from_symbol
|
||||
ON code_edges_chunk(from_symbol_qualified);
|
||||
|
||||
CREATE TABLE IF NOT EXISTS code_edges_symbol (
|
||||
id SERIAL PRIMARY KEY,
|
||||
@@ -403,6 +408,10 @@ CREATE INDEX IF NOT EXISTS idx_code_edges_symbol_from
|
||||
ON code_edges_symbol(from_chunk_id, edge_type);
|
||||
CREATE INDEX IF NOT EXISTS idx_code_edges_symbol_to
|
||||
ON code_edges_symbol(to_symbol_qualified, edge_type);
|
||||
-- getCalleesOf companion to idx_code_edges_chunk_from_symbol above.
|
||||
-- Mirrors migration v116.
|
||||
CREATE INDEX IF NOT EXISTS idx_code_edges_symbol_from_symbol
|
||||
ON code_edges_symbol(from_symbol_qualified);
|
||||
|
||||
-- ============================================================
|
||||
-- links: cross-references between pages
|
||||
|
||||
@@ -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)');
|
||||
});
|
||||
});
|
||||
@@ -142,6 +142,27 @@ describe('buildFactsAlterRecipe', () => {
|
||||
const recipe = buildFactsAlterRecipe(1536, 1280, 'halfvec');
|
||||
expect(recipe).toMatch(/DROP INDEX[\s\S]*ALTER TABLE[\s\S]*CREATE INDEX/);
|
||||
});
|
||||
|
||||
test('dimension change NULLs embeddings BEFORE the alter (pgvector refuses cross-dim casts)', () => {
|
||||
// Same defect class fixed for content_chunks in embeddingMismatchMessage:
|
||||
// pgvector aborts a cross-dimension ALTER while rows still hold old-width
|
||||
// vectors. The dims-change recipe must wipe first; order pinned.
|
||||
const recipe = buildFactsAlterRecipe(1536, 1280, 'halfvec');
|
||||
const nullIdx = recipe.indexOf('UPDATE facts SET embedding = NULL');
|
||||
const alterIdx = recipe.indexOf('ALTER TABLE facts ALTER COLUMN embedding TYPE');
|
||||
expect(nullIdx).toBeGreaterThan(-1);
|
||||
expect(alterIdx).toBeGreaterThan(-1);
|
||||
expect(nullIdx).toBeLessThan(alterIdx);
|
||||
});
|
||||
|
||||
test('same-dim type swap PRESERVES embeddings (no NULL wipe)', () => {
|
||||
// halfvec(1536) <-> vector(1536): the USING cast is lossless and the
|
||||
// whole point is keeping the data. A wipe here would destroy valid
|
||||
// embeddings for no reason.
|
||||
const recipe = buildFactsAlterRecipe(1536, 1536, 'vector');
|
||||
expect(recipe).not.toContain('UPDATE facts SET embedding = NULL');
|
||||
expect(recipe).toContain('USING embedding::vector(1536)');
|
||||
});
|
||||
});
|
||||
|
||||
describe('FactsEmbeddingDimMismatchError', () => {
|
||||
|
||||
@@ -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