From f46cac966d8cbec293972aa49e640b847fcb240c Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Mon, 18 May 2026 13:31:29 -0700 Subject: [PATCH] fix(doctor): resolve whoknows fixture from module location, not cwd MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `gbrain doctor` warned about a missing whoknows fixture for every install that wasn't standing in the gbrain source repo at run time — which is everyone. The check used `process.cwd()` to locate the fixture, so any real user (running doctor against `~/.gbrain`) saw a spurious warning. `resolveWhoknowsFixturePath()` walks up from `import.meta.url` looking for the source-repo signature (`src/cli.ts` + `skills/RESOLVER.md`), respects `GBRAIN_WHOKNOWS_FIXTURE_PATH` env override (absolute or cwd-relative), and returns null with an actionable warning when the fixture can't be located. Closes #969. Cherry-picked from PR #1034. Co-Authored-By: mvanhorn --- src/commands/doctor.ts | 54 ++++++++++++++++--- test/whoknows-doctor.test.ts | 102 ++++++++++++++++++++++++----------- 2 files changed, 118 insertions(+), 38 deletions(-) diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index 6cff082ed..0cdcd89b5 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -9,8 +9,9 @@ import { compareVersions } from './migrations/index.ts'; import { createProgress, startHeartbeat, type ProgressReporter } from '../core/progress.ts'; import { getCliOptions, cliOptsToProgressOptions } from '../core/cli-options.ts'; import type { DbUrlSource } from '../core/config.ts'; -import { join } from 'path'; -import { existsSync, readFileSync, readdirSync } from 'fs'; +import { dirname, isAbsolute, join, resolve as resolvePath } from 'path'; +import { fileURLToPath } from 'url'; +import { existsSync, readFileSync, readdirSync, statSync } from 'fs'; export interface Check { name: string; @@ -86,6 +87,41 @@ export function computeDoctorReport(checks: Check[]): DoctorReport { * Tolerance matches migration v48: any value with abs(weight - on_grid) > 1e-3 * is genuinely off-grid (the 0.05 grid is 5e-2; float32 noise is ~1e-7). */ +const WHOKNOWS_FIXTURE_RELATIVE_PATH = 'test/fixtures/whoknows-eval.jsonl'; + +function isGbrainSourceRoot(dir: string): boolean { + return ( + existsSync(join(dir, 'src', 'cli.ts')) && + existsSync(join(dir, 'skills', 'RESOLVER.md')) + ); +} + +export function resolveWhoknowsFixturePath( + env: NodeJS.ProcessEnv = process.env, + moduleUrl: string = import.meta.url, +): string | null { + if (env.GBRAIN_WHOKNOWS_FIXTURE_PATH) { + return isAbsolute(env.GBRAIN_WHOKNOWS_FIXTURE_PATH) + ? env.GBRAIN_WHOKNOWS_FIXTURE_PATH + : resolvePath(process.cwd(), env.GBRAIN_WHOKNOWS_FIXTURE_PATH); + } + + try { + let dir = dirname(fileURLToPath(moduleUrl)); + for (let i = 0; i < 10; i++) { + if (isGbrainSourceRoot(dir)) return join(dir, WHOKNOWS_FIXTURE_RELATIVE_PATH); + const parent = dirname(dir); + if (parent === dir) break; + dir = parent; + } + } catch { + // Some bundlers/runtimes may not expose a normal file: import URL. + // Doctor should surface an override hint instead of fabricating a path. + } + + return null; +} + /** * v0.33: whoknows_health — verify the eval fixture is present at the * documented path. Lightweight; just checks file existence and row count, @@ -98,15 +134,19 @@ export function computeDoctorReport(checks: Check[]): DoctorReport { */ export async function whoknowsHealthCheck(_engine: BrainEngine): Promise { try { - const { existsSync, readFileSync, statSync } = await import('fs'); - const path = await import('path'); - const repoRoot = process.cwd(); - const fixturePath = path.join(repoRoot, 'test/fixtures/whoknows-eval.jsonl'); + const fixturePath = resolveWhoknowsFixturePath(); + if (!fixturePath) { + return { + name: 'whoknows_health', + status: 'warn', + message: 'whoknows eval fixture path could not be resolved. Set GBRAIN_WHOKNOWS_FIXTURE_PATH to the absolute path for test/fixtures/whoknows-eval.jsonl.', + }; + } if (!existsSync(fixturePath)) { return { name: 'whoknows_health', status: 'warn', - message: `whoknows eval fixture missing at test/fixtures/whoknows-eval.jsonl. Fix: hand-label 10 queries you'd actually run, format {query, expected_top_3_slugs, notes}.`, + message: `whoknows eval fixture missing at ${fixturePath}. Fix: hand-label 10 queries you'd actually run, format {query, expected_top_3_slugs, notes}.`, }; } const stat = statSync(fixturePath); diff --git a/test/whoknows-doctor.test.ts b/test/whoknows-doctor.test.ts index f6d5229a9..16231aea9 100644 --- a/test/whoknows-doctor.test.ts +++ b/test/whoknows-doctor.test.ts @@ -1,14 +1,16 @@ import { describe, it, expect, beforeAll, afterAll, beforeEach } from 'bun:test'; -import { mkdtempSync, rmSync, writeFileSync, mkdirSync, existsSync } from 'fs'; +import { mkdtempSync, rmSync, writeFileSync, mkdirSync } from 'fs'; import { tmpdir } from 'os'; import { join } from 'path'; -import { whoknowsHealthCheck } from '../src/commands/doctor.ts'; +import { resolveWhoknowsFixturePath, whoknowsHealthCheck } from '../src/commands/doctor.ts'; +import { withEnv } from './helpers/with-env.ts'; /** * v0.33 whoknows_health doctor check — fixture-only assertion. The - * check inspects the working-directory fixture file; it does NOT need - * an engine. We pass a sentinel object cast to BrainEngine for the - * type contract since the check intentionally ignores its argument. + * check resolves the shipped fixture from the module path unless + * GBRAIN_WHOKNOWS_FIXTURE_PATH is set. It does NOT need an engine. + * We pass a sentinel object cast to BrainEngine for the type contract + * since the check intentionally ignores its argument. */ const stubEngine = {} as Parameters[0]; @@ -39,12 +41,29 @@ function cleanup() { } describe('whoknows_health doctor check', () => { - it('warns when fixture file is missing entirely', async () => { + it('resolves the default fixture when cwd is not the repo', async () => { try { - const check = await whoknowsHealthCheck(stubEngine); - expect(check.name).toBe('whoknows_health'); - expect(check.status).toBe('warn'); - expect(check.message).toContain('fixture missing'); + await withEnv({ GBRAIN_WHOKNOWS_FIXTURE_PATH: undefined }, async () => { + const check = await whoknowsHealthCheck(stubEngine); + expect(check.name).toBe('whoknows_health'); + expect(check.status).toBe('ok'); + expect(check.message).toContain('queries'); + }); + } finally { + cleanup(); + } + }); + + it('warns when env override fixture file is missing entirely', async () => { + try { + const fixturePath = join(workDir, 'missing-whoknows-eval.jsonl'); + await withEnv({ GBRAIN_WHOKNOWS_FIXTURE_PATH: fixturePath }, async () => { + const check = await whoknowsHealthCheck(stubEngine); + expect(check.name).toBe('whoknows_health'); + expect(check.status).toBe('warn'); + expect(check.message).toContain('fixture missing'); + expect(check.message).toContain(fixturePath); + }); } finally { cleanup(); } @@ -52,11 +71,14 @@ describe('whoknows_health doctor check', () => { it('warns when fixture exists but is empty', async () => { try { - mkdirSync('test/fixtures', { recursive: true }); - writeFileSync('test/fixtures/whoknows-eval.jsonl', ''); - const check = await whoknowsHealthCheck(stubEngine); - expect(check.status).toBe('warn'); - expect(check.message).toContain('empty'); + const fixturePath = join(workDir, 'test/fixtures/whoknows-eval.jsonl'); + mkdirSync(join(workDir, 'test/fixtures'), { recursive: true }); + writeFileSync(fixturePath, ''); + await withEnv({ GBRAIN_WHOKNOWS_FIXTURE_PATH: fixturePath }, async () => { + const check = await whoknowsHealthCheck(stubEngine); + expect(check.status).toBe('warn'); + expect(check.message).toContain('empty'); + }); } finally { cleanup(); } @@ -64,30 +86,36 @@ describe('whoknows_health doctor check', () => { it('warns when fixture has fewer than 5 rows', async () => { try { - mkdirSync('test/fixtures', { recursive: true }); + const fixturePath = join(workDir, 'test/fixtures/whoknows-eval.jsonl'); + mkdirSync(join(workDir, 'test/fixtures'), { recursive: true }); writeFileSync( - 'test/fixtures/whoknows-eval.jsonl', + fixturePath, '{"query":"a","expected_top_3_slugs":["x"]}\n' + '{"query":"b","expected_top_3_slugs":["y"]}\n', ); - const check = await whoknowsHealthCheck(stubEngine); - expect(check.status).toBe('warn'); - expect(check.message).toContain('2 row'); + await withEnv({ GBRAIN_WHOKNOWS_FIXTURE_PATH: fixturePath }, async () => { + const check = await whoknowsHealthCheck(stubEngine); + expect(check.status).toBe('warn'); + expect(check.message).toContain('2 row'); + }); } finally { cleanup(); } }); - it('passes when fixture has at least 5 rows', async () => { + it('honors env override when fixture has at least 5 rows', async () => { try { - mkdirSync('test/fixtures', { recursive: true }); + const fixturePath = join(workDir, 'test/fixtures/whoknows-eval.jsonl'); + mkdirSync(join(workDir, 'test/fixtures'), { recursive: true }); const rows = Array.from({ length: 10 }, (_, i) => JSON.stringify({ query: `q${i}`, expected_top_3_slugs: [`p${i}`] }), ).join('\n'); - writeFileSync('test/fixtures/whoknows-eval.jsonl', rows + '\n'); - const check = await whoknowsHealthCheck(stubEngine); - expect(check.status).toBe('ok'); - expect(check.message).toContain('10 queries'); + writeFileSync(fixturePath, rows + '\n'); + await withEnv({ GBRAIN_WHOKNOWS_FIXTURE_PATH: fixturePath }, async () => { + const check = await whoknowsHealthCheck(stubEngine); + expect(check.status).toBe('ok'); + expect(check.message).toContain('10 queries'); + }); } finally { cleanup(); } @@ -95,7 +123,8 @@ describe('whoknows_health doctor check', () => { it('ignores comment lines and blank lines when counting rows', async () => { try { - mkdirSync('test/fixtures', { recursive: true }); + const fixturePath = join(workDir, 'test/fixtures/whoknows-eval.jsonl'); + mkdirSync(join(workDir, 'test/fixtures'), { recursive: true }); const content = [ '# comment', '// another comment', @@ -106,10 +135,21 @@ describe('whoknows_health doctor check', () => { '{"query":"d","expected_top_3_slugs":["w"]}', '{"query":"e","expected_top_3_slugs":["v"]}', ].join('\n'); - writeFileSync('test/fixtures/whoknows-eval.jsonl', content + '\n'); - const check = await whoknowsHealthCheck(stubEngine); - expect(check.status).toBe('ok'); - expect(check.message).toContain('5 queries'); + writeFileSync(fixturePath, content + '\n'); + await withEnv({ GBRAIN_WHOKNOWS_FIXTURE_PATH: fixturePath }, async () => { + const check = await whoknowsHealthCheck(stubEngine); + expect(check.status).toBe('ok'); + expect(check.message).toContain('5 queries'); + }); + } finally { + cleanup(); + } + }); + + it('returns null when the default fixture path cannot be resolved', () => { + try { + const fixturePath = resolveWhoknowsFixturePath({}, 'not-a-file-url'); + expect(fixturePath).toBeNull(); } finally { cleanup(); }