sessions: reconcile mixed PR rows, harden v5 adoption, bound models

Round-2 review follow-ups.

- Mixed legacy/live category breakdown: a PR row that combines an expired
  legacy contribution (even-split, no turn data) with a live per-turn
  contribution now emits an explicit "Legacy estimate (no per-turn detail)"
  category carrying the legacy share, so the expansion reconciles to the row
  cost instead of silently dropping it. A legacy-only row still shows no
  breakdown.
- v5 adoption is now per cached file: one malformed entry is skipped instead
  of rejecting the whole v5 cache and dropping every valid expired PR session.
- The desktop "Other (N more PRs)" line moved to the table footer as a muted,
  separated summary rather than a sorted row.
- Row expansion resets when the PR set changes (period/provider/data), so a
  stale expansion cannot linger on a row that is gone.
- Payload models per row are capped to the top 4 by attributed cost, with a
  name-ascending tie-break; categories get the same stable tie-break.

Tests: mixed live+legacy reconciliation, legacy-only omission, model cap and
tie-break, corrupt-plus-valid v5 adoption, and the desktop expansion-reset.

app/renderer/sections/PullRequests.test.tsx

@@ -1,5 +1,5 @@
 // @vitest-environment jsdom
-import { render, screen } from '@testing-library/react'
+import { render, screen, waitFor } from '@testing-library/react'
 import userEvent from '@testing-library/user-event'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 
@@ -125,6 +125,23 @@ describe('PullRequests', () => {
     expect(screen.queryByText('Feature work')).toBeNull()
   })
 
+  it('closes an open expansion when the period changes the PR set', async () => {
+    const changed: PrPayload = { ...SAMPLE, rows: [SAMPLE.rows[0]!] } // #781 dropped
+    getOverview.mockImplementation((period: string) => Promise.resolve(makePayload(period === 'lifetime' ? SAMPLE : changed)))
+    const { rerender } = render(<PullRequests period="lifetime" provider="all" />)
+
+    const link = await screen.findByRole('link', { name: 'getagentseal/codeburn#780' })
+    await userEvent.click(rowForLink(link))
+    expect(rowForLink(link)).toHaveAttribute('aria-expanded', 'true')
+
+    rerender(<PullRequests period="week" provider="all" />)
+    // The new period drops #781, so the PR set changes and the stale expansion
+    // resets once the new data lands (wait for the breakdown to disappear).
+    await waitFor(() => expect(screen.queryByText('Feature work')).toBeNull())
+    const link2 = await screen.findByRole('link', { name: 'getagentseal/codeburn#780' })
+    expect(rowForLink(link2)).toHaveAttribute('aria-expanded', 'false')
+  })
+
   it('toggles expansion from the keyboard with Enter', async () => {
     getOverview.mockResolvedValue(makePayload(SAMPLE))
     render(<PullRequests period="lifetime" provider="all" />)

app/renderer/sections/PullRequests.tsx

@@ -1,5 +1,5 @@
 import type { KeyboardEvent, MouseEvent } from 'react'
-import { useState } from 'react'
+import { useEffect, useState } from 'react'
 
 import { CliErrorPanel } from '../components/CliErrorPanel'
 import { EmptyNote } from '../components/EmptyState'
@@ -83,6 +83,11 @@ function PullRequestsPage({ pullRequests, staleError }: { pullRequests?: PullReq
 function PrTable({ pullRequests }: { pullRequests: PullRequests }) {
   const { rows, distinctCost, distinctSessions, attributedCost, unattributedCost, otherPrCount, otherPrCost } = pullRequests
   const [expandedUrl, setExpandedUrl] = useState<string | null>(null)
+  // Reset any open expansion when the PR set changes (a period/provider switch or
+  // a refresh that alters the list): a stale expandedUrl would otherwise linger
+  // pointing at a row that is no longer present.
+  const rowKey = rows.map(row => row.url).join('|')
+  useEffect(() => { setExpandedUrl(null) }, [rowKey])
 
   // A new-attribution payload carries `attributedCost`; an older by-reference
   // payload omits it, so the rows are not summable and the footer must differ.
@@ -118,7 +123,11 @@ function PrTable({ pullRequests }: { pullRequests: PullRequests }) {
                 onToggle={() => setExpandedUrl(current => current === pr.url ? null : pr.url)}
               />
             ))}
-            {otherCount > 0 && (
+          </tbody>
+          {otherCount > 0 && (
+            // A muted summary line, kept out of the sorted rows: its cost is an
+            // aggregate of the capped-away PRs and can exceed a visible row.
+            <tfoot>
               <tr className="pr-other-row">
                 <td className="pr-other-label">Other ({otherCount.toLocaleString('en-US')} more PRs)</td>
                 <td className="pr-models"></td>
@@ -128,8 +137,8 @@ function PrTable({ pullRequests }: { pullRequests: PullRequests }) {
                 <td className="num pr-span"></td>
                 <td className="pr-chevron-cell"></td>
               </tr>
-            )}
-          </tbody>
+            </tfoot>
+          )}
         </table>
       </div>
       {summable ? (

app/renderer/styles/plain.css

@@ -601,7 +601,8 @@ td:first-child { font-size: var(--fs-body); font-weight: var(--fw-body); }
 .pr-cat-name { overflow: hidden; color: var(--ink); font-weight: 560; text-overflow: ellipsis; white-space: nowrap; }
 .pr-cat-main strong { flex: 0 0 auto; font-family: var(--mono); font-size: 11.5px; font-variant-numeric: tabular-nums; }
 .pr-cat-empty { margin: 0; color: var(--mut2); font-size: var(--fs-meta); }
-.pr-table td.pr-other-label { color: var(--mut2); font-weight: var(--fw-medium); text-align: left; }
+.pr-table tfoot .pr-other-row td { border-top: 1px solid var(--line); color: var(--mut2); font-style: italic; }
+.pr-table td.pr-other-label { color: var(--mut2); font-weight: var(--fw-medium); font-style: italic; text-align: left; }
 .pr-footnote { margin: 12px 2px 2px; color: var(--mut); font-size: var(--fs-meta); line-height: 1.55; }
 .pr-unattributed { margin: 4px 2px 2px; color: var(--mut2); font-size: var(--fs-meta); font-variant-numeric: tabular-nums; }
 .opt-waste { min-width: 0; }

src/session-cache.ts

@@ -369,15 +369,14 @@ function validateCache(raw: unknown): raw is SessionCache {
 // file itself is never written or deleted (old binaries still own it).
 const V5_CACHE_FILE = 'session-cache.v5.json'
 
-// v5-shaped validation: identical to validateCache but pinned to version 5. The
-// per-turn schema is a superset (prRefs is optional), so a v5 file passes the
-// section/file/turn validators unchanged.
-function validateV5Cache(raw: unknown): raw is SessionCache {
+// Lightweight top-level check: a version-5 cache with a providers object. The
+// individual files are validated per-entry in adoptV5Cache so one corrupt entry
+// cannot drop every valid expired-transcript PR session along with it.
+function isV5CacheEnvelope(raw: unknown): raw is { version: number; providers: Record<string, unknown> } {
   if (!raw || typeof raw !== 'object') return false
   const o = raw as Record<string, unknown>
-  if (o['version'] !== 5) return false
-  if (!o['providers'] || typeof o['providers'] !== 'object' || Array.isArray(o['providers'])) return false
-  return Object.values(o['providers'] as Record<string, unknown>).every(validateProviderSection)
+  return o['version'] === 5
+    && !!o['providers'] && typeof o['providers'] === 'object' && !Array.isArray(o['providers'])
 }
 
 // One-time migration for the 5 -> 6 bump (per-turn prRefs capture). A fresh v6
@@ -386,24 +385,31 @@ function validateV5Cache(raw: unknown): raw is SessionCache {
 // Carry forward exactly the v5 entries whose source no longer exists AND that
 // carry prLinks (they can never re-parse, but they hold attributable PR spend);
 // present sources are intentionally dropped so they re-parse fresh under v6 and
-// gain per-turn refs. Each carried section takes the CURRENT envFingerprint so
-// the scan reuses it and appends the freshly-parsed present sources. The daily
-// cache, which owns durable cost history, is not touched.
+// gain per-turn refs. Each file is validated individually, so a single corrupt
+// entry is skipped rather than discarding the whole cache. Each carried section
+// takes the CURRENT envFingerprint so the scan reuses it and appends the
+// freshly-parsed present sources. The daily cache (durable cost history) is not
+// touched.
 async function adoptV5Cache(): Promise<SessionCache | null> {
   try {
     const raw = await readFile(join(getCacheDir(), V5_CACHE_FILE), 'utf-8')
     const parsed = JSON.parse(raw)
-    if (!validateV5Cache(parsed)) return null
+    if (!isV5CacheEnvelope(parsed)) return null
     const migrated: SessionCache = { version: CACHE_VERSION, providers: {}, complete: false }
     for (const [provider, section] of Object.entries(parsed.providers)) {
+      if (!section || typeof section !== 'object') continue
+      const rawFiles = (section as Record<string, unknown>)['files']
       const files: Record<string, CachedFile> = {}
-      for (const [path, file] of Object.entries(section.files)) {
-        if (!existsSync(path) && file.prLinks?.length) files[path] = file
+      if (rawFiles && typeof rawFiles === 'object' && !Array.isArray(rawFiles)) {
+        for (const [path, file] of Object.entries(rawFiles as Record<string, unknown>)) {
+          if (!validateCachedFile(file)) continue
+          if (!existsSync(path) && file.prLinks?.length) files[path] = file
+        }
       }
       migrated.providers[provider] = {
         envFingerprint: computeEnvFingerprint(provider),
         files,
-        ...(section.durable ? { durable: true } : {}),
+        ...((section as Record<string, unknown>)['durable'] ? { durable: true } : {}),
       }
     }
     return migrated

src/sessions-report.ts

@@ -255,6 +255,7 @@ export function attributeSessionPrSpend(session: AttributableSession): SessionPr
 export function aggregateByPr(projects: ProjectSummary[]): PrRow[] {
   const byUrl = new Map<string, {
     cost: number; savingsUSD: number; calls: number; approx: boolean
+    legacyCost: number
     sessions: Set<string>; firstStarted: string; lastEnded: string
     models: Map<string, number>; categories: Map<string, number>
   }>()
@@ -268,7 +269,7 @@ export function aggregateByPr(projects: ProjectSummary[]): PrRow[] {
       for (const [url, c] of perUrl) {
         if (c.cost === 0 && c.calls === 0 && c.savingsUSD === 0) continue
         const row = byUrl.get(url) ?? {
-          cost: 0, savingsUSD: 0, calls: 0, approx: false,
+          cost: 0, savingsUSD: 0, calls: 0, approx: false, legacyCost: 0,
           sessions: new Set<string>(), firstStarted: session.firstTimestamp, lastEnded: session.lastTimestamp,
           models: new Map<string, number>(), categories: new Map<string, number>(),
         }
@@ -276,7 +277,9 @@ export function aggregateByPr(projects: ProjectSummary[]): PrRow[] {
         row.savingsUSD += c.savingsUSD
         row.calls += c.calls
         row.sessions.add(sessionKey)
-        if (c.approx) row.approx = true
+        // A legacy (approx) contribution carries no per-turn categories; track its
+        // cost so a mixed row can reconcile its category breakdown to the total.
+        if (c.approx) { row.approx = true; row.legacyCost += c.cost }
         for (const [m, mc] of c.models) addToMap(row.models, m, mc)
         for (const [cat, cc] of c.categories) addToMap(row.categories, cat, cc)
         if (session.firstTimestamp < row.firstStarted) row.firstStarted = session.firstTimestamp
@@ -288,13 +291,25 @@ export function aggregateByPr(projects: ProjectSummary[]): PrRow[] {
   return [...byUrl.entries()]
     .map(([url, r]) => {
       // Collapse raw model names to short display names, summing costs that map
-      // to the same short name, then order by attributed cost.
+      // to the same short name, then order by attributed cost (name asc breaks
+      // ties for a stable order) and cap at the top 4 to bound the payload.
       const shortCosts = new Map<string, number>()
       for (const [raw, mc] of r.models) addToMap(shortCosts, getShortModelName(raw), mc)
-      const models = [...shortCosts.entries()].sort((a, b) => b[1] - a[1]).map(([name]) => name)
+      const models = [...shortCosts.entries()]
+        .sort((a, b) => b[1] - a[1] || a[0].localeCompare(b[0]))
+        .slice(0, 4)
+        .map(([name]) => name)
       const categories = [...r.categories.entries()]
-        .sort((a, b) => b[1] - a[1])
         .map(([cat, cost]) => ({ name: CATEGORY_LABELS[cat as TaskCategory] ?? cat, cost }))
+      // Mixed row: live per-turn categories exist AND part of the row came from a
+      // legacy even-split (no turn data). Add a synthetic line for the legacy
+      // share so the expansion reconciles with the row cost instead of silently
+      // dropping it. A legacy-only row keeps no categories (it surfaces as "no
+      // per-turn detail"), so there is nothing to reconcile there.
+      if (categories.length > 0 && r.legacyCost > 0) {
+        categories.push({ name: 'Legacy estimate (no per-turn detail)', cost: r.legacyCost })
+      }
+      categories.sort((a, b) => b.cost - a.cost || a.name.localeCompare(b.name))
       return {
         url, label: shortenPrUrl(url),
         cost: r.cost, savingsUSD: r.savingsUSD,

tests/session-cache-v5-adoption.test.ts

@@ -93,4 +93,44 @@ describe('v5 -> v6 cache adoption of expired PR sessions', () => {
     // Model union is still surfaced on legacy rows.
     expect(rows[0]!.models.length).toBeGreaterThan(0)
   })
+
+  it('skips a corrupt v5 entry but still adopts a valid expired PR entry', async () => {
+    await loadPricing()
+    const goodPath = join(configDir, 'projects', 'good-proj', 'good.jsonl')
+    const badPath = join(configDir, 'projects', 'bad-proj', 'bad.jsonl')
+    const v5 = {
+      version: 5,
+      complete: true,
+      providers: {
+        claude: {
+          envFingerprint: 'stale-v5',
+          files: {
+            // Corrupt: malformed turns fail validateCachedFile and are skipped.
+            [badPath]: {
+              fingerprint: { dev: 9, ino: 9, mtimeMs: 9, sizeBytes: 9 },
+              mcpInventory: [],
+              prLinks: ['https://github.com/o/r/pull/9'],
+              turns: [{ garbage: true }],
+            },
+            // Valid expired PR entry that must survive alongside the corrupt one.
+            [goodPath]: {
+              fingerprint: { dev: 1, ino: 2, mtimeMs: 3, sizeBytes: 4 },
+              mcpInventory: [],
+              canonicalCwd: '/good/proj',
+              canonicalProjectName: 'good-proj',
+              prLinks: ['https://github.com/o/r/pull/1'],
+              turns: [{ timestamp: '2026-07-20T10:00:00.000Z', sessionId: 'good', userMessage: 'work', calls: [cachedCall('gk1', 40)] }],
+            },
+          },
+        },
+      },
+    }
+    await writeFile(join(cacheDir, 'session-cache.v5.json'), JSON.stringify(v5))
+
+    const range = { start: new Date('2026-07-20T00:00:00Z'), end: new Date('2026-07-20T23:59:59Z') }
+    const rows = aggregateByPr(await parseAllSessions(range, 'claude'))
+    const urls = rows.map(r => r.url)
+    expect(urls).toContain('https://github.com/o/r/pull/1') // valid entry survives
+    expect(urls).not.toContain('https://github.com/o/r/pull/9') // corrupt entry skipped, not fatal
+  })
 })

tests/sessions-by-pr.test.ts

@@ -55,6 +55,15 @@ function sessionWithTurns(id: string, prLinks: string[], turns: ClassifiedTurn[]
   return { ...session(id, 0, 0, prLinks, first, last), turns }
 }
 
+// A turn referencing `prRefs` whose calls each carry a [model, cost] pair.
+function turnModels(prRefs: string[], calls: Array<[string, number]>): ClassifiedTurn {
+  return {
+    userMessage: '', timestamp: '2026-07-01T10:00:00Z', sessionId: 's',
+    category: 'coding', retries: 0, hasEdits: false, prRefs,
+    assistantCalls: calls.map(([model, cost]) => ({ ...call(cost), model })),
+  }
+}
+
 function project(sessions: SessionSummary[]): ProjectSummary {
   return { project: 'p', projectPath: '/p', sessions, totalCostUSD: 0, totalSavingsUSD: 0, totalApiCalls: 0, totalProxiedCostUSD: 0 }
 }
@@ -304,6 +313,43 @@ describe('models + categories attribution', () => {
   })
 })
 
+describe('mixed live + legacy category reconciliation (round-3 finding 1)', () => {
+  it('adds a synthetic legacy line so a mixed row reconciles to its cost', () => {
+    const rows = aggregateByPr([project([
+      sessionWithTurns('live', [A], [cturn(10, 1, [A])]), // live: Coding $10
+      session('legacy', 90, 5, [A]),                       // legacy: $90, no turn data
+    ])])
+    const row = rows.find(r => r.url === A)!
+    expect(row.cost).toBeCloseTo(100, 6)
+    expect(row.approx).toBe(true)
+    const cats = row.categories!
+    expect(cats.reduce((s, c) => s + c.cost, 0)).toBeCloseTo(row.cost, 6) // reconciles
+    expect(cats.find(c => c.name === 'Coding')!.cost).toBeCloseTo(10, 6)
+    expect(cats.find(c => c.name === 'Legacy estimate (no per-turn detail)')!.cost).toBeCloseTo(90, 6)
+  })
+
+  it('a legacy-only row still omits categories (surfaces the no-detail note)', () => {
+    const rows = aggregateByPr([project([session('legacy', 90, 5, [A])])])
+    expect(rows[0]!.categories).toBeUndefined()
+  })
+})
+
+describe('model list bounds (round-3 finding 5)', () => {
+  it('caps a row to the top 4 models by attributed cost', () => {
+    const rows = aggregateByPr([project([
+      sessionWithTurns('s', [A], [turnModels([A], [['m-f', 60], ['m-e', 50], ['m-d', 40], ['m-c', 30], ['m-b', 20], ['m-a', 10]])]),
+    ])])
+    expect(rows[0]!.models).toEqual(['m-f', 'm-e', 'm-d', 'm-c'])
+  })
+
+  it('breaks model ties by name ascending for a stable order', () => {
+    const rows = aggregateByPr([project([
+      sessionWithTurns('s', [A], [turnModels([A], [['m-d', 10], ['m-a', 10], ['m-c', 10], ['m-b', 10]])]),
+    ])])
+    expect(rows[0]!.models).toEqual(['m-a', 'm-b', 'm-c', 'm-d'])
+  })
+})
+
 describe('distinct-session keying (finding 7)', () => {
   it('counts two same-sessionId sessions in different projects as two', () => {
     const s1 = sessionWithTurns('same-id', [A], [cturn(10, 1, [A])])