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.
This commit is contained in:
Dongmin,Yu 2026-08-18 10:30:11 +09:00 committed by GitHub
parent eadc99ef99
commit cbb09d0170
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 149 additions and 30 deletions

View file

@ -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
}

View file

@ -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)

View file

@ -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 })
}
})

View file

@ -551,7 +551,18 @@ export async function scanJsonlFile(
return { calls, cwds, apiCalls, userMessages }
}
async function scanSessions(dateRange?: DateRange): Promise<ScanData> {
// 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<ScanData> {
if (!providerCoversClaude(provider)) {
return { toolCalls: [], projectCwds: new Set(), apiCalls: [], userMessages: [] }
}
const sources = await discoverAllSessions('claude')
const allCalls: ToolCall[] = []
const allCwds = new Set<string>()
@ -2977,7 +2988,7 @@ export function computeInputCostRate(projects: ProjectSummary[]): number {
type CacheEntry = { data: OptimizeResult; ts: number }
const resultCache = new Map<string, CacheEntry>()
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<OptimizeResult> {
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<string, string> } = {},
opts: { format?: 'text' | 'json'; appliedHeader?: string; previouslyApplied?: Record<string, string>; provider?: string } = {},
): Promise<void> {
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)

View file

@ -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)

View file

@ -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<string> {
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')
})
})
})
// ============================================================================