From 117aa833cc8e5eb646f9dbddc7d29cd22ba557ce Mon Sep 17 00:00:00 2001 From: iamtoruk Date: Tue, 18 Aug 2026 08:22:33 -0700 Subject: [PATCH] fix(optimize): scope the apply-able subtotal to the local MCP subset A mixed local + claude.ai connector finding is class `fix`, but `--apply` only mutates the local servers. classTotals now credits the `fix` group with `applyTokensSaved` when present, so the "Fix now (apply-able)" subtotal, the "apply-able: ~$X" headline and `summary.byClass.fix` (CLI, TUI and desktop all read these) describe what apply can actually recover. The finding keeps the whole opportunity in its own `tokensSaved`. Also fixes the desktop connector fixture, which predated the class/basis fields, and adds class-level coverage: connector-only findings resolve to `nudge` (no apply payload), a local server named like a connector stays manual-only, and local-only findings keep their full subtotal. --- app/renderer/sections/Optimize.test.tsx | 3 ++ src/optimize.ts | 14 ++++-- tests/mcp-coverage.test.ts | 57 +++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 3 deletions(-) diff --git a/app/renderer/sections/Optimize.test.tsx b/app/renderer/sections/Optimize.test.tsx index 663084a3..70013522 100644 --- a/app/renderer/sections/Optimize.test.tsx +++ b/app/renderer/sections/Optimize.test.tsx @@ -214,11 +214,14 @@ describe('Optimize', () => { id: 'mcp-low-coverage', title: 'Underused claude.ai connector', explanation: 'The connector loads unused tools.', severity: 'medium', trend: null, tokensSaved: 2_000, estimatedSavingsUSD: 1, + // Connector-only: no appliable plan, so the finding is a nudge. + class: 'nudge', basis: 'estimated', fix: { type: 'paste', destination: 'manual', label: 'Manage the connector where it loads:', text: 'Open /mcp and disable claude.ai Google Calendar.', }, }) + report.summary.byClass.nudge = { tokensSaved: 19_400, savingsUSD: 9.7, count: 2 } getOptimizeReport.mockResolvedValue(report) render() const row = await screen.findByRole('button', { name: /Underused claude.ai connector/ }) diff --git a/src/optimize.ts b/src/optimize.ts index ea740cb8..631ba38f 100644 --- a/src/optimize.ts +++ b/src/optimize.ts @@ -403,9 +403,17 @@ export function classTotals(findings: WasteFinding[], costRate: number): Record< keep: { tokensSaved: 0, savingsUSD: 0, count: 0 }, } for (const f of findings) { - const t = totals[findingClass(f)] - t.tokensSaved += f.tokensSaved - t.savingsUSD += f.tokensSaved * costRate + const cls = findingClass(f) + // A `fix` whose plan owns only part of its estimate (a mixed local + + // claude.ai connector MCP finding) contributes only the apply-able + // subset, so this subtotal and the "apply-able" headline never promise + // what `--apply` cannot recover. The finding keeps the whole + // opportunity in its own `tokensSaved`, so the fix subtotal can be + // smaller than the findings listed under it. + const tokens = cls === 'fix' ? f.applyTokensSaved ?? f.tokensSaved : f.tokensSaved + const t = totals[cls] + t.tokensSaved += tokens + t.savingsUSD += tokens * costRate t.count++ } return totals diff --git a/tests/mcp-coverage.test.ts b/tests/mcp-coverage.test.ts index 77301bc5..0dc7f36c 100644 --- a/tests/mcp-coverage.test.ts +++ b/tests/mcp-coverage.test.ts @@ -3,6 +3,8 @@ import { describe, it, expect, vi } from 'vitest' import { aggregateMcpCoverage, buildOptimizeJsonReport, + classTotals, + findingClass, detectMcpProfileAdvisor, detectMcpToolCoverage, estimateMcpSchemaCost, @@ -925,3 +927,58 @@ describe('detectMcpProfileAdvisor', () => { expect(detectMcpProfileAdvisor(projects, coverage)).toBeNull() }) }) + +// --------------------------------------------------------------------------- +// Connector findings under the fix/nudge/keep classification (#1019) +// --------------------------------------------------------------------------- + +describe('connector findings and finding class', () => { + const inventoryFor = (servers: string[]) => servers.flatMap(server => + Array.from({ length: 20 }, (_, i) => `mcp__${server}__t${i}`), + ) + const twoSessions = (servers: string[]) => ['a', 'b'].map(sessionId => makeSession({ + sessionId, + inventory: inventoryFor(servers), + turns: [makeTurn([makeCall({ cacheCreation: 50_000 })])], + })) + + it('classifies a connector-only finding as a nudge, since nothing is appliable', () => { + const finding = detectMcpToolCoverage([project(twoSessions(['claude_ai_Gmail']))]) + + expect(finding).not.toBeNull() + expect(finding!.apply).toBeUndefined() + expect(findingClass(finding!)).toBe('nudge') + expect(finding!.tokensSaved).toBeGreaterThan(0) + // Never lands in the "apply-able" subtotal. + expect(classTotals([finding!], 0.00002).fix).toEqual({ tokensSaved: 0, savingsUSD: 0, count: 0 }) + }) + + it('counts only the local subset of a mixed finding towards the apply-able subtotal', () => { + const finding = detectMcpToolCoverage([project(twoSessions(['filesystem', 'claude_ai_Slack']))]) + + expect(finding).not.toBeNull() + expect(findingClass(finding!)).toBe('fix') + expect(finding).toMatchObject({ tokensSaved: 40_000, applyTokensSaved: 20_000 }) + expect(classTotals([finding!], 0.00002).fix).toEqual({ tokensSaved: 20_000, savingsUSD: 0.4, count: 1 }) + }) + + it("leaves a local-only finding's subtotal at its full estimate", () => { + const finding = detectMcpToolCoverage([project(twoSessions(['filesystem']))]) + + expect(finding).not.toBeNull() + expect(finding!.applyTokensSaved).toBeUndefined() + expect(classTotals([finding!], 0.00002).fix.tokensSaved).toBe(finding!.tokensSaved) + }) + + it('treats a local server named like a connector namespace as manual only', () => { + // Known limitation: the namespace prefix is the only connector signal in + // the transcript, so a local server literally named claude_ai_* gets the + // manual /mcp guidance rather than a remove command. Conservative by + // design: CodeBurn never emits a command that could hit a connector. + const finding = detectMcpToolCoverage([project(twoSessions(['claude_ai_homegrown']))]) + + expect(finding!.fix.type).toBe('paste') + expect(finding!.apply).toBeUndefined() + expect(findingClass(finding!)).toBe('nudge') + }) +})