mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-28 14:59:47 +00:00
* fix: bound tree-sitter chunker + harden walker + plumb strategy
`gbrain sync --strategy code` against a 1500-file repo could pin one
thread at 99% CPU for hours with zero disk writes and a `page_count`
that stayed at 0. Three real defects, all closed in one commit:
1. **Tree-sitter chunker had no wall-clock cap.** A single pathological
file could wedge the whole sync inside WASM. New `parseWithTimeout`
helper in src/core/chunkers/code.ts wraps `parser.parse()` with
`setTimeoutMicros(timeoutMs * 1000)`, throws `ChunkerTimeoutError`
on null, and the caller's try/finally reaps parser+tree (closes the
leak codex flagged where the catch block returned without delete()).
Default 30s, override via `GBRAIN_CHUNKER_TIMEOUT_MS`. Falls back to
recursive chunks on timeout — degrades search quality on that one
file, doesn't wedge sync.
2. **Code-strategy first-sync silently no-op'd on code files.**
`performFullSync` called `runImport(repoPath)` with no strategy;
`runImport` only ever walked `.md`/`.mdx`. Now `opts.strategy`
threads end-to-end (full-sync write path AND dry-run). Code files
actually reach the dispatcher, which already routes them to
`importCodeFile` correctly.
3. **Walker was thrice-redundant.** `collectMarkdownFiles` (lstat-safe,
import path) and `walkSyncableFiles` (statSync, cost-preview path,
weaker for no good reason) collapsed into one hardened
`collectSyncableFiles` in src/commands/import.ts: lstat + symlink-
skip with canonical log line; inode-cycle Map keyed on
`${st_dev}:${st_ino}` (defense-in-depth for non-symlink loops);
`MAX_WALK_DEPTH=32` structural backstop with `GBRAIN_MAX_WALK_DEPTH`
override; `.sort()` output (codex C8: `runImport`'s checkpoint
resume is index-based against a sorted list). Walker-context
multimodal carve-out preserved at one site (codex C5).
Plus structured `[gbrain phase] <name> start/done` stderr lines on
git_pull, fullsync.import, collect_files, and per-file slow path
(>5s). When the next hang lands, log says which phase wedged.
Tests:
- `test/sync-walker-symlink.test.ts` — 7 cases (self-symlink loop,
symlink-chain inode cycle, max-depth bailout, strategy filter,
dot-dir skip, multimodal preservation, deterministic ordering)
- `test/chunker-timeout.test.ts` — 7 cases (parser-stub seam,
ChunkerTimeoutError shape, env wiring, fallback behavior, fail-loud
if setTimeoutMicros API missing, cleanup contract under exception)
Smoke against the user's actual amarillo-v2 repo: 494 code files
walked in 22ms, 2 symlinks skipped with the canonical log line.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore: bump version 0.30.1 → 0.31.2 + CHANGELOG + TODOS
VERSION 0.31.2, package.json synced. CHANGELOG entry under [0.31.2]
with full release-summary + numbers + upgrader-cost note + To take
advantage block. v0.30.2 entry preserved below from master. TODOS.md
files the gbrain query <common-keyword> 7-day-zombie investigation
(PIDs 39429, 46624) and the deferred amarillo-shape PGLite + Postgres
E2E as v0.31.3 follow-ups.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(test): use withEnv() helper instead of direct process.env mutation
CI's check-test-isolation lint (rule R1) flagged the two new test files
for mutating process.env directly. The repo-wide convention is to wrap
env mutations in withEnv() (test/helpers/with-env.ts), which saves +
restores prior values via try/finally even when the callback throws.
Direct process.env writes leak across files in the same bun test
process (parallel runner loads multiple files into one shard process).
Both files refactored:
- test/sync-walker-symlink.test.ts (GBRAIN_EMBEDDING_MULTIMODAL)
- test/chunker-timeout.test.ts (GBRAIN_CHUNKER_TIMEOUT_MS)
All 14 cases still pass. `bun run verify` clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
118 lines
4.7 KiB
TypeScript
118 lines
4.7 KiB
TypeScript
/**
|
||
* v0.31.2 chunker timeout regression tests.
|
||
*
|
||
* Closes the bug class where a single pathological code file could
|
||
* wedge the entire sync inside tree-sitter WASM at 99% CPU with no
|
||
* I/O and no observable progress. The fix bounds parser.parse() with
|
||
* setTimeoutMicros and falls back to recursive chunks on timeout.
|
||
*
|
||
* Test design (per codex C9): the runtime parser is non-deterministic
|
||
* about how long it takes to parse arbitrary input, so "force timeout
|
||
* via huge input" is machine-speed dependent and flaky on slow CI. We
|
||
* stub the ParserLike seam directly to assert the timeout contract,
|
||
* then verify the env wiring with a 1ms cap that reliably loses.
|
||
*/
|
||
import { describe, test, expect } from 'bun:test';
|
||
import {
|
||
parseWithTimeout,
|
||
ChunkerTimeoutError,
|
||
chunkCodeTextFull,
|
||
} from '../src/core/chunkers/code.ts';
|
||
import { withEnv } from './helpers/with-env.ts';
|
||
|
||
const REAL_TS = `export function add(a: number, b: number): number { return a + b; }
|
||
export class Counter {
|
||
private n = 0;
|
||
increment(): void { this.n++; }
|
||
get value(): number { return this.n; }
|
||
}
|
||
`;
|
||
|
||
describe('parseWithTimeout — pure-function seam', () => {
|
||
test('1. throws ChunkerTimeoutError when parser.parse returns null', () => {
|
||
const stub = {
|
||
_timeoutCalls: 0,
|
||
_parseCalls: 0,
|
||
setTimeoutMicros(_t: number) { this._timeoutCalls++; },
|
||
parse(_s: string) { this._parseCalls++; return null; },
|
||
};
|
||
|
||
expect(() => parseWithTimeout(stub, 'x', 50, 'foo.ts')).toThrow(ChunkerTimeoutError);
|
||
expect(stub._timeoutCalls).toBe(1);
|
||
expect(stub._parseCalls).toBe(1);
|
||
});
|
||
|
||
test('2. throws clear error if setTimeoutMicros API is missing', () => {
|
||
// A future web-tree-sitter that drops the API must NOT silently
|
||
// regress to no-timeout behavior.
|
||
const stub = {
|
||
parse(_s: string) { return { rootNode: null, delete: () => {} }; },
|
||
} as any;
|
||
|
||
expect(() => parseWithTimeout(stub, 'x', 50, 'foo.ts'))
|
||
.toThrow(/setTimeoutMicros/);
|
||
});
|
||
|
||
test('3. returns the tree on success; calls setTimeoutMicros once', () => {
|
||
const stub = {
|
||
_timeout: 0,
|
||
setTimeoutMicros(t: number) { this._timeout = t; },
|
||
parse(_s: string) { return { rootNode: { type: 'program' }, delete: () => {} }; },
|
||
};
|
||
|
||
const tree = parseWithTimeout(stub, 'x', 50, 'foo.ts') as { rootNode: { type: string } };
|
||
expect(tree.rootNode.type).toBe('program');
|
||
expect(stub._timeout).toBe(50_000); // microseconds = ms × 1000
|
||
});
|
||
|
||
test('4. ChunkerTimeoutError carries filePath + timeoutMs for actionable logs', () => {
|
||
const stub = { setTimeoutMicros() {}, parse() { return null; } };
|
||
try {
|
||
parseWithTimeout(stub, 'x', 30_000, 'src/big.ts');
|
||
throw new Error('should have thrown');
|
||
} catch (e: unknown) {
|
||
expect(e).toBeInstanceOf(ChunkerTimeoutError);
|
||
expect((e as ChunkerTimeoutError).filePath).toBe('src/big.ts');
|
||
expect((e as ChunkerTimeoutError).timeoutMs).toBe(30_000);
|
||
}
|
||
});
|
||
});
|
||
|
||
describe('chunkCodeTextFull — integration with real parser', () => {
|
||
test('5. default-timeout path on real code completes well under cap', async () => {
|
||
const result = await chunkCodeTextFull(REAL_TS, 'sample.ts');
|
||
expect(result.chunks.length).toBeGreaterThan(0);
|
||
});
|
||
|
||
test('6. GBRAIN_CHUNKER_TIMEOUT_MS=1 reliably forces fallback', async () => {
|
||
// 1ms is below any plausible real-parser wall-clock. The parser
|
||
// returns null; chunkCodeTextFull catches the ChunkerTimeoutError
|
||
// and falls back to fallbackChunks (recursive text chunker).
|
||
await withEnv({ GBRAIN_CHUNKER_TIMEOUT_MS: '1' }, async () => {
|
||
const result = await chunkCodeTextFull(REAL_TS, 'sample.ts');
|
||
// Recursive fallback still produces chunks; assertion is that the
|
||
// call returned cleanly instead of hanging.
|
||
expect(Array.isArray(result.chunks)).toBe(true);
|
||
expect(result.edges).toEqual([]); // edges only emitted on tree-sitter path
|
||
});
|
||
});
|
||
});
|
||
|
||
describe('cleanup contract under exception', () => {
|
||
test('7. parser.delete() still fires when timeout throws (codex C4)', async () => {
|
||
// chunkCodeTextFull's finally must reap parser+tree even when
|
||
// parseWithTimeout throws ChunkerTimeoutError. We can't directly
|
||
// observe parser.delete() in a real WASM run, but we can test the
|
||
// shape: forcing the env-timeout path through chunkCodeTextFull a
|
||
// few times must not leak (smoke test — Bun GC won't catch a real
|
||
// WASM leak but if cleanup were missing on the throw path, repeated
|
||
// calls would visibly degrade).
|
||
await withEnv({ GBRAIN_CHUNKER_TIMEOUT_MS: '1' }, async () => {
|
||
for (let i = 0; i < 5; i++) {
|
||
const result = await chunkCodeTextFull(REAL_TS, `sample-${i}.ts`);
|
||
expect(Array.isArray(result.chunks)).toBe(true);
|
||
}
|
||
});
|
||
});
|
||
});
|