diff --git a/src/compare-stats.ts b/src/compare-stats.ts index 1e3db2d6..0068f86a 100644 --- a/src/compare-stats.ts +++ b/src/compare-stats.ts @@ -2,6 +2,7 @@ import { readdir, readFile } from 'fs/promises' import { join } from 'path' import type { ProjectSummary } from './types.js' +import { getShortModelName } from './models.js' const PLANNING_TOOLS = new Set(['TaskCreate', 'TaskUpdate', 'TodoWrite', 'EnterPlanMode', 'ExitPlanMode']) @@ -73,6 +74,16 @@ export function aggregateModelStats(projects: ProjectSummary[]): ModelStats[] { return [...byModel.values()].sort((a, b) => b.cost - a.cost) } +/// Look up a model by the exact canonical id (what --model-a/--model-b has +/// always accepted) or, failing that, by its display name (what the compare +/// picker actually shows the user, e.g. "Opus 4.8") - case-insensitively. +/// Reuses getShortModelName, the existing canonical -> display mapping, +/// rather than a new alias table. +export function findModelStat(models: ModelStats[], input: string): ModelStats | undefined { + return models.find(m => m.model === input) + ?? models.find(m => getShortModelName(m.model).toLowerCase() === input.toLowerCase()) +} + export type ComparisonRow = { section: string label: string diff --git a/src/compare.tsx b/src/compare.tsx index 76dccf27..e9dbc37a 100644 --- a/src/compare.tsx +++ b/src/compare.tsx @@ -2,7 +2,7 @@ import React, { useState, useEffect, useRef } from 'react' import { render, Box, Text, useInput, useApp, useStdout } from 'ink' import type { ModelStats, ComparisonRow, CategoryComparison, WorkingStyleRow } from './compare-stats.js' -import { aggregateModelStats, computeComparison, computeCategoryComparison, computeWorkingStyle, scanSelfCorrections } from './compare-stats.js' +import { aggregateModelStats, computeComparison, computeCategoryComparison, computeWorkingStyle, findModelStat, scanSelfCorrections } from './compare-stats.js' import { formatCost } from './format.js' import { parseAllSessions, setInteractiveScanUI } from './parser.js' import { getAllProviders } from './providers/index.js' @@ -333,9 +333,13 @@ function ComparisonResults({ modelA, modelB, rows, categories, workingStyle, onB type CompareViewProps = { projects: ProjectSummary[] onBack: () => void + // Pre-resolved canonical model ids from --model-a/--model-b (already + // validated to exist in `projects`' aggregated stats by the caller). When + // set, comparison results load immediately instead of showing the picker. + presetModels?: [string, string] } -export function CompareView({ projects, onBack }: CompareViewProps) { +export function CompareView({ projects, onBack, presetModels }: CompareViewProps) { const { exit } = useApp() const [phase, setPhase] = useState<'select' | 'loading' | 'results'>('select') const [models, setModels] = useState(() => aggregateModelStats(projects)) @@ -347,13 +351,13 @@ export function CompareView({ projects, onBack }: CompareViewProps) { } return recs }) - const [pickedNames, setPickedNames] = useState<[string, string] | null>(null) + const [pickedNames, setPickedNames] = useState<[string, string] | null>(presetModels ?? null) const [selectedA, setSelectedA] = useState(null) const [selectedB, setSelectedB] = useState(null) const [rows, setRows] = useState([]) const [categories, setCategories] = useState([]) const [style, setStyle] = useState([]) - const [loadTrigger, setLoadTrigger] = useState(0) + const [loadTrigger, setLoadTrigger] = useState(presetModels ? 1 : 0) const projectsRef = useRef(projects) projectsRef.current = projects @@ -504,7 +508,7 @@ export function CompareView({ projects, onBack }: CompareViewProps) { ) } -export async function renderCompare(range: DateRange, provider: string): Promise { +export async function renderCompare(range: DateRange, provider: string, modelA?: string, modelB?: string): Promise { // Interactive Ink UI: suppress the CLI scan-progress line for the whole // lifetime so it can't print over the rendered comparison. Plain CLI // commands still show progress. @@ -517,8 +521,28 @@ export async function renderCompare(range: DateRange, provider: string): Promise patchStdoutForWindows() const projects = await parseAllSessions(range, provider) + + // --model-a/--model-b: resolve up front (by canonical id or display name, + // same lookup the JSON path uses) so the TUI jumps straight to results + // instead of ignoring the flags and showing the picker. + let presetModels: [string, string] | undefined + if (modelA && modelB) { + const models = aggregateModelStats(projects) + const a = findModelStat(models, modelA) + const b = findModelStat(models, modelB) + if (!a) { + process.stderr.write(`codeburn compare: model not found: "${modelA}".\n`) + process.exit(1) + } + if (!b) { + process.stderr.write(`codeburn compare: model not found: "${modelB}".\n`) + process.exit(1) + } + presetModels = [a.model, b.model] + } + const { waitUntilExit } = render( - process.exit(0)} /> + process.exit(0)} presetModels={presetModels} /> ) await waitUntilExit() } diff --git a/src/dashboard.tsx b/src/dashboard.tsx index 1b68a222..3b889bfb 100644 --- a/src/dashboard.tsx +++ b/src/dashboard.tsx @@ -3,7 +3,7 @@ import { homedir } from 'os' import React, { useState, useCallback, useEffect, useRef } from 'react' import { render, Box, Text, useInput, useApp, useWindowSize } from 'ink' import { CATEGORY_LABELS, type DateRange, type ProjectSummary, type TaskCategory } from './types.js' -import { formatCost, formatTokens, markEstimated } from './format.js' +import { formatCost, formatTokens, markEstimated, carriedCostNote } from './format.js' import { aggregateModelEfficiency } from './model-efficiency.js' import { parseAllSessions, filterProjectsByDateRange, filterProjectsByName, setInteractiveScanUI } from './parser.js' import { findUnpricedModels, loadPricing } from './models.js' @@ -37,6 +37,16 @@ export function scrollHistoryCursor(cursor: number, direction: -1 | 1, pageSize: return Math.max(0, Math.min(cursor + direction, maxCursor)) } +// The Daily Activity panel's row count comes from a bounded live scan (see +// getDashboardScanRange), which can undercount vs. the durable-cache-backed +// Overview headline for the same period (expired session files aren't in the +// live scan but are still in the durable cache). "days scanned" names that +// population so the two counts read as different questions, not a +// contradiction. +export function dailyActivityFooter(cursor: number, days: number, rowCount: number): string { + return `Showing ${cursor + 1}–${Math.min(cursor + days, rowCount)} of ${rowCount} days scanned · newest first` +} + // Scrollable mode keeps the dashboard up when only the selected period is // empty (full history still renders), but a truly-new user with no history at // all should still get the clean empty state instead of a zeroed shell. @@ -143,6 +153,11 @@ export type DurableOverview = { outputTokens: number cacheReadTokens: number cacheWriteTokens: number + // Cost from days whose session logs have since expired, carried forward + // from the daily cache. Surfaced so the Overview headline can explain why + // its total may exceed what the (live-scan-bounded) Daily Activity panel + // below can show. See carriedCostNote in format.ts. + carriedCostUSD: number } async function computeDurableOverview( @@ -154,7 +169,7 @@ async function computeDurableOverview( day: string | null, ): Promise { const range = day ? getDayRange(day) : customRange ?? getPeriodRange(period) - const { data } = await buildDurablePeriod( + const { data, carriedCostUSD } = await buildDurablePeriod( { range, label: PERIOD_LABELS[period] }, { provider, project: projectFilter ?? [], exclude: excludeFilter ?? [] }, ) @@ -167,6 +182,7 @@ async function computeDurableOverview( outputTokens: data.outputTokens, cacheReadTokens: data.cacheReadTokens, cacheWriteTokens: data.cacheWriteTokens, + carriedCostUSD, } } @@ -315,6 +331,9 @@ function Overview({ projects, label, width, planUsages, durable }: { projects: P saved by local models )} + {durable && carriedCostNote(durable.carriedCostUSD) && ( + {carriedCostNote(durable.carriedCostUSD)} + )} {activePlanUsages.length > 0 && ( <> {activePlanUsages.map(planUsage => { @@ -379,7 +398,7 @@ function DailyActivity({ projects, days = 14, pw, bw, scrollable = false, cursor ))} {scrollable && orderedRows.length > 0 && ( - Showing {cursor + 1}–{Math.min(cursor + days, orderedRows.length)} of {orderedRows.length} · newest first + {dailyActivityFooter(cursor, days, orderedRows.length)} )} } diff --git a/src/format.ts b/src/format.ts index 4ba27349..46aac14f 100644 --- a/src/format.ts +++ b/src/format.ts @@ -15,6 +15,16 @@ export function markEstimated(costStr: string, isEstimated: boolean): string { return isEstimated ? `~${costStr}` : costStr } +/// Shared wording for the durable-cache carry-forward footnote: some of a +/// period's total came from days whose session logs have since expired, but +/// the figure is real (preserved in the durable daily cache). overview.ts and +/// dashboard.tsx both show this so a headline that includes carried days +/// doesn't read as inconsistent with detail views that can only see +/// surviving session files. +export function carriedCostNote(carriedCostUSD: number): string | null { + return carriedCostUSD > 0 ? `includes ${formatCost(carriedCostUSD)} preserved from expired session logs` : null +} + export function formatTokens(n: number): string { // Guard against Infinity / NaN / negatives that would otherwise leak into // the UI as "Infinity" or "NaN" strings when an upstream calculation glitches. diff --git a/src/main.ts b/src/main.ts index ae6867cf..a2e09371 100644 --- a/src/main.ts +++ b/src/main.ts @@ -1857,7 +1857,7 @@ program await loadPricing() const { range, label } = getDateRange(opts.period) if (opts.format === 'json') { - const { aggregateModelStats, buildCompareJson, renderCompareJson, scanSelfCorrections } = await import('./compare-stats.js') + const { aggregateModelStats, buildCompareJson, findModelStat, renderCompareJson, scanSelfCorrections } = await import('./compare-stats.js') const projects = await parseAllSessions(range, opts.provider) const models = aggregateModelStats(projects) @@ -1880,8 +1880,8 @@ program process.stderr.write('codeburn compare: --model-a and --model-b must be provided together.\n') process.exit(1) } - const modelA = models.find(model => model.model === opts.modelA) - const modelB = models.find(model => model.model === opts.modelB) + const modelA = findModelStat(models, opts.modelA) + const modelB = findModelStat(models, opts.modelB) if (!modelA) { process.stderr.write(`codeburn compare: model not found: "${opts.modelA}".\n`) process.exit(1) @@ -1893,7 +1893,13 @@ program process.stdout.write(renderCompareJson(buildCompareJson(projects, modelA, modelB, label, opts.provider)) + '\n') return } - await renderCompare(range, opts.provider) + if (opts.modelA || opts.modelB) { + if (!opts.modelA || !opts.modelB) { + process.stderr.write('codeburn compare: --model-a and --model-b must be provided together.\n') + process.exit(1) + } + } + await renderCompare(range, opts.provider, opts.modelA, opts.modelB) }) program diff --git a/src/web-dashboard.ts b/src/web-dashboard.ts index 83d42903..283146b8 100644 --- a/src/web-dashboard.ts +++ b/src/web-dashboard.ts @@ -322,8 +322,12 @@ export async function runWebDashboard(opts: { writeJsonError(res, 400, 'provider (claude|codex) and id are required') return } + // findClaudeSession/findCodexSession resolve an id prefix the same way + // `codeburn context ` does (see context-tree.ts's resolveSession) - + // trust that match instead of re-requiring the full id here, so a prefix + // that works on the CLI works through the API too. const ref = provider === 'claude' ? await findClaudeSession(id) : await findCodexSession(id) - if (!ref || ref.sessionId !== id) { + if (!ref) { writeJsonError(res, 404, `no ${provider} session ${id}`) return } diff --git a/tests/cli-emitters.test.ts b/tests/cli-emitters.test.ts index 1611409d..6ef862ed 100644 --- a/tests/cli-emitters.test.ts +++ b/tests/cli-emitters.test.ts @@ -79,6 +79,40 @@ describe('CLI JSON emitters', () => { } }) + // Issue #767 item 1: --model-a/--model-b used to require the canonical id + // verbatim, with no way to pass the display name the compare picker itself + // shows (e.g. "Sonnet 4.5" for claude-sonnet-4-5). + it('resolves --model-a/--model-b by display name', async () => { + const home = await makeHome() + try { + const result = runCli([ + 'compare', '--format', 'json', '--period', 'all', '--provider', 'claude', + '--model-a', 'Sonnet 4.5', '--model-b', 'Opus 4.5', + ], home) + expect(result.status, result.stderr).toBe(0) + const report = JSON.parse(result.stdout) + expect(report.modelA.model).toBe('claude-sonnet-4-5') + expect(report.modelB.model).toBe('claude-opus-4-5') + } finally { + await rm(home, { recursive: true, force: true }) + } + }) + + // Issue #767 item 1: the TUI path (default --format) used to silently + // ignore --model-a/--model-b entirely. It should at least apply the same + // "both or neither" validation the JSON path already had, rather than + // dropping a lone --model-a on the floor. + it('rejects a lone --model-a for the TUI (default) format same as JSON', async () => { + const home = await makeHome() + try { + const result = runCli(['compare', '--period', 'all', '--provider', 'claude', '--model-a', 'claude-sonnet-4-5'], home) + expect(result.status).not.toBe(0) + expect(result.stderr).toMatch(/--model-a and --model-b must be provided together/) + } finally { + await rm(home, { recursive: true, force: true }) + } + }) + it('emits flat session rows as JSON', async () => { const home = await makeHome() try { diff --git a/tests/compare-stats.test.ts b/tests/compare-stats.test.ts index 02a9e9ac..f110264a 100644 --- a/tests/compare-stats.test.ts +++ b/tests/compare-stats.test.ts @@ -2,7 +2,7 @@ import { mkdtemp, mkdir, rm, writeFile } from 'fs/promises' import { join } from 'path' import { tmpdir } from 'os' import { describe, it, expect, beforeEach, afterEach } from 'vitest' -import { aggregateModelStats, buildCompareJson, computeComparison, computeCategoryComparison, computeWorkingStyle, renderCompareJson, scanSelfCorrections, type ModelStats } from '../src/compare-stats.js' +import { aggregateModelStats, buildCompareJson, computeComparison, computeCategoryComparison, computeWorkingStyle, findModelStat, renderCompareJson, scanSelfCorrections, type ModelStats } from '../src/compare-stats.js' import type { ProjectSummary, SessionSummary, ClassifiedTurn } from '../src/types.js' function makeTurn(model: string, cost: number, opts: { hasEdits?: boolean; retries?: number; outputTokens?: number; inputTokens?: number; cacheRead?: number; cacheWrite?: number; timestamp?: string; category?: string; hasAgentSpawn?: boolean; hasPlanMode?: boolean; speed?: 'standard' | 'fast'; tools?: string[] } = {}): ClassifiedTurn { @@ -596,3 +596,30 @@ describe('computeWorkingStyle', () => { expect(planning.valueA).toBeCloseTo(50) }) }) + +// Issue #767 item 1: `codeburn compare --model-a/--model-b` (both the JSON +// path and the TUI picker) required the canonical model id (e.g. +// claude-opus-4-8) with no way to pass the display name shown in the picker +// itself (e.g. "Opus 4.8"). findModelStat is the shared lookup both surfaces +// use, reusing getShortModelName (the existing canonical -> display mapping) +// instead of a new alias table. +describe('findModelStat', () => { + it('matches the exact canonical model id', () => { + const project = makeProject([makeTurn('claude-opus-4-6', 0.10)]) + const stats = aggregateModelStats([project]) + expect(findModelStat(stats, 'claude-opus-4-6')?.model).toBe('claude-opus-4-6') + }) + + it('matches the display name, case-insensitively', () => { + const project = makeProject([makeTurn('claude-opus-4-6', 0.10)]) + const stats = aggregateModelStats([project]) + expect(findModelStat(stats, 'opus 4.6')?.model).toBe('claude-opus-4-6') + expect(findModelStat(stats, 'Opus 4.6')?.model).toBe('claude-opus-4-6') + }) + + it('returns undefined for an unknown model', () => { + const project = makeProject([makeTurn('claude-opus-4-6', 0.10)]) + const stats = aggregateModelStats([project]) + expect(findModelStat(stats, 'claude-opus-9-9')).toBeUndefined() + }) +}) diff --git a/tests/context-tree-api-prefix.test.ts b/tests/context-tree-api-prefix.test.ts new file mode 100644 index 00000000..f95ee096 --- /dev/null +++ b/tests/context-tree-api-prefix.test.ts @@ -0,0 +1,81 @@ +import { mkdtemp, mkdir, rm, writeFile } from 'fs/promises' +import { tmpdir } from 'os' +import { join } from 'path' +import type { AddressInfo } from 'net' +import type { Server } from 'http' + +import { afterEach, beforeEach, describe, expect, it } from 'vitest' + +import { runWebDashboard } from '../src/web-dashboard.js' + +// Issue #767 item 2: `codeburn context ` accepts an 8-char session id +// prefix (findClaudeSession does a startsWith match), but /api/context/tree +// additionally required the resolved ref's sessionId to equal the id passed +// in verbatim -- so the same prefix that works on the CLI 404s through the +// API. findClaudeSession is the single source of truth for prefix resolution +// on both surfaces; the API should trust it exactly like the CLI does instead +// of re-validating for an exact match afterwards. +const SESSION_ID = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' +const SESSION_PREFIX = SESSION_ID.slice(0, 8) + +describe('web dashboard /api/context/tree: session id prefix', () => { + let server: Server + let base: string + let homeDir: string + let cacheDir: string + + beforeEach(async () => { + homeDir = await mkdtemp(join(tmpdir(), 'codeburn-context-home-')) + cacheDir = await mkdtemp(join(tmpdir(), 'codeburn-context-cache-')) + process.env['HOME'] = homeDir + process.env['CODEBURN_CACHE_DIR'] = cacheDir + delete process.env['CLAUDE_CONFIG_DIR'] + + const projectDir = join(homeDir, '.claude', 'projects', '-tmp-fixture-project') + await mkdir(projectDir, { recursive: true }) + await writeFile(join(projectDir, `${SESSION_ID}.jsonl`), JSON.stringify({ + type: 'assistant', + sessionId: SESSION_ID, + timestamp: '2026-07-01T00:00:00.000Z', + cwd: '/tmp/fixture-project', + message: { + id: 'msg-1', + type: 'message', + role: 'assistant', + model: 'claude-sonnet-4-5', + content: [{ type: 'text', text: 'hi' }], + usage: { input_tokens: 100, output_tokens: 50 }, + }, + }) + '\n') + + server = await runWebDashboard({ + period: 'today', provider: 'all', project: [], exclude: [], port: 0, open: false, + }) + base = `http://127.0.0.1:${(server.address() as AddressInfo).port}` + }) + + afterEach(async () => { + await new Promise((resolve) => server.close(() => resolve())) + await rm(homeDir, { recursive: true, force: true }) + await rm(cacheDir, { recursive: true, force: true }) + }) + + it('resolves a full session id (control case)', async () => { + const res = await fetch(`${base}/api/context/tree?provider=claude&id=${SESSION_ID}`) + expect(res.status).toBe(200) + const payload = await res.json() as { session: { sessionId: string } } + expect(payload.session.sessionId).toBe(SESSION_ID) + }) + + it('resolves an 8-char session id prefix, matching the CLI', async () => { + const res = await fetch(`${base}/api/context/tree?provider=claude&id=${SESSION_PREFIX}`) + expect(res.status).toBe(200) + const payload = await res.json() as { session: { sessionId: string } } + expect(payload.session.sessionId).toBe(SESSION_ID) + }) + + it('still 404s for an id that matches nothing', async () => { + const res = await fetch(`${base}/api/context/tree?provider=claude&id=deadbeef`) + expect(res.status).toBe(404) + }) +}) diff --git a/tests/dashboard.test.ts b/tests/dashboard.test.ts index 59ada8fd..d9af0848 100644 --- a/tests/dashboard.test.ts +++ b/tests/dashboard.test.ts @@ -2,7 +2,7 @@ import { homedir } from 'os' import { describe, it, expect } from 'vitest' -import { getDailyActivityRows, getDashboardScanRange, pageHistoryCursor, scrollHistoryCursor, selectDashboardPeriodProjects, shortProject, showEmptyState } from '../src/dashboard.js' +import { dailyActivityFooter, getDailyActivityRows, getDashboardScanRange, pageHistoryCursor, scrollHistoryCursor, selectDashboardPeriodProjects, shortProject, showEmptyState } from '../src/dashboard.js' import { getDateRange } from '../src/cli-date.js' import { formatCost } from '../src/format.js' import type { ProjectSummary, SessionSummary } from '../src/types.js' @@ -248,3 +248,18 @@ describe('showEmptyState', () => { expect(showEmptyState(2, false, 0, false)).toBe(false) }) }) + +// Issue #767 item 3: the Daily Activity panel's "of N" is a count of days +// found by the bounded live scan, not the same population the Overview +// headline (durable cache) counts for the period. The two numbers are each +// correct for what they measure, but nothing on screen said so - label the +// denominator instead of changing it. +describe('dailyActivityFooter', () => { + it('labels the count as the scanned-days population, not a bare total', () => { + expect(dailyActivityFooter(0, 14, 37)).toBe('Showing 1–14 of 37 days scanned · newest first') + }) + + it('clamps the visible end to the row count', () => { + expect(dailyActivityFooter(30, 14, 37)).toBe('Showing 31–37 of 37 days scanned · newest first') + }) +}) diff --git a/tests/format-carried-cost-note.test.ts b/tests/format-carried-cost-note.test.ts new file mode 100644 index 00000000..5f20cadd --- /dev/null +++ b/tests/format-carried-cost-note.test.ts @@ -0,0 +1,20 @@ +import { describe, it, expect } from 'vitest' + +import { carriedCostNote } from '../src/format.js' + +// Issue #767 item 3: the dashboard TUI's Daily Activity panel counts active +// days from a bounded live scan while the Overview headline (durable cache) +// can include cost from days whose session files have since expired. The two +// numbers are each internally consistent but read as a contradiction with no +// explanation. overview.ts already has a footnote for exactly this case +// ("includes $X preserved from expired session logs"); this helper is the +// shared, testable piece of that same wording so dashboard.tsx can reuse it. +describe('carriedCostNote', () => { + it('is null when nothing was carried forward', () => { + expect(carriedCostNote(0)).toBeNull() + }) + + it('explains carried cost when present', () => { + expect(carriedCostNote(1.23)).toBe('includes $1.23 preserved from expired session logs') + }) +})