diff --git a/src/menubar-json.ts b/src/menubar-json.ts index e696572..c2933c2 100644 --- a/src/menubar-json.ts +++ b/src/menubar-json.ts @@ -241,7 +241,9 @@ export type MenubarPayload = { /// for privacy; distinct sessions and total edit calls per file. topReworkedFiles: Array<{ path: string; sessions: number; edits: number }> /// Share (0-1) of cost-bearing calls that resolved a price. - pricingCoverage: number + /// null when not computable (no scan data on this path) — "unknown" must + /// never render as 100% coverage. + pricingCoverage: number | null retryTax: { totalUSD: number retries: number @@ -477,7 +479,7 @@ export function buildMenubarPayload( topSessions: buildTopSessions(current.topSessions ?? []), workflow: buildWorkflow(current.workflow), topReworkedFiles: buildTopReworkedFiles(current.topReworkedFiles), - pricingCoverage: current.pricingCoverage ?? 1, + pricingCoverage: current.pricingCoverage ?? null, retryTax: retryTax ?? { totalUSD: 0, retries: 0, editTurns: 0, byModel: [] }, routingWaste: routingWaste ?? { totalSavingsUSD: 0, baselineModel: '', baselineCostPerEdit: 0, byModel: [] }, tools: breakdowns?.tools ?? [], diff --git a/src/models.ts b/src/models.ts index 1818975..a863e6e 100644 --- a/src/models.ts +++ b/src/models.ts @@ -724,6 +724,20 @@ function exactPriceOverrideFor(model: string): ModelCosts | null { // raw ids carries cost > 0 and is not flagged. Local-looking models and // models with a local-savings mapping are excluded because $0 is their // correct cost, as are zero-rate USER overrides (explicitly declared free). +/// Models whose $0 cost is CORRECT rather than a pricing gap, mirroring the +/// exclusions findUnpricedModels applies: local-looking models, models mapped +/// to a local-savings baseline, and models an exact zero-rate user override +/// declares free. Used to keep their calls out of the pricing-coverage +/// denominator — otherwise a 95%-ollama user reads high coverage while every +/// genuinely cost-bearing call is unpriced. +export function isExpectedFreeModel(model: string): boolean { + if (looksLikeLocalModel(model)) return true + if (getLocalSavingsBaseline(model)) return true + const costs = getModelCosts(model) + if (costs && !hasBillableRate(costs) && exactPriceOverrideFor(model)) return true + return false +} + export function findUnpricedModels( rows: Iterable<{ model: string; calls: number; cost: number; tokens?: number }>, ): UnpricedModelUsage[] { diff --git a/src/usage-aggregator.ts b/src/usage-aggregator.ts index 75fa6cc..8a8f441 100644 --- a/src/usage-aggregator.ts +++ b/src/usage-aggregator.ts @@ -2,7 +2,7 @@ import { homedir } from 'node:os' import { CATEGORY_LABELS, type ProjectSummary, type TaskCategory, type DateRange } from './types.js' import { type PeriodData, type ProviderCost, type BreakdownArrays, type MenubarPayload, type ClaudeConfigSelector, buildMenubarPayload } from './menubar-json.js' import { parseAllSessions, filterProjectsByName, filterProjectsByDays, filterProjectsByClaudeConfigSource, isSessionHydrationComplete } from './parser.js' -import { findUnpricedModels, getLocalModelSavingsConfigHash, getPriceOverridesConfigHash, getShortModelName } from './models.js' +import { findUnpricedModels, getLocalModelSavingsConfigHash, getPriceOverridesConfigHash, getShortModelName, isExpectedFreeModel } from './models.js' import { getAllProviders, safeDiscoverSessions } from './providers/index.js' import { claude, getClaudeConfigDirs, getDesktopSessionsDir } from './providers/claude.js' import { stat } from 'node:fs/promises' @@ -46,7 +46,7 @@ export function buildPeriodData(label: string, projects: ProjectSummary[]): Peri const unpricedModels = findUnpricedModels(Object.entries(modelTotals) .map(([model, d]) => ({ model, calls: d.calls, cost: d.cost, tokens: d.tokens }))) const costBearingCalls = Object.entries(modelTotals) - .reduce((s, [model, d]) => s + (model === '' ? 0 : d.calls), 0) + .reduce((s, [model, d]) => s + (model === '' || isExpectedFreeModel(model) ? 0 : d.calls), 0) const unpricedCalls = unpricedModels.reduce((s, m) => s + m.calls, 0) const corrections = scanUserCorrections(projects) diff --git a/src/workflow-insights.ts b/src/workflow-insights.ts index 370cec2..a51c37e 100644 --- a/src/workflow-insights.ts +++ b/src/workflow-insights.ts @@ -7,8 +7,10 @@ import { CATEGORY_LABELS, type ProjectSummary, type TaskCategory } from './types // assistant's own apologies). These match a *user* follow-up telling the // assistant it got something wrong. Deliberately conservative: bare "wrong" or // "undo" are excluded because they show up in ordinary task requests ("fix the -// wrong output", "undo the migration"). Every pattern requires a correction -// context so praise like "you were right" or "that's right" never trips it. +// wrong output", "undo the migration"), and "revert"/"undo" only match +// demonstratives ("that", "this", "the last") so an opening request like +// "revert the migration we shipped" never trips them. Every pattern requires a +// correction context so praise like "you were right" never counts. export const USER_CORRECTION_PATTERNS: RegExp[] = [ /\bthat'?s (?:not|n'?t) (?:what|right|correct|it)\b/i, /\bthat'?s (?:wrong|incorrect)\b/i, @@ -16,7 +18,7 @@ export const USER_CORRECTION_PATTERNS: RegExp[] = [ /\bnot what I (?:meant|wanted|asked|said)\b/i, /\bno,? I (?:meant|wanted|said|asked for)\b/i, /\byou (?:missed|forgot|misunderstood|broke)\b/i, - /\brevert (?:that|it|this|the|your)\b/i, + /\brevert (?:that|it|this|your|the last|the change)\b/i, /\bundo (?:that|it|this|your|the last|the change)\b/i, /\bwrong (?:file|approach|place|method|function|answer|way|direction)\b/i, /\bstill (?:wrong|broken|failing|not working)\b/i, @@ -39,10 +41,20 @@ export function scanUserCorrections(projects: ProjectSummary[]): UserCorrectionS let userTurns = 0 for (const project of projects) { for (const session of project.sessions) { + // A correction is a FOLLOW-UP by definition: the session's opening + // prompt cannot be correcting this assistant, however correction-shaped + // its task text reads ("revert the last release", "you broke it" pasted + // from a bug report). Skipping it trades a few missed corrections in + // resumed transcripts for never counting openers — the conservative side. + let sawPrompt = false for (const turn of session.turns) { const msg = turn.userMessage if (!msg || !msg.trim()) continue userTurns++ + if (!sawPrompt) { + sawPrompt = true + continue + } if (matchesCorrection(msg)) corrections++ } } @@ -64,7 +76,10 @@ export function sessionTimeToFirstEditMs(session: ProjectSummary['sessions'][num for (const call of turn.assistantCalls) { if (!callHasEditTools(call.tools)) continue const editMs = Date.parse(call.timestamp) - if (Number.isNaN(editMs)) continue + // The FIRST edit is the measurement target; if its timestamp is + // unparseable the session's time-to-first-edit is unknown. Skipping to + // the next parseable edit would silently measure a later one. + if (Number.isNaN(editMs)) return null // Clamp: out-of-order timestamps across resumed transcripts would // otherwise pull the median negative. return Math.max(0, editMs - startMs) @@ -93,11 +108,20 @@ export type ReworkedFile = { edits: number } +/// Forward-slash normalization: tool calls carry model-emitted paths verbatim, +/// so on Windows they arrive backslashed. Without this the relativize and +/// basename passes (which split on '/') are no-ops there and the full absolute +/// path, username included, would ship in a payload that can leave the machine. +function normalizeSlashes(p: string): string { + return p.replace(/\\/g, '/') +} + function relativizePath(absPath: string, projectPath: string): string { - if (projectPath && (absPath === projectPath || absPath.startsWith(projectPath + '/'))) { - return absPath.slice(projectPath.length + 1) || absPath + const project = normalizeSlashes(projectPath) + if (project && (absPath === project || absPath.startsWith(project + '/'))) { + return absPath.slice(project.length + 1) || absPath } - const home = homedir() + const home = normalizeSlashes(homedir()) if (absPath === home || absPath.startsWith(home + '/')) return '~' + absPath.slice(home.length) return absPath } @@ -119,10 +143,13 @@ export function aggregateFileChurn(projects: ProjectSummary[], limit = 15): Rewo for (const step of call.toolSequence) { for (const tc of step) { if (!EDIT_TOOLS.has(tc.tool) || !tc.file) continue - let acc = byPath.get(tc.file) + // Key on the normalized path so the same file emitted with + // backslashes and forward slashes accumulates as one entry. + const file = normalizeSlashes(tc.file) + let acc = byPath.get(file) if (!acc) { - acc = { path: relativizePath(tc.file, project.projectPath), sessions: new Set(), edits: 0 } - byPath.set(tc.file, acc) + acc = { path: relativizePath(file, project.projectPath), sessions: new Set(), edits: 0 } + byPath.set(file, acc) } acc.sessions.add(session.sessionId) acc.edits++ @@ -150,10 +177,14 @@ export function computePricingCoverage(totalCostBearingCalls: number, unpricedCa export type CategoryOneShot = { category: string; rate: number; editTurns: number } +/// Minimum edit turns before a category's one-shot rate is trusted (shared by +/// worstOneShotCategory and the coaching gate so they can never disagree). +export const MIN_ONE_SHOT_EDIT_TURNS = 5 + /// The task category with the weakest one-shot rate (over enough edit turns to /// trust), used by the coaching notes. Rate is a percentage (0-100), matching /// model-efficiency and the report's category one-shot figures. -export function worstOneShotCategory(projects: ProjectSummary[], minEditTurns = 5): CategoryOneShot | null { +export function worstOneShotCategory(projects: ProjectSummary[], minEditTurns = MIN_ONE_SHOT_EDIT_TURNS): CategoryOneShot | null { const acc = new Map() for (const project of projects) { for (const session of project.sessions) { @@ -204,7 +235,7 @@ export function buildCoachingNotes(input: CoachingInput): string[] { const notes: string[] = [] const ws = input.worstOneShot - if (ws && ws.editTurns >= 5 && ws.rate < ONE_SHOT_LOW_PERCENT) { + if (ws && ws.editTurns >= MIN_ONE_SHOT_EDIT_TURNS && ws.rate < ONE_SHOT_LOW_PERCENT) { notes.push(`One-shot rate on ${ws.category} is ${Math.round(ws.rate)}% over ${ws.editTurns} edit turns. Add the constraints up front or split the work into smaller edits.`) } diff --git a/tests/menubar-json.test.ts b/tests/menubar-json.test.ts index bb771e3..2958489 100644 --- a/tests/menubar-json.test.ts +++ b/tests/menubar-json.test.ts @@ -38,6 +38,9 @@ describe('buildMenubarPayload', () => { expect(payload.current.label).toBe('7 Days') expect(payload.current.cost).toBe(1248.01) expect(payload.current.calls).toBe(11231) + // An absent pricingCoverage is UNKNOWN and must render as null, never as + // a fabricated 100% coverage (post-#756 review finding). + expect(payload.current.pricingCoverage).toBeNull() expect(payload.current.sessions).toBe(97) expect(payload.current.inputTokens).toBe(19100) expect(payload.current.outputTokens).toBe(675600) diff --git a/tests/workflow-insights.test.ts b/tests/workflow-insights.test.ts index e566e69..e900d20 100644 --- a/tests/workflow-insights.test.ts +++ b/tests/workflow-insights.test.ts @@ -3,12 +3,14 @@ import { describe, expect, it } from 'vitest' import { scanUserCorrections, medianTimeToFirstEditMs, + sessionTimeToFirstEditMs, aggregateFileChurn, computePricingCoverage, worstOneShotCategory, buildCoachingNotes, USER_CORRECTION_PATTERNS, } from '../src/workflow-insights.js' +import { isExpectedFreeModel } from '../src/models.js' import type { ClassifiedTurn, ParsedApiCall, ProjectSummary, SessionSummary, TaskCategory, ToolCall } from '../src/types.js' function call(opts: { tools?: string[]; timestamp?: string; toolSequence?: ToolCall[][]; model?: string; costUSD?: number } = {}): ParsedApiCall { @@ -74,13 +76,13 @@ function cat(editTurns: number, oneShotTurns: number): SessionSummary['categoryB } describe('scanUserCorrections', () => { - it('counts turns whose user message signals a correction', () => { + it('counts follow-up turns whose user message signals a correction', () => { const p = project([session('s1', [ + turn({ userMessage: "add a new feature please" }), turn({ userMessage: "no, I meant the other file" }), turn({ userMessage: "that's not what I asked for" }), turn({ userMessage: "you missed the edge case" }), turn({ userMessage: "revert that change" }), - turn({ userMessage: "add a new feature please" }), ])]) const r = scanUserCorrections([p]) expect(r.corrections).toBe(4) @@ -88,6 +90,26 @@ describe('scanUserCorrections', () => { expect(r.correctionRate).toBeCloseTo(0.8) }) + it('never counts the session-opening prompt, however correction-shaped', () => { + // An opener cannot be correcting THIS assistant; "revert the last release" + // as a first message is a task, not a correction. + const p = project([session('s1', [ + turn({ userMessage: 'revert the last release' }), + turn({ userMessage: 'thanks, looks good' }), + ])]) + const r = scanUserCorrections([p]) + expect(r.corrections).toBe(0) + expect(r.userTurns).toBe(2) + }) + + it('does not match "revert the " task phrasing even as a follow-up', () => { + const p = project([session('s1', [ + turn({ userMessage: 'build the feature' }), + turn({ userMessage: 'now revert the migration we shipped last week' }), + ])]) + expect(scanUserCorrections([p]).corrections).toBe(0) + }) + it('does not flag praise or ordinary requests (false-positive guards)', () => { const phrases = [ 'you were right about that', @@ -105,14 +127,15 @@ describe('scanUserCorrections', () => { it('ignores continuation turns with no fresh prompt', () => { const p = project([session('s1', [ + turn({ userMessage: 'build the feature' }), turn({ userMessage: '' }), turn({ userMessage: ' ' }), turn({ userMessage: 'that is wrong' }), ])]) const r = scanUserCorrections([p]) - expect(r.userTurns).toBe(1) + expect(r.userTurns).toBe(2) expect(r.corrections).toBe(1) - expect(r.correctionRate).toBe(1) + expect(r.correctionRate).toBe(0.5) }) it('returns a null rate for empty input', () => { @@ -286,3 +309,35 @@ describe('buildCoachingNotes', () => { expect(highRateLowCount).toEqual([]) }) }) + +// ── Review-findings regressions (post-#756 review) ───────────────────────── +describe('review-findings regressions', () => { + const edit = (ts: string) => call({ tools: ['Edit'], timestamp: ts }) + + it('an unparseable FIRST edit timestamp yields null, never time-to-a-later-edit', () => { + const s = session('s1', [ + turn({ timestamp: '2026-06-01T10:00:00Z', calls: [edit('garbage-timestamp')] }), + turn({ timestamp: '2026-06-01T10:30:00Z', calls: [edit('2026-06-01T10:30:00Z')] }), + ]) + expect(sessionTimeToFirstEditMs(s)).toBeNull() + }) + + it('normalizes backslashed Windows paths: one churn entry, relativized, no username leak', () => { + const p = project([session('s1', [ + turn({ calls: [call({ tools: ['Edit'], toolSequence: [[{ tool: 'Edit', file: 'C:\\work\\proj\\src\\a.ts' }]] })] }), + turn({ calls: [call({ tools: ['Edit'], toolSequence: [[{ tool: 'Edit', file: 'C:/work/proj/src/a.ts' }]] })] }), + ])], 'C:\\work\\proj') + const churn = aggregateFileChurn([p]) + expect(churn).toHaveLength(1) + expect(churn[0]!.path).toBe('src/a.ts') + expect(churn[0]!.edits).toBe(2) + }) + + it('isExpectedFreeModel excludes local-style models from the coverage denominator', () => { + expect(isExpectedFreeModel('qwen3.6:35b-a3b-bf16')).toBe(true) + expect(isExpectedFreeModel('llama-3-8b-q4')).toBe(true) + expect(isExpectedFreeModel('claude-opus-4-8')).toBe(false) + // 95 local calls + 5 unpriced cloud calls: coverage must be 0, not 0.95. + expect(computePricingCoverage(5, 5)).toBe(0) + }) +})