From 74358329e1ddffbfa92b2da641eceffd21771fbb Mon Sep 17 00:00:00 2001 From: Brett Date: Wed, 22 Jul 2026 20:51:04 -0500 Subject: [PATCH] fix(doctor): stop claiming "Brain is at target" when the target is unreachable (#2151) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `gbrain doctor --remediation-plan` printed two consecutive lines that contradicted each other when the brain was below target AND the target was unreachable with autonomous remediation: Brain score: 45/100 → target 90 Target unreachable: max with autonomous remediation is 70/100. No remediations needed. Brain is at target. The second sentence hid the real next step (configure the prereqs that would lift `max_reachable_score`) and made the brain look healthy when it was not. Fix: gate the "Brain is at target" line on `brain_score_current >= targetScore`. When the plan is empty AND the brain is below target, the "Target unreachable" line above is already the user-facing explanation; the `Blocked checks` block below surfaces the manual gap. Extracted `renderRemediationPlanLines(plan, targetScore): string[]` as a pure helper alongside `runRemediationPlan` so the regression coverage asserts on the rendered output directly rather than mocking `console.log`. `runRemediationPlan` now joins the lines verbatim through console.log; behavior is byte-identical for every case other than the fixed contradiction. Five regression tests cover: unreachable-and-below-target (the bug case), reachable-and-at-target, exact-target, below-target-with-plan, unreachable-with-partial-plan. 38 tests across the adjacent doctor test files stay green; `bun run typecheck` clean. --- src/commands/doctor.ts | 60 ++++++++++-- test/doctor-remediation-plan-render.test.ts | 103 ++++++++++++++++++++ 2 files changed, 155 insertions(+), 8 deletions(-) create mode 100644 test/doctor-remediation-plan-render.test.ts diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index 73dc89721..e57025188 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -7777,27 +7777,71 @@ export async function runRemediationPlan( return; } - // Human output - console.log(`Brain score: ${plan.brain_score_current}/100 → target ${targetScore}`); + for (const line of renderRemediationPlanLines(plan, targetScore)) { + console.log(line); + } +} + +/** + * Human-render the remediation plan into a sequence of console lines. + * Exported for unit-test access — `runRemediationPlan` consumes it + * verbatim and only adds the JSON-mode short-circuit. + * + * Gating the "at target" line on `brain_score_current >= targetScore` + * is load-bearing: when the plan is empty AND the target is unreachable, + * the prior shape printed both "Target unreachable: …" and "Brain is at + * target" back-to-back, which contradicted itself and hid the real next + * step (manual prereq config to lift `max_reachable_score`). + */ +export function renderRemediationPlanLines( + plan: RemediationPlanShape, + targetScore: number, +): string[] { + const lines: string[] = []; + lines.push(`Brain score: ${plan.brain_score_current}/100 → target ${targetScore}`); if (plan.target_unreachable) { - console.log(`Target unreachable: max with autonomous remediation is ${plan.max_reachable_score}/100.`); + lines.push(`Target unreachable: max with autonomous remediation is ${plan.max_reachable_score}/100.`); } if (plan.plan.length === 0) { - console.log('No remediations needed. Brain is at target.'); + if (plan.brain_score_current >= targetScore) { + lines.push('No remediations needed. Brain is at target.'); + } + // When brain_score < targetScore and plan is empty, the unreachable + // line (if applicable) is the user-facing explanation; the blocked- + // checks block below surfaces the manual gap. Don't follow with a + // misleading "at target" claim. } else { - console.log(`Plan: ${plan.plan.length} step(s), est ${plan.est_total_seconds}s, est $${plan.est_total_usd_cost.toFixed(2)}`); + lines.push(`Plan: ${plan.plan.length} step(s), est ${plan.est_total_seconds}s, est $${plan.est_total_usd_cost.toFixed(2)}`); for (const step of plan.plan) { const protectedMark = step.protected ? ' [PROTECTED]' : ''; const costMark = step.est_usd_cost ? ` ($${step.est_usd_cost.toFixed(2)})` : ''; - console.log(` ${step.step}. [${step.severity}] ${step.job}${protectedMark} — ${step.rationale}${costMark}`); + lines.push(` ${step.step}. [${step.severity}] ${step.job}${protectedMark} — ${step.rationale}${costMark}`); } } if (plan.blocked.length > 0) { - console.log(`\nBlocked checks (prereq missing):`); + lines.push(`\nBlocked checks (prereq missing):`); for (const b of plan.blocked) { - console.log(` - ${b.check}: ${b.reason}`); + lines.push(` - ${b.check}: ${b.reason}`); } } + return lines; +} + +interface RemediationPlanShape { + brain_score_current: number; + target_unreachable: boolean; + max_reachable_score: number; + plan: Array<{ + step: number; + severity: string; + job: string; + protected?: boolean; + est_usd_cost?: number; + rationale: string; + }>; + est_total_seconds: number; + est_total_usd_cost: number; + blocked: Array<{ check: string; reason: string }>; } /** diff --git a/test/doctor-remediation-plan-render.test.ts b/test/doctor-remediation-plan-render.test.ts new file mode 100644 index 000000000..b73d314ff --- /dev/null +++ b/test/doctor-remediation-plan-render.test.ts @@ -0,0 +1,103 @@ +// Regression coverage for the `gbrain doctor --remediation-plan` verdict +// contradiction: when the brain was below target AND the target was +// unreachable, the human renderer printed "Target unreachable: max with +// autonomous remediation is N/100" followed immediately by "No +// remediations needed. Brain is at target." — two consecutive lines that +// contradicted each other and hid the real next step. + +import { describe, test, expect } from 'bun:test'; +import { renderRemediationPlanLines } from '../src/commands/doctor.ts'; + +type Plan = Parameters[0]; + +function planFixture(overrides: Partial): Plan { + return { + brain_score_current: 0, + target_unreachable: false, + max_reachable_score: 100, + plan: [], + est_total_seconds: 0, + est_total_usd_cost: 0, + blocked: [], + ...overrides, + }; +} + +describe('renderRemediationPlanLines', () => { + test('unreachable + brain below target — never claims "Brain is at target"', () => { + const plan = planFixture({ + brain_score_current: 45, + target_unreachable: true, + max_reachable_score: 70, + plan: [], + blocked: [{ check: 'link_density', reason: 'no enrichment keys configured' }], + }); + const text = renderRemediationPlanLines(plan, 90).join('\n'); + expect(text).toContain('Brain score: 45/100'); + expect(text).toContain('Target unreachable: max with autonomous remediation is 70/100'); + expect(text).not.toContain('Brain is at target'); + expect(text).toContain('Blocked checks'); + }); + + test('reachable, brain at or above target, no plan — emits the "at target" line', () => { + const plan = planFixture({ + brain_score_current: 95, + target_unreachable: false, + max_reachable_score: 100, + plan: [], + }); + const text = renderRemediationPlanLines(plan, 90).join('\n'); + expect(text).toContain('Brain is at target'); + expect(text).not.toContain('Target unreachable'); + }); + + test('brain at exact target with empty plan — still "at target"', () => { + const plan = planFixture({ + brain_score_current: 90, + target_unreachable: false, + plan: [], + }); + const text = renderRemediationPlanLines(plan, 90).join('\n'); + expect(text).toContain('Brain is at target'); + }); + + test('brain below target with plan steps — lists the plan, no "at target" line', () => { + const plan = planFixture({ + brain_score_current: 60, + target_unreachable: false, + max_reachable_score: 100, + est_total_seconds: 120, + est_total_usd_cost: 0.4, + plan: [ + { step: 1, severity: 'high', job: 'embed-coverage', rationale: 'missing embeddings' }, + { step: 2, severity: 'med', job: 'consolidate', rationale: 'pending entity merges', est_usd_cost: 0.4 }, + ], + }); + const lines = renderRemediationPlanLines(plan, 90); + const text = lines.join('\n'); + expect(text).toContain('Plan: 2 step(s)'); + expect(text).toContain('1. [high] embed-coverage'); + expect(text).toContain('2. [med] consolidate'); + expect(text).toContain('($0.40)'); + expect(text).not.toContain('Brain is at target'); + }); + + test('unreachable but a partial plan exists — plan prints, "at target" suppressed', () => { + const plan = planFixture({ + brain_score_current: 30, + target_unreachable: true, + max_reachable_score: 55, + est_total_seconds: 90, + est_total_usd_cost: 0.2, + plan: [ + { step: 1, severity: 'high', job: 'embed-coverage', rationale: 'reach max_reachable' }, + ], + blocked: [{ check: 'enrichment', reason: 'no provider key configured' }], + }); + const text = renderRemediationPlanLines(plan, 90).join('\n'); + expect(text).toContain('Target unreachable: max with autonomous remediation is 55/100'); + expect(text).toContain('Plan: 1 step(s)'); + expect(text).toContain('Blocked checks'); + expect(text).not.toContain('Brain is at target'); + }); +});