insights: close the post-merge review findings on workflow intelligence

Five verified findings from the #756 review, each with a regression test:

Corrections now count only follow-up prompts. The session-opening prompt
cannot be correcting this assistant, however correction-shaped its task
text reads, and the revert pattern no longer matches ordinary 'revert
the <noun>' requests (undo was already guarded; the doc claimed both
were).

Time-to-first-edit returns null when the first edit's timestamp is
unparseable instead of silently measuring to the first parseable one.

File churn normalizes backslashed paths before keying, relativizing, and
basenaming. Previously the payload's basename-only privacy redaction was
a no-op on Windows and shipped the full absolute path, username
included; mixed separators also split one file across churn entries.

Pricing coverage excludes deliberately-free models (local-looking,
local-savings mapped, zero-rate overrides) from the denominator via the
same exclusions findUnpricedModels applies, so a mostly-ollama user no
longer reads high coverage while every cost-bearing call is unpriced.
An absent coverage value now reaches the payload as null, never as a
fabricated 100%.

Also shares the one-shot minimum-edit-turns threshold between
worstOneShotCategory and the coaching gate so they cannot drift.

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 ?? [],

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[] {

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 === '<synthetic>' ? 0 : d.calls), 0)
+    .reduce((s, [model, d]) => s + (model === '<synthetic>' || isExpectedFreeModel(model) ? 0 : d.calls), 0)
   const unpricedCalls = unpricedModels.reduce((s, m) => s + m.calls, 0)
   const corrections = scanUserCorrections(projects)
 

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<string, { editTurns: number; oneShotTurns: number }>()
   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.`)
   }
 

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)

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 <noun>" 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)
+  })
+})