From ad9fa70587dcfd08a0019effdc75172ca390d845 Mon Sep 17 00:00:00 2001 From: Aditya Vikram Singh <247195684+avs-io@users.noreply.github.com> Date: Wed, 12 Aug 2026 20:57:23 +0530 Subject: [PATCH] fix(optimize): scope MCP apply savings --- src/act/optimize-apply.ts | 3 ++- src/act/report.ts | 2 +- src/optimize.ts | 8 ++++++++ tests/act-report.test.ts | 40 ++++++++++++++++++++++++++++++++++++ tests/mcp-coverage.test.ts | 19 +++++++++-------- tests/optimize-apply.test.ts | 25 ++++++++++++++++++++++ 6 files changed, 87 insertions(+), 10 deletions(-) diff --git a/src/act/optimize-apply.ts b/src/act/optimize-apply.ts index 5b90685a..d85305d5 100644 --- a/src/act/optimize-apply.ts +++ b/src/act/optimize-apply.ts @@ -42,7 +42,8 @@ export function renderApplyList(appliable: FindingPlan[], manual: FindingPlan[], lines.push(chalk.bold(' Appliable config-class fixes:')) appliable.forEach((fp, i) => { const f = fp.finding - const savings = `~${formatTokens(f.tokensSaved)} tokens${costRate > 0 ? `, ~${formatCost(f.tokensSaved * costRate)}` : ''}` + const actionTokensSaved = f.applyTokensSaved ?? f.tokensSaved + const savings = `~${formatTokens(actionTokensSaved)} tokens${costRate > 0 ? `, ~${formatCost(actionTokensSaved * costRate)}` : ''}` lines.push('') lines.push(` ${i + 1}. ${f.title} ${chalk.hex('#FFD700')(`(${savings})`)}`) for (const line of changeLines(fp)) lines.push(chalk.dim(` ${line}`)) diff --git a/src/act/report.ts b/src/act/report.ts index 30c88120..5379ee81 100644 --- a/src/act/report.ts +++ b/src/act/report.ts @@ -688,7 +688,7 @@ export function captureBaseline(finding: WasteFinding, kind: ActionKind, ctx: Ca const common = { windowDays: ctx.windowDays, capturedAt: ctx.now.toISOString(), - estimatedTokens: Math.max(0, Math.round(finding.tokensSaved)), + estimatedTokens: Math.max(0, Math.round(finding.applyTokensSaved ?? finding.tokensSaved)), } if (MCP_KINDS.has(kind)) { diff --git a/src/optimize.ts b/src/optimize.ts index 36301b2a..d1b04769 100644 --- a/src/optimize.ts +++ b/src/optimize.ts @@ -300,6 +300,10 @@ export type WasteFinding = { explanation: string impact: Impact tokensSaved: number + /// Savings attributable to the automatic mutation when it covers only a + /// subset of the finding. Omitted when `tokensSaved` already describes the + /// whole apply action (or when the finding is manual-only). + applyTokensSaved?: number fix: WasteAction trend?: Trend apply?: FindingApply @@ -1066,6 +1070,9 @@ export function detectMcpToolCoverage( // bucket and overstate `tokensSaved`. const cost = estimateMcpSchemaCost(unusedCountsByServer, projects, flaggedServers) const tokensSaved = Math.round(cost.effectiveInputTokens) + const applyTokensSaved = localServers.length > 0 && connectorServers.length > 0 + ? Math.round(estimateMcpSchemaCost(unusedCountsByServer, projects, localServers).effectiveInputTokens) + : undefined const impact: Impact = tokensSaved >= MCP_COVERAGE_HIGH_IMPACT_TOKENS ? 'high' : flagged.length >= UNUSED_MCP_HIGH_THRESHOLD @@ -1105,6 +1112,7 @@ export function detectMcpToolCoverage( `${lines.join('; ')}.${connectorGuidance}`, impact, tokensSaved, + ...(applyTokensSaved !== undefined ? { applyTokensSaved } : {}), fix, ...(localServers.length > 0 ? { apply: { kind: 'mcp-remove' as const, servers: localServers } } diff --git a/tests/act-report.test.ts b/tests/act-report.test.ts index 80834d0d..b40648a0 100644 --- a/tests/act-report.test.ts +++ b/tests/act-report.test.ts @@ -762,3 +762,43 @@ describe('defer baseline capture', () => { expect(b).toBeUndefined() }) }) + +describe('partial-action baseline capture', () => { + it('persists the savings attributable to the local mutation, not the full mixed finding', () => { + const finding: WasteFinding = { + id: 'mcp-low-coverage', + title: '2 MCP servers with low tool coverage', + explanation: '', + impact: 'medium', + tokensSaved: 40_000, + applyTokensSaved: 20_000, + fix: { type: 'command', label: '', text: "claude mcp remove 'filesystem'" }, + apply: { kind: 'mcp-remove', servers: ['filesystem'] }, + } + const sessions = sessionsAt(2, daysAgo(1), { + mcpInventory: Array.from({ length: 20 }, (_, i) => `mcp__filesystem__t${i}`), + }) + + const baseline = captureBaseline(finding, 'mcp-remove', { + projects: [projectOf(sessions)], + coverage: [{ + server: 'filesystem', + toolsAvailable: 20, + toolsInvoked: 0, + unusedTools: [], + invocations: 0, + loadedSessions: 2, + coverageRatio: 0, + }], + windowDays: 14, + now: NOW, + }) + + expect(baseline).toMatchObject({ + estimatedTokens: 20_000, + sessions: 2, + metrics: { filesystem: 8_000 }, + }) + expect(finding.tokensSaved).toBe(40_000) + }) +}) diff --git a/tests/mcp-coverage.test.ts b/tests/mcp-coverage.test.ts index c2443b37..c036301b 100644 --- a/tests/mcp-coverage.test.ts +++ b/tests/mcp-coverage.test.ts @@ -434,18 +434,21 @@ describe('detectMcpToolCoverage', () => { }) it('keeps mixed connector guidance visible while making only the local server executable', () => { - const sessions: SessionSummary[] = [] - for (const server of ['filesystem', 'claude_ai_Slack']) { - const inventory = Array.from({ length: 20 }, (_, i) => `mcp__${server}__t${i}`) - sessions.push( - makeSession({ sessionId: `${server}-a`, inventory }), - makeSession({ sessionId: `${server}-b`, inventory }), - ) - } + const inventory = ['filesystem', 'claude_ai_Slack'].flatMap(server => + Array.from({ length: 20 }, (_, i) => `mcp__${server}__t${i}`), + ) + const sessions: SessionSummary[] = [ + makeSession({ sessionId: 'mixed-a', inventory, turns: [makeTurn([makeCall({ cacheCreation: 50_000 })])] }), + makeSession({ sessionId: 'mixed-b', inventory, turns: [makeTurn([makeCall({ cacheCreation: 50_000 })])] }), + ] const finding = detectMcpToolCoverage([project(sessions)]) expect(finding).not.toBeNull() + // The finding describes both opportunities: 40 unused tool schemas across + // two sessions = 40K effective tokens. The automatic mutation owns only + // the 20 local schemas = 20K; the connector portion remains manual. + expect(finding).toMatchObject({ tokensSaved: 40_000, applyTokensSaved: 20_000 }) expect(finding!.explanation).toContain('claude_ai_Slack') expect(finding!.explanation).toContain('/mcp') expect(finding!.explanation).toContain('claude.ai Settings > Connectors') diff --git a/tests/optimize-apply.test.ts b/tests/optimize-apply.test.ts index d74ded99..0ed495f6 100644 --- a/tests/optimize-apply.test.ts +++ b/tests/optimize-apply.test.ts @@ -5,6 +5,7 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { createHash } from 'node:crypto' import { PassThrough, Writable } from 'node:stream' +import stripAnsi from 'strip-ansi' import { planFor, planFindings, type PlanContext } from '../src/act/plans.js' import { renderApplyList, runOptimizeApply, type ApplyOptions } from '../src/act/optimize-apply.js' @@ -138,6 +139,30 @@ describe('mcp-remove plan', () => { netlify: { command: 'local-netlify' }, }) }) + + it('previews only the savings attributable to a mixed finding local mutation', async () => { + const fx = await makeFixture() + await writeFile(join(fx.home, '.claude.json'), JSON.stringify({ + mcpServers: { filesystem: { command: 'filesystem' } }, + }, null, 2) + '\n') + const finding: WasteFinding = { + id: 'mcp-low-coverage', + title: '2 MCP servers with low tool coverage', + explanation: '', + impact: 'medium', + tokensSaved: 80_000, + applyTokensSaved: 20_000, + fix: { type: 'command', label: '', text: "claude mcp remove 'filesystem'" }, + apply: { kind: 'mcp-remove', servers: ['filesystem'] }, + } + const plans = planFindings([finding], { homeDir: fx.home, cwd: fx.project }) + + const preview = stripAnsi(renderApplyList(plans, [], 0.000002)) + + expect(preview).toContain('(~20.0K tokens, ~$0.040)') + expect(preview).not.toContain('~80.0K tokens') + expect(preview).not.toContain('~$0.160') + }) }) describe('mcp-project-scope plan', () => {