From cbb09d0170fad6804e1937ed1ffbba0e7de1cd96 Mon Sep 17 00:00:00 2001 From: "Dongmin,Yu" Date: Tue, 18 Aug 2026 10:30:11 +0900 Subject: [PATCH] fix(optimize): scope transcript-derived findings to the selected provider (#1003) * fix(optimize): scope transcript-derived findings to the selected provider scanSessions() always ran discoverAllSessions('claude'), so every finding it feeds was computed from Claude transcripts regardless of --provider, while the header (sessions, calls, cost) came from the already-filtered projects. Under --provider codex the two described different providers, and the Claude-derived numbers read as the selected provider's. Skip the scan when the filter excludes Claude, and skip the detectors it feeds rather than handing them an empty scan: emptiness reads as "never invoked", so an empty scan turned every skill, agent and command into a reported ghost. Findings derived from projects (MCP tool coverage, capability reliability, low-worth sessions, context bloat, outliers, model recommendations) already filter correctly and still run. The result cache key now carries the provider, since the provider decides whether the scan runs at all. * fix(optimize): thread the provider through the apply, aggregator and TUI scans The previous commit fixed one of four scanAndDetect callers. The other three carry a provider filter and dropped it: - act/optimize-apply.ts: `optimize --apply` branches in main.ts before the code that threads it, so a Codex-scoped run planned applies off Claude findings. This is the worst of the three because `unused-skills` is appliable and its plan moves directories out of ~/.claude/skills. - usage-aggregator.ts: AggregateOpts.provider was already honoured for the usage half but not for the optimize half, so the menubar, desktop and web surfaces carried the same mismatch. - dashboard.tsx: `p` cycles activeProvider and `o` opens optimize off the same state, so the TUI could show Claude findings under a Codex view. activeProvider joins the callback deps; reloadData already clears optimizeResult on a provider switch, so no extra invalidation is needed. Covered by a dry-run test on the apply path: under `provider: 'codex'` a fake home holding an uninvoked skill must plan nothing, and under 'claude' the same fixture must still plan the archive. --- src/act/optimize-apply.ts | 6 ++- src/dashboard.tsx | 4 +- src/main.ts | 6 +-- src/optimize.ts | 68 +++++++++++++++++++--------- src/usage-aggregator.ts | 2 +- tests/optimize-fs.test.ts | 93 ++++++++++++++++++++++++++++++++++++++- 6 files changed, 149 insertions(+), 30 deletions(-) diff --git a/src/act/optimize-apply.ts b/src/act/optimize-apply.ts index 5b90685a..b72af509 100644 --- a/src/act/optimize-apply.ts +++ b/src/act/optimize-apply.ts @@ -13,6 +13,10 @@ export type ApplyOptions = { yes?: boolean dryRun?: boolean only?: string + // Mirrors `optimize --provider`. The scan below only reads Claude + // transcripts, and this path does not just report findings, it plans and + // applies them - a Codex-scoped run must never offer to edit ~/.claude. + provider?: string actionsDir?: string ctx?: PlanContext // Test seams: crafted findings skip the session scan; streams default to @@ -103,7 +107,7 @@ export async function runOptimizeApply( let costRate = opts.costRate ?? 0 if (!findings) { errout.write(chalk.dim(' Analyzing your sessions...\n')) - const scanned = await scanAndDetect(projects, dateRange) + const scanned = await scanAndDetect(projects, dateRange, opts.provider) findings = scanned.findings costRate = scanned.costRate } diff --git a/src/dashboard.tsx b/src/dashboard.tsx index b66b41cf..9cc3db50 100644 --- a/src/dashboard.tsx +++ b/src/dashboard.tsx @@ -1460,14 +1460,14 @@ export function InteractiveDashboard({ initialProjects, initialDailyHistoryProje const generation = reloadGenerationRef.current setOptimizeLoading(true) try { - const result = await scanAndDetect(projects, currentRange()) + const result = await scanAndDetect(projects, currentRange(), activeProvider) if (reloadGenerationRef.current === generation) setOptimizeResult(result) } catch (error) { console.error(error) } finally { if (reloadGenerationRef.current === generation) setOptimizeLoading(false) } - }, [optimizeAvailable, projects, currentRange, optimizeLoading, optimizeResult]) + }, [optimizeAvailable, projects, currentRange, optimizeLoading, optimizeResult, activeProvider]) useEffect(() => { const refreshIntervalMs = getRefreshIntervalMs(refreshSeconds ?? 0) diff --git a/src/main.ts b/src/main.ts index d201920b..aa3063a4 100644 --- a/src/main.ts +++ b/src/main.ts @@ -1831,7 +1831,7 @@ program const projects = await parseAllSessions(range, opts.provider) if (opts.apply) { const { runOptimizeApply } = await import('./act/optimize-apply.js') - await runOptimizeApply(projects, range, { yes: opts.yes, dryRun: opts.dryRun, only: opts.only }) + await runOptimizeApply(projects, range, { yes: opts.yes, dryRun: opts.dryRun, only: opts.only, provider: opts.provider }) return } assertFormat(format, ['text', 'json'], 'optimize') @@ -1849,9 +1849,9 @@ program appliedHeader = buildOptimizeAppliedHeader(applied) ?? undefined previouslyApplied = applied.appliedByFinding } catch { /* the header is optional; never block the findings */ } - await runOptimize(projects, label, range, { format, appliedHeader, previouslyApplied }) + await runOptimize(projects, label, range, { format, appliedHeader, previouslyApplied, provider: opts.provider }) } else { - await runOptimize(projects, label, range, { format }) + await runOptimize(projects, label, range, { format, provider: opts.provider }) } }) diff --git a/src/optimize.ts b/src/optimize.ts index 63d49330..190a8a8b 100644 --- a/src/optimize.ts +++ b/src/optimize.ts @@ -551,7 +551,18 @@ export async function scanJsonlFile( return { calls, cwds, apiCalls, userMessages } } -async function scanSessions(dateRange?: DateRange): Promise { +// The session scan reads Claude Code transcripts only, so a `--provider` that +// excludes Claude leaves nothing for it to do. Callers must also skip the +// detectors it feeds (see `claudeOnly` in scanAndDetect) — the empty scan +// returned here is an absence of measurement, not a measurement of absence. +export function providerCoversClaude(provider?: string): boolean { + return !provider || provider === 'all' || provider === 'claude' +} + +async function scanSessions(dateRange?: DateRange, provider?: string): Promise { + if (!providerCoversClaude(provider)) { + return { toolCalls: [], projectCwds: new Set(), apiCalls: [], userMessages: [] } + } const sources = await discoverAllSessions('claude') const allCalls: ToolCall[] = [] const allCwds = new Set() @@ -2977,7 +2988,7 @@ export function computeInputCostRate(projects: ProjectSummary[]): number { type CacheEntry = { data: OptimizeResult; ts: number } const resultCache = new Map() -export function cacheKey(projects: ProjectSummary[], dateRange: DateRange | undefined): string { +export function cacheKey(projects: ProjectSummary[], dateRange: DateRange | undefined, provider?: string): string { const dr = dateRange ? `${dateRange.start.getTime()}-${dateRange.end.getTime()}` : 'all' // Fingerprint enough of the dataset that two materially different inputs // cannot collide onto one cached OptimizeResult. Project count + api-call @@ -2994,23 +3005,27 @@ export function cacheKey(projects: ProjectSummary[], dateRange: DateRange | unde } // Costs scaled to whole micro-dollars so float jitter cannot thrash the key. const fingerprint = `${projects.length}:${calls}:${Math.round(cost * 1e6)}:${Math.round(savings * 1e6)}:${Math.round(proxied * 1e6)}` - return `${dr}:${fingerprint}` + // The provider decides whether the Claude session scan runs at all, so two + // filters that happen to share a project fingerprint must not share a result. + return `${provider ?? 'all'}:${dr}:${fingerprint}` } export async function scanAndDetect( projects: ProjectSummary[], dateRange?: DateRange, + provider?: string, ): Promise { if (projects.length === 0) { return { findings: [], costRate: 0, healthScore: 100, healthGrade: 'A', modelRecommendations: [] } } - const key = cacheKey(projects, dateRange) + const key = cacheKey(projects, dateRange, provider) const cached = resultCache.get(key) if (cached && Date.now() - cached.ts < RESULT_CACHE_TTL_MS) return cached.data const costRate = computeInputCostRate(projects) - const { toolCalls, projectCwds, apiCalls, userMessages } = await scanSessions(dateRange) + const scanCoversClaude = providerCoversClaude(provider) + const { toolCalls, projectCwds, apiCalls, userMessages } = await scanSessions(dateRange, provider) const mcpCoverage = aggregateMcpCoverage(projects) const findings: WasteFinding[] = [] @@ -3025,35 +3040,44 @@ export async function scanAndDetect( ) const firstSessionIds = findYoungProjectFirstSessionIds(projects) const outlierExclusions = new Set([...lowWorthSessionIds, ...contextBloatVisibleIds, ...firstSessionIds]) + // Detectors fed by the session scan or by `~/.claude` config only mean + // anything when the run covers Claude. Under a different `--provider` they + // must be skipped rather than handed an empty scan: emptiness reads as + // "never invoked", so every skill, agent and command would be reported as + // unused when it was simply not measured. + const claudeOnly = (detect: () => WasteFinding | null): (() => WasteFinding | null) => + scanCoversClaude ? detect : () => null const syncDetectors: Array<() => WasteFinding | null> = [ - () => detectCacheBloat(apiCalls, projects, dateRange), - () => detectLowReadEditRatio(toolCalls), - () => detectJunkReads(toolCalls, dateRange), - () => detectDuplicateReads(toolCalls, dateRange), - () => detectUnusedMcp(toolCalls, projects, projectCwds, mcpCoverage), + claudeOnly(() => detectCacheBloat(apiCalls, projects, dateRange)), + claudeOnly(() => detectLowReadEditRatio(toolCalls)), + claudeOnly(() => detectJunkReads(toolCalls, dateRange)), + claudeOnly(() => detectDuplicateReads(toolCalls, dateRange)), + claudeOnly(() => detectUnusedMcp(toolCalls, projects, projectCwds, mcpCoverage)), () => detectMcpToolCoverage(projects, mcpCoverage), () => detectMcpProfileAdvisor(projects, mcpCoverage), // mcp-deferral-gaps family (#614): detection only, no apply plans yet. - () => detectMcpDeferralOff(toolCalls, projects, projectCwds, apiCalls), - () => detectMcpAlwaysLoadHygiene(projects, projectCwds, apiCalls, mcpCoverage), - () => detectMcpDeferThreshold(projects, projectCwds), + claudeOnly(() => detectMcpDeferralOff(toolCalls, projects, projectCwds, apiCalls)), + claudeOnly(() => detectMcpAlwaysLoadHygiene(projects, projectCwds, apiCalls, mcpCoverage)), + claudeOnly(() => detectMcpDeferThreshold(projects, projectCwds)), () => detectCapabilityReliability(projects), () => detectLowWorthSessions(projects), () => detectContextBloat(projects, lowWorthSessionIds), () => detectSessionOutliers(projects, outlierExclusions), - () => detectBloatedClaudeMd(projectCwds), - () => detectBashBloat(), + claudeOnly(() => detectBloatedClaudeMd(projectCwds)), + claudeOnly(() => detectBashBloat()), ] for (const detect of syncDetectors) { const finding = detect() if (finding) findings.push(finding) } - const ghostResults = await Promise.all([ - detectGhostAgents(toolCalls), - detectGhostSkills(toolCalls), - detectGhostCommands(userMessages), - ]) + const ghostResults = scanCoversClaude + ? await Promise.all([ + detectGhostAgents(toolCalls), + detectGhostSkills(toolCalls), + detectGhostCommands(userMessages), + ]) + : [] for (const f of ghostResults) if (f) findings.push(f) findings.sort((a, b) => urgencyScore(b) - urgencyScore(a)) @@ -3281,7 +3305,7 @@ export async function runOptimize( projects: ProjectSummary[], periodLabel: string, dateRange?: DateRange, - opts: { format?: 'text' | 'json'; appliedHeader?: string; previouslyApplied?: Record } = {}, + opts: { format?: 'text' | 'json'; appliedHeader?: string; previouslyApplied?: Record; provider?: string } = {}, ): Promise { const format = opts.format ?? 'text' if (projects.length === 0 && format === 'text') { @@ -3293,7 +3317,7 @@ export async function runOptimize( process.stderr.write(chalk.dim(' Analyzing your sessions...\n')) } - const result = await scanAndDetect(projects, dateRange) + const result = await scanAndDetect(projects, dateRange, opts.provider) const { findings, costRate, healthScore, healthGrade } = result const sessions = projects.flatMap(p => p.sessions) const periodCost = projects.reduce((s, p) => s + p.totalCostUSD, 0) diff --git a/src/usage-aggregator.ts b/src/usage-aggregator.ts index 82aeb732..4350413b 100644 --- a/src/usage-aggregator.ts +++ b/src/usage-aggregator.ts @@ -935,7 +935,7 @@ export async function buildMenubarPayloadForRange(periodInfo: PeriodInfo, opts: } })() - const optimize = opts.optimize === false ? null : await scanAndDetect(scanProjects, scanRange) + const optimize = opts.optimize === false ? null : await scanAndDetect(scanProjects, scanRange, opts.provider) const granularRange = opts.daysSelection?.range ?? scanRange const granularHistory = opts.timeline === false ? undefined : buildGranularHistory(scanProjects, granularRange) return buildMenubarPayload(currentData, providers, optimize, dailyHistory, retryTax, routingWaste, breakdowns, claudeConfigs, granularHistory) diff --git a/tests/optimize-fs.test.ts b/tests/optimize-fs.test.ts index 2364e08a..7f9d76ea 100644 --- a/tests/optimize-fs.test.ts +++ b/tests/optimize-fs.test.ts @@ -1,4 +1,5 @@ -import { describe, it, expect, afterAll, beforeEach, vi } from 'vitest' +import { describe, it, expect, afterAll, afterEach, beforeEach, vi } from 'vitest' +import { Writable } from 'node:stream' import { mkdtempSync, rmSync, mkdirSync, writeFileSync, utimesSync } from 'fs' import { tmpdir } from 'os' import { join } from 'path' @@ -29,6 +30,8 @@ import { estimateContextBudget, discoverProjectCwd, } from '../src/context-budget.js' +import type { ProjectSummary } from '../src/types.js' +import { runOptimizeApply } from '../src/act/optimize-apply.js' // ============================================================================ // Helpers for filesystem fixtures @@ -371,6 +374,94 @@ describe('scanAndDetect', () => { expect(result.healthGrade).toBe('A') expect(result.costRate).toBe(0) }) + + // The session scan only ever reads Claude Code transcripts, so under a + // non-Claude --provider it used to report Claude-derived findings beside a + // header scoped to the other provider - e.g. `optimize --provider codex` + // printing a read/edit ratio counted from Claude sessions. + describe('provider scoping', () => { + // These fixtures live in the shared fake home, so they have to come back + // out: later suites in this file assert on an otherwise empty ~/.claude. + const CLAUDE_DIR = join(FAKE_HOME_FOR_MOCK, '.claude') + afterEach(() => { + for (const sub of ['projects', 'skills']) { + rmSync(join(CLAUDE_DIR, sub), { recursive: true, force: true }) + } + }) + + function claudeSessionWithEditHeavyTurns(): void { + const projectDir = join(CLAUDE_DIR, 'projects', 'provider-scope') + mkdirSync(projectDir, { recursive: true }) + const now = new Date().toISOString() + const entry = (name: string, file: string) => JSON.stringify({ + type: 'assistant', timestamp: now, + message: { content: [{ type: 'tool_use', name, input: { file_path: file } }] }, + }) + const lines = [entry('Read', '/src/a.ts')] + for (let i = 0; i < 12; i++) lines.push(entry('Edit', `/src/f${i}.ts`)) + writeFileSync(join(projectDir, 'session.jsonl'), lines.join('\n')) + } + + // scanAndDetect memoises on (provider, range, project fingerprint) for 60s, + // and the cache is module-level, so tests that differ only in what is on + // disk would serve each other's results. `seed` moves the fingerprint so + // each case scans for real. + function projectFixture(seed: number): ProjectSummary { + return { + project: 'provider-scope', + projectPath: '/tmp/provider-scope', + sessions: [], + totalCostUSD: 1, + totalApiCalls: 13 + seed, + } as unknown as ProjectSummary + } + + it('reports transcript-derived findings when scoped to claude', async () => { + claudeSessionWithEditHeavyTurns() + const result = await scanAndDetect([projectFixture(1)], undefined, 'claude') + expect(result.findings.map(f => f.id)).toContain('read-edit-ratio') + }) + + it('omits transcript-derived findings when scoped to another provider', async () => { + claudeSessionWithEditHeavyTurns() + mkdirSync(join(CLAUDE_DIR, 'skills', 'never-invoked'), { recursive: true }) + writeFileSync(join(CLAUDE_DIR, 'skills', 'never-invoked', 'SKILL.md'), '# skill\n') + + const result = await scanAndDetect([projectFixture(2)], undefined, 'codex') + const ids = result.findings.map(f => f.id) + + expect(ids).not.toContain('read-edit-ratio') + // An unmeasured skill must not be reported as an unused one: the scan + // returns nothing under this filter, which is not evidence of disuse. + expect(ids).not.toContain('unused-skills') + }) + + // The apply path reaches scanAndDetect through its own entry point, so it + // needs its own guard: `unused-skills` is appliable, and its plan moves + // directories out of ~/.claude/skills. Reporting a Codex-labelled finding + // is a wrong number; offering to archive every skill off one is a wrong + // number with side effects. + async function applyDryRun(provider: string): Promise { + const chunks: string[] = [] + const output = new Writable({ write(c, _e, cb) { chunks.push(String(c)); cb() } }) + const errorOutput = new Writable({ write(_c, _e, cb) { cb() } }) + await runOptimizeApply([projectFixture(3)], undefined, { provider, dryRun: true, output, errorOutput }) + return chunks.join('') + } + + it('plans no applies from Claude findings when scoped to another provider', async () => { + claudeSessionWithEditHeavyTurns() + mkdirSync(join(CLAUDE_DIR, 'skills', 'never-invoked'), { recursive: true }) + writeFileSync(join(CLAUDE_DIR, 'skills', 'never-invoked', 'SKILL.md'), '# skill\n') + + const codex = await applyDryRun('codex') + expect(codex).toContain('No appliable config-class fixes') + expect(codex).not.toContain('never-invoked') + + const claude = await applyDryRun('claude') + expect(claude).toContain('never-invoked') + }) + }) }) // ============================================================================