From 139727b8c7bf23bef0bf02ce58e0ab4d1559ee09 Mon Sep 17 00:00:00 2001 From: reviewer Date: Tue, 21 Jul 2026 03:18:39 +0200 Subject: [PATCH] sessions: round-2 hardening of subagent PR attribution Address a second adversarial review of the new machinery. - ID collision: parents and a child's parent reference are keyed by provider + sessionId, not bare sessionId. When two distinct parents still share a key (true duplicate/imported data), the child folds into NEITHER (deterministic skip, stays standalone): correctness over coverage. - Recursion dedup is global: one claimed-set spans all of a parent's direct children, so a descendant reachable through two paths (a diamond or duplicate id) folds exactly once and a parent-link cycle terminates. - Cache adoption migrates every prior version oldest-to-newest and MERGES per source path (newer wins per entry), so a sparse or partial newer file no longer masks older-only expired-PR orphans. - Fold anchors (0-cost PR-linked parents kept only for attribution) live in a new ProjectSummary.subagentAnchors, never in `sessions`, so they no longer contaminate session counts, averages, or any per-session report. Folded PR rows take their date span from the contributing child activity rather than the anchor's empty timestamps. - One-pass buildPrAttribution computes rows and totals together; the payload builder and CLI call it once. Drops the identity-keyed memoization, which could return stale folds if the array was mutated. - Ambiguous multi-block spawn-result pairing leaves the spawn link unset on purpose; the child then folds via the timestamp fallback rather than pairing with the wrong id or disappearing. Every fix is mutation-verified. A fresh real-data drive re-proves the no-double-count identity to the cent (parent-only cost plus folded cost equals the folded total) and that the PR rows sum to attributedCost. --- src/main.ts | 8 +- src/parser.ts | 37 ++-- src/session-cache.ts | 23 +- src/sessions-report.ts | 268 ++++++++++++++---------- src/types.ts | 7 + src/usage-aggregator.ts | 6 +- tests/parser-rich-capture.test.ts | 16 ++ tests/parser-subagent-range.test.ts | 6 + tests/session-cache-v5-adoption.test.ts | 26 ++- tests/sessions-subagent-fold.test.ts | 99 ++++++--- 10 files changed, 333 insertions(+), 163 deletions(-) diff --git a/src/main.ts b/src/main.ts index c6d9af6..bebcce2 100644 --- a/src/main.ts +++ b/src/main.ts @@ -2011,7 +2011,7 @@ program .action(async (opts) => { assertProvider(opts.provider, 'sessions') assertFormat(opts.format, ['table', 'json'], 'sessions') - const { aggregateSessions, aggregateByPr, prLinkedTotals, renderJson, renderTable } = await import('./sessions-report.js') + const { aggregateSessions, buildPrAttribution, renderJson, renderTable } = await import('./sessions-report.js') await loadPricing() let range @@ -2028,16 +2028,16 @@ program const projects = await parseAllSessions(range, opts.provider) if (opts.byPr) { - const prRows = aggregateByPr(projects) + const { rows: prRows, totals } = buildPrAttribution(projects) if (opts.format === 'json') { - process.stdout.write(JSON.stringify({ prs: prRows, distinct: prLinkedTotals(projects) }, null, 2) + '\n') + process.stdout.write(JSON.stringify({ prs: prRows, distinct: totals }, null, 2) + '\n') return } if (prRows.length === 0) { process.stdout.write('No sessions with captured PR links in this period. Links are captured as sessions are parsed; older transcripts gain them on their next re-parse.\n') return } - const { unattributedCost, sessions, subagentSessions } = prLinkedTotals(projects) + const { unattributedCost, sessions, subagentSessions } = totals const { renderTable: renderTextTable } = await import('./text-table.js') const modelsCell = (models: string[]): string => models.length === 0 ? '' : models.slice(0, 2).join(', ') + (models.length > 2 ? ` +${models.length - 2}` : '') diff --git a/src/parser.ts b/src/parser.ts index 028a076..20e09de 100644 --- a/src/parser.ts +++ b/src/parser.ts @@ -1284,7 +1284,10 @@ export function collectSessionMeta(entry: JournalEntry, meta: SessionMeta): void } else if (results.length > 1) { // Several batched tool results share one entry: pair the agentId with the // block whose `content` is the spawn result (equals `toolUseResult.content`), - // so an unrelated sibling block cannot capture the id. Skip if ambiguous. + // so an unrelated sibling block cannot capture the id. When the match is + // ambiguous (identical blocks, or none match) the spawn link is left + // unset ON PURPOSE: the child then folds via the timestamp-bucket fallback + // in resolveChild rather than risk pairing with the wrong id. const turContent = JSON.stringify((tur as Record)['content']) const matches = results.filter(b => JSON.stringify(b['content']) === turContent) if (matches.length === 1) spawnId = matches[0]!['tool_use_id'] as string @@ -2130,7 +2133,7 @@ async function scanProjectDirs( } } - const projectMap = new Map }>() + const projectMap = new Map }>() const allFiles = [ ...unchangedFiles.map(f => ({ filePath: f.filePath, dirName: f.dirName, source: f.source })), @@ -2187,10 +2190,13 @@ async function scanProjectDirs( } // A PR-linked parent that spawned subagents is kept even when its OWN turns all - // fall out of range, as a 0-cost fold anchor: an in-range child (an async agent + // fall out of range, as a 0-cost fold ANCHOR: an in-range child (an async agent // that outlived the parent's last in-range turn) still needs the parent's - // `prLinks` / `spawnPrSets` to attribute. It contributes no spend of its own. + // `prLinks` / `spawnPrSets` to attribute. An anchor carries no in-range spend + // and is stored OUTSIDE `sessions` (see subagentAnchors) so it never + // contaminates session counts, averages, or any other per-session report. const isSpawnAnchor = Object.keys(spawnPrSets).length > 0 && cachedFile.isSidechain !== true + const anchorOnly = classifiedTurns.length === 0 && isSpawnAnchor if (classifiedTurns.length === 0 && !isSpawnAnchor) continue const sessionId = basename(filePath, '.jsonl') @@ -2217,17 +2223,17 @@ async function scanProjectDirs( } if (Object.keys(spawnPrSets).length > 0) session.spawnPrSets = spawnPrSets - if (session.apiCalls > 0 || isSpawnAnchor) { + if (session.apiCalls > 0 || anchorOnly) { const projectKey = cachedFile.canonicalCwd ? normalizeProjectPathKey(cachedFile.canonicalCwd) : `slug:${dirName}` const existing = projectMap.get(projectKey) - if (existing) { - existing.sessions.push(session) - existing.dirNames.add(dirName) - } else { - projectMap.set(projectKey, { project: projectName, projectPath, sessions: [session], dirNames: new Set([dirName]) }) - } + // An anchor (no in-range spend) goes into a separate bucket, never `sessions`. + const target = existing ?? { project: projectName, projectPath, sessions: [], anchors: [], dirNames: new Set([dirName]) } + if (anchorOnly) target.anchors.push(session) + else target.sessions.push(session) + target.dirNames.add(dirName) + if (!existing) projectMap.set(projectKey, target) } } @@ -2245,12 +2251,13 @@ async function scanProjectDirs( if (!cwdKey) continue const target = projectMap.get(cwdKey)! target.sessions.push(...entry.sessions) + target.anchors.push(...entry.anchors) projectMap.delete(key) } const projects: ProjectSummary[] = [] - for (const { project, projectPath, sessions } of projectMap.values()) { - projects.push(summarizeProject(project, projectPath, sessions)) + for (const { project, projectPath, sessions, anchors } of projectMap.values()) { + projects.push(summarizeProject(project, projectPath, sessions, anchors)) } return projects @@ -2263,7 +2270,7 @@ async function scanProjectDirs( /// `totalProxiedCostUSD` (subscription-covered). All ProjectSummary callers go /// through here so the rule stays consistent across the fresh, cached, and /// date/day-filtered paths. -function summarizeProject(project: string, projectPath: string, sessions: SessionSummary[]): ProjectSummary { +function summarizeProject(project: string, projectPath: string, sessions: SessionSummary[], anchors: SessionSummary[] = []): ProjectSummary { const totalCostUSD = sessions.reduce((s, sess) => s + sess.totalCostUSD, 0) return { project, @@ -2274,6 +2281,8 @@ function summarizeProject(project: string, projectPath: string, sessions: Sessio totalEstimatedCostUSD: sessions.reduce((s, sess) => s + (sess.totalEstimatedCostUSD ?? 0), 0), totalApiCalls: sessions.reduce((s, sess) => s + sess.apiCalls, 0), totalProxiedCostUSD: isProxiedPath(projectPath) ? totalCostUSD : 0, + // Fold anchors travel separately (0-cost, out of every per-session total). + ...(anchors.length > 0 ? { subagentAnchors: anchors } : {}), } } diff --git a/src/session-cache.ts b/src/session-cache.ts index 34f1ea9..ee9aab1 100644 --- a/src/session-cache.ts +++ b/src/session-cache.ts @@ -454,13 +454,28 @@ async function adoptPriorCache(version: number): Promise { } } -// Adopt the newest prior versioned cache present on disk (v6 before v5). +// Adopt EVERY prior versioned cache present on disk, migrating OLDEST first and +// merging per source path so a newer version wins per entry. Returning the newest +// alone would be wrong: a sparse or partial newer file (e.g. v6 holding only some +// orphans) would mask older-only orphans that still hold attributable spend. Newer +// entries overwrite older ones for the same path; entries unique to an older +// version survive. async function adoptNewestPriorCache(): Promise { - for (const version of PRIOR_CACHE_VERSIONS) { + const oldestFirst = [...PRIOR_CACHE_VERSIONS].sort((a, b) => a - b) + let merged: SessionCache | null = null + for (const version of oldestFirst) { const adopted = await adoptPriorCache(version) - if (adopted) return adopted + if (!adopted) continue + if (!merged) { merged = adopted; continue } + for (const [provider, section] of Object.entries(adopted.providers)) { + const existing = merged.providers[provider] + if (!existing) { merged.providers[provider] = section; continue } + // Newer version's entries overwrite older ones for the same source path. + Object.assign(existing.files, section.files) + if (section.durable) existing.durable = true + } } - return null + return merged } export async function loadCache(): Promise { diff --git a/src/sessions-report.ts b/src/sessions-report.ts index fece1d3..d9bac37 100644 --- a/src/sessions-report.ts +++ b/src/sessions-report.ts @@ -179,13 +179,17 @@ export function allocateEven(total: number, n: number): number[] { /// three sum to `cost` (derived from the same sessions), so folding never adds a /// rounding gap. `foldedSessions` counts the subtree (self plus folded /// descendants); `spawnAtMs` is the TOP child's first-activity epoch (the whole -/// subtree resolves against the top parent through the top child's spawn). +/// subtree resolves against the top parent through the top child's spawn); +/// `firstTs`/`lastTs` are the subtree's real activity span, used for the PR row's +/// date range when the parent has no in-range turns of its own. export type ChildFold = { agentId: string cost: number calls: number savingsUSD: number spawnAtMs: number + firstTs: string + lastTs: string models: Map categories: Map foldedSessions: number @@ -203,31 +207,50 @@ function selfLinks(session: { prLinks?: string[] }): boolean { return !!session.prLinks?.length } +// A session id alone is NOT globally unique: imported or duplicated transcripts, +// or two providers, can reuse one id, letting two parents claim one child and +// corrupting the attribution map. Key parents (and a child's parent reference) by +// provider + id so linkage stays one-to-one; the ambiguity skip in +// resolveSubagentAttribution handles a genuine same-provider id collision. +// Subagent linkage (parentSessionId / agentSpawnLinks / spawnPrSets) is Claude +// only, and a fold ANCHOR has no in-range turns to infer a provider from, so a +// linkage-bearing session is always attributed to 'claude'; this keeps a parent +// and its child on the same key. +function linkageProvider(session: SessionSummary): string { + if (session.parentSessionId || session.agentSpawnLinks || session.spawnPrSets) return 'claude' + return inferProvider(session) +} +function providerSessionKey(session: SessionSummary): string { + return `${linkageProvider(session)}${session.sessionId}` +} +function providerParentKey(session: SessionSummary): string { + return `${linkageProvider(session)}${session.parentSessionId ?? ''}` +} + /// Index every sidechain (subagent) session by the parent that spawned it, keyed -/// by `parentSessionId` ALONE. Parent ids are UUIDs and subagent ids are -/// `agent-`, both globally unique, so a composite project key is unnecessary -/// and would drop a child whose worktree cwd resolves to a different -/// ProjectSummary than its parent. (The base PR's project+sessionId composite key -/// guards distinct-session COUNTING, a separate concern.) A child whose parent is -/// absent from the scan is never looked up, so it stays a standalone orphan. +/// by provider + `parentSessionId`. A child whose parent is absent from the scan +/// is never looked up, so it stays a standalone orphan. export function buildSubagentIndex(projects: ProjectSummary[]): Map { const index = new Map() for (const project of projects) for (const session of project.sessions) { if (!session.parentSessionId) continue - const list = index.get(session.parentSessionId) + const key = providerParentKey(session) + const list = index.get(key) if (list) list.push(session) - else index.set(session.parentSessionId, [session]) + else index.set(key, [session]) } return index } -// Aggregate a child session and its non-self-linking descendants, depth-first, -// guarded by `visited` against cycles (and against a session being pulled twice). -// A self-linking descendant is skipped: it attributes standalone. `spawnAtMs` -// stays the TOP child's, since the whole subtree resolves against the top parent. -function buildChildFold(child: SessionSummary, index: Map, visited: Set): ChildFold { - visited.add(child.sessionId) +// Aggregate a child session and its non-self-linking descendants, depth-first. +// `claimed` is ONE set per parent resolution (shared across all direct children), +// so a descendant reachable through two paths (duplicate/diamond ids) folds +// exactly once and a parent-link cycle terminates. A self-linking descendant is +// skipped: it attributes standalone. `spawnAtMs` stays the TOP child's, since the +// whole subtree resolves against the top parent. +function buildChildFold(child: SessionSummary, index: Map, claimed: Set): ChildFold { + claimed.add(child.sessionId) const models = new Map() const categories = new Map() for (const turn of child.turns) { @@ -241,15 +264,19 @@ function buildChildFold(child: SessionSummary, index: Map fold.lastTs) fold.lastTs = gcf.lastTs } return fold } @@ -293,38 +320,47 @@ function resolveChild(parent: SessionSummary, fold: ChildFold): ResolvedChild { return { fold, prSet: current, unlinked: false } } -/// Resolve every folded child to its parent's PR set, ONCE. Both `aggregateByPr` -/// and `prLinkedTotals` consume this instead of each rebuilding the index and -/// re-resolving. Keyed by parent sessionId; only PR-bearing parents (the ones the -/// consumers iterate) are resolved. A self-linking direct child is skipped (it -/// attributes standalone); its own non-self-linking descendants fold into it when -/// it is itself iterated as a PR-bearing parent. +/// Resolve every folded child to its parent's PR set, once. Keyed by the parent's +/// provider + sessionId. Parents come from each project's `sessions` AND its +/// `subagentAnchors` (PR-linked parents kept only for folding). When two DISTINCT +/// parents share a key (true duplicate data), the child is folded into NEITHER +/// (deterministic skip, stays standalone): correctness over coverage. export type SubagentAttribution = Map -// Memoize by the `projects` array identity so `aggregateByPr` and `prLinkedTotals` -// (called with the same array in one report render) resolve the subagent tree -// once, not twice. Keyed weakly, so it is released with the array; a fresh report -// builds a new array and recomputes. Public signature is unchanged. -const attributionCache = new WeakMap() - export function resolveSubagentAttribution(projects: ProjectSummary[]): SubagentAttribution { - const memo = attributionCache.get(projects) - if (memo) return memo const index = buildSubagentIndex(projects) + // Count PR-bearing parents per key (real sessions AND anchors) to detect an + // ambiguous key claimed by more than one distinct parent. + const parentCount = new Map() + const bump = (s: SessionSummary): void => { + if (!s.prLinks?.length) return + const k = providerSessionKey(s) + parentCount.set(k, (parentCount.get(k) ?? 0) + 1) + } + for (const project of projects) { + for (const s of project.sessions) bump(s) + for (const a of project.subagentAnchors ?? []) bump(a) + } const out: SubagentAttribution = new Map() - for (const project of projects) - for (const parent of project.sessions) { - if (!parent.prLinks?.length) continue - const direct = index.get(parent.sessionId) - if (!direct?.length) continue - const resolved: ResolvedChild[] = [] - for (const child of direct) { - if (selfLinks(child)) continue - resolved.push(resolveChild(parent, buildChildFold(child, index, new Set()))) - } - if (resolved.length) out.set(parent.sessionId, resolved) + const resolveParent = (parent: SessionSummary): void => { + if (!parent.prLinks?.length) return + const k = providerSessionKey(parent) + if (out.has(k)) return // already resolved for this key + if ((parentCount.get(k) ?? 0) > 1) { out.set(k, []); return } // ambiguous: fold nothing + const direct = index.get(k) + if (!direct?.length) return + const claimed = new Set() // one claimed set across all direct children + const resolved: ResolvedChild[] = [] + for (const child of direct) { + if (claimed.has(child.sessionId) || selfLinks(child)) continue + resolved.push(resolveChild(parent, buildChildFold(child, index, claimed))) } - attributionCache.set(projects, out) + if (resolved.length) out.set(k, resolved) + } + for (const project of projects) { + for (const parent of project.sessions) resolveParent(parent) + for (const anchor of project.subagentAnchors ?? []) resolveParent(anchor) + } return out } @@ -403,12 +439,23 @@ export function attributeSessionPrSpend(session: AttributableSession): SessionPr return { perUrl, unattributed } } -/// Spend attributed to each pull request at turn granularity (see -/// attributeSessionPrSpend). Rows carry ATTRIBUTED cost/calls and ARE summable; -/// `sessions` counts the distinct sessions that contributed any spend to the PR; -/// `approx` marks rows fed by the legacy even-split fallback; `models` and -/// `categories` are the attributed model/category breakdowns. Sorted by cost, desc. -export function aggregateByPr(projects: ProjectSummary[]): PrRow[] { +/// PR-attribution totals. `attributedCost` is the sum of the per-PR rows; +/// `unattributedCost` is pre-reference overhead not tied to any specific PR; +/// `cost` = attributed + unattributed = the PR-linked spend INCLUDING folded +/// subagent runs, so it exceeds the parents' own spend. `sessions` counts distinct +/// PR-linked PARENT sessions ONLY (0-cost fold anchors are excluded); it is +/// `subagentSessions` (folded subtrees: children plus descendants) that explains +/// the extra spend. +export type PrTotals = { cost: number; sessions: number; subagentSessions: number; attributedCost: number; unattributedCost: number } +export type PrAttribution = { rows: PrRow[]; totals: PrTotals } + +/// Spend by pull request, at turn granularity, with subagent runs folded into the +/// PR each was working on. Computed in ONE pass so `aggregateByPr` (rows) and +/// `prLinkedTotals` (totals) never disagree; the payload builder should call this +/// once and read both. Rows carry ATTRIBUTED cost/calls and ARE summable; `approx` +/// marks legacy even-split rows; `models`/`categories` are the attributed +/// breakdowns. Sorted by cost, descending. +export function buildPrAttribution(projects: ProjectSummary[]): PrAttribution { const byUrl = new Map; categories: Map }>() const attribution = resolveSubagentAttribution(projects) + let attributedCost = 0 + let unattributedCost = 0 + let sessions = 0 + let subagentSessions = 0 + // Add one contribution (a parent turn's share, or a folded child's share) to a - // PR row. `sessionKey` is the PARENT session identity, so a folded child does - // not inflate the row's distinct-session count beyond its parent. + // PR row. `sessionKey` is the contributing PARENT's identity, so a folded child + // does not inflate the row's distinct-session count beyond its parent. Empty + // timestamps (a 0-turn anchor) never widen the span; folded children pass their + // OWN activity span, which is what dates an anchor-only row. const addTo = ( url: string, sessionKey: string, firstTs: string, lastTs: string, cost: number, savings: number, calls: number, approx: boolean, @@ -437,41 +491,56 @@ export function aggregateByPr(projects: ProjectSummary[]): PrRow[] { if (approx) { row.approx = true; row.legacyCost += cost } for (const [m, mc] of models) addToMap(row.models, m, mc) for (const [cat, cc] of categories) addToMap(row.categories, cat, cc) - if (firstTs < row.firstStarted) row.firstStarted = firstTs - if (lastTs > row.lastEnded) row.lastEnded = lastTs + if (firstTs && (!row.firstStarted || firstTs < row.firstStarted)) row.firstStarted = firstTs + if (lastTs && lastTs > row.lastEnded) row.lastEnded = lastTs byUrl.set(url, row) } + + // Fold one parent's resolved children into rows + totals. An unlinked child + // contributes nothing; a child with no active PR goes to unattributed (no row). + const foldChildren = (parent: SessionSummary): void => { + const sessionKey = `${parent.project} ${parent.sessionId}` + for (const rc of attribution.get(providerSessionKey(parent)) ?? []) { + if (rc.unlinked) continue + subagentSessions += rc.fold.foldedSessions + if (!rc.prSet?.length) { unattributedCost += rc.fold.cost; continue } + attributedCost += rc.fold.cost + const prs = rc.prSet + const share = 1 / prs.length + const callAlloc = allocateEven(rc.fold.calls, prs.length) + prs.forEach((url, i) => { + const models = new Map() + for (const [m, mc] of rc.fold.models) models.set(m, mc * share) + const categories = new Map() + for (const [cat, cc] of rc.fold.categories) categories.set(cat, cc * share) + addTo(url, sessionKey, rc.fold.firstTs, rc.fold.lastTs, + rc.fold.cost * share, rc.fold.savingsUSD * share, callAlloc[i]!, false, models, categories) + }) + } + } + for (const project of projects) { for (const session of project.sessions) { if (!session.prLinks?.length) continue - // Key on project + sessionId: a transcript basename (sessionId) can repeat - // across projects, so sessionId alone would undercount distinct sessions. - const sessionKey = `${session.project}${session.sessionId}` - const { perUrl } = attributeSessionPrSpend(session) + sessions += 1 + const sessionKey = `${session.project} ${session.sessionId}` + const { perUrl, unattributed } = attributeSessionPrSpend(session) for (const [url, c] of perUrl) { + attributedCost += c.cost addTo(url, sessionKey, session.firstTimestamp, session.lastTimestamp, c.cost, c.savingsUSD, c.calls, c.approx, c.models, c.categories) } - // Fold this session's resolved subagent children into the PR each was - // working on. A child resolved to no PR (unattributed) or after the parent's - // last turn (unlinked) creates no row; it surfaces only in prLinkedTotals. - // Folded children carry no `approx` (they have genuine per-turn categories). - for (const rc of attribution.get(session.sessionId) ?? []) { - if (rc.unlinked || !rc.prSet?.length) continue - const prs = rc.prSet - const share = 1 / prs.length - const callAlloc = allocateEven(rc.fold.calls, prs.length) - prs.forEach((url, i) => { - const models = new Map() - for (const [m, mc] of rc.fold.models) models.set(m, mc * share) - const categories = new Map() - for (const [cat, cc] of rc.fold.categories) categories.set(cat, cc * share) - addTo(url, sessionKey, session.firstTimestamp, session.lastTimestamp, - rc.fold.cost * share, rc.fold.savingsUSD * share, callAlloc[i]!, false, models, categories) - }) - } + unattributedCost += unattributed.cost + foldChildren(session) + } + // Anchor parents: fold their children only. NOT counted in `sessions`, and no + // own spend (they have no in-range turns). + for (const anchor of project.subagentAnchors ?? []) { + if (!anchor.prLinks?.length) continue + foldChildren(anchor) } } - return [...byUrl.entries()] + + const rows = [...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 (name asc breaks @@ -504,41 +573,18 @@ export function aggregateByPr(projects: ProjectSummary[]): PrRow[] { } }) .sort((a, b) => b.cost - a.cost) + + return { rows, totals: { cost: attributedCost + unattributedCost, sessions, subagentSessions, attributedCost, unattributedCost } } } -/// Totals across every PR-linked session. `attributedCost` is the sum of the -/// per-PR rows (a summable total, unlike the old by-reference rows); -/// `unattributedCost` is the pre-reference overhead not tied to any specific PR. -/// `cost` = attributed + unattributed = the PR-linked spend, now INCLUDING the -/// subagent runs folded into those sessions, so it exceeds the parents' own spend. -/// `sessions` counts distinct PR-linked PARENT sessions; `subagentSessions` counts -/// the child runs folded into them (each still a standalone row in the sessions -/// list). Report both so the footer is honest about what the total covers. -export function prLinkedTotals(projects: ProjectSummary[]): { cost: number; sessions: number; subagentSessions: number; attributedCost: number; unattributedCost: number } { - let attributedCost = 0 - let unattributedCost = 0 - let sessions = 0 - let subagentSessions = 0 - const attribution = resolveSubagentAttribution(projects) - for (const project of projects) { - for (const session of project.sessions) { - if (!session.prLinks?.length) continue - sessions += 1 - const { perUrl, unattributed } = attributeSessionPrSpend(session) - for (const c of perUrl.values()) attributedCost += c.cost - unattributedCost += unattributed.cost - // Fold resolved children: an unlinked child contributes nothing; otherwise - // count its whole folded subtree and add its spend to attributed (resolved to - // a PR) or unattributed (no active PR at its spawn). - for (const rc of attribution.get(session.sessionId) ?? []) { - if (rc.unlinked) continue - subagentSessions += rc.fold.foldedSessions - if (rc.prSet?.length) attributedCost += rc.fold.cost - else unattributedCost += rc.fold.cost - } - } - } - return { cost: attributedCost + unattributedCost, sessions, subagentSessions, attributedCost, unattributedCost } +/// Spend attributed to each pull request (thin wrapper over buildPrAttribution). +export function aggregateByPr(projects: ProjectSummary[]): PrRow[] { + return buildPrAttribution(projects).rows +} + +/// Totals across every PR-linked session (thin wrapper over buildPrAttribution). +export function prLinkedTotals(projects: ProjectSummary[]): PrTotals { + return buildPrAttribution(projects).totals } export type BranchRow = { diff --git a/src/types.ts b/src/types.ts index 9b1a8b1..b856a67 100644 --- a/src/types.ts +++ b/src/types.ts @@ -285,6 +285,13 @@ export type ProjectSummary = { // net out-of-pocket for the project is `totalCostUSD - totalProxiedCostUSD`. // 0 when the project is not under a configured proxy path. totalProxiedCostUSD: number + /// Claude Code only: PR-linked parent sessions whose OWN turns all fell outside + /// the report range but which spawned an in-range subagent. Kept ONLY as fold + /// anchors for by-PR subagent attribution; they carry no in-range spend and are + /// deliberately NOT in `sessions`, so they never touch session counts, averages, + /// or any other per-session report. Consumed only by the by-PR resolver. Absent + /// when none. + subagentAnchors?: SessionSummary[] } export type DateRange = { diff --git a/src/usage-aggregator.ts b/src/usage-aggregator.ts index 9bff80f..4235bb2 100644 --- a/src/usage-aggregator.ts +++ b/src/usage-aggregator.ts @@ -10,7 +10,7 @@ import { aggregateProjectsIntoDays, buildPeriodDataFromDays } from './day-aggreg import { aggregateModelEfficiency } from './model-efficiency.js' import { aggregateModels } from './models-report.js' import { scanUserCorrections, medianTimeToFirstEditMs, aggregateFileChurn, computePricingCoverage } from './workflow-insights.js' -import { aggregateByPr, prLinkedTotals, aggregateByBranch } from './sessions-report.js' +import { buildPrAttribution, aggregateByBranch } from './sessions-report.js' import { scanAndDetect } from './optimize.js' import { getDaysInRange, ensureCacheHydrated, emptyCache, BACKFILL_DAYS, toDateString, type DailyCache, type DailyEntry } from './daily-cache.js' import { buildGranularHistory } from './granular-history.js' @@ -663,9 +663,9 @@ export async function buildMenubarPayloadForRange(periodInfo: PeriodInfo, opts: // Claude-config-scoped path (which replaces scanProjects with one config's // sessions) so this stays the genuine unscoped all-provider aggregation. if (isAllProviders && !effectivelyScoped) { - const prRows = aggregateByPr(scanProjects) + // One pass yields both rows and totals, so they never disagree. + const { rows: prRows, totals: prTotals } = buildPrAttribution(scanProjects) if (prRows.length > 0) { - const prTotals = prLinkedTotals(scanProjects) const shownRows = prRows.slice(0, TOP_PULL_REQUESTS) const otherRows = prRows.slice(TOP_PULL_REQUESTS) currentData.pullRequests = { diff --git a/tests/parser-rich-capture.test.ts b/tests/parser-rich-capture.test.ts index ab6777d..c72c9ef 100644 --- a/tests/parser-rich-capture.test.ts +++ b/tests/parser-rich-capture.test.ts @@ -296,6 +296,22 @@ describe('collectSessionMeta subagent linkage', () => { } as JournalEntry, meta) expect(meta.agentSpawnLinks).toEqual({ a999: 'toolu_spawn' }) }) + + it('omits the spawn link (deferring to the timestamp fallback) when blocks are ambiguous', () => { + const meta = emptySessionMeta() + collectSessionMeta({ + type: 'user', + // Two tool_result blocks with IDENTICAL content: the content match cannot + // pick one, so the link is omitted on purpose (resolveChild's timestamp + // fallback then folds the child rather than risking the wrong id). + message: { role: 'user', content: [ + { type: 'tool_result', tool_use_id: 'toolu_a', content: 'same' }, + { type: 'tool_result', tool_use_id: 'toolu_b', content: 'same' }, + ] }, + toolUseResult: { status: 'completed', agentId: 'a777', content: 'same' }, + } as JournalEntry, meta) + expect(meta.agentSpawnLinks).toEqual({}) // no guess + }) }) // ── per-turn subagent spawn ids (spawnToolUseIds) ────────────────────── diff --git a/tests/parser-subagent-range.test.ts b/tests/parser-subagent-range.test.ts index 940dbab..7f02efc 100644 --- a/tests/parser-subagent-range.test.ts +++ b/tests/parser-subagent-range.test.ts @@ -66,6 +66,12 @@ describe('subagent fold across a date-range boundary', () => { // The child session is present (its work is in range) as a standalone session. const childPresent = projects.some(p => p.sessions.some(s => s.sessionId === `agent-${AGENT}`)) expect(childPresent).toBe(true) + // The anchor parent (0 in-range turns) must NOT contaminate the sessions list; + // it lives in subagentAnchors only. + const anchorInSessions = projects.some(p => p.sessions.some(s => s.sessionId === PARENT)) + expect(anchorInSessions).toBe(false) + const anchorHeldSeparately = projects.some(p => (p.subagentAnchors ?? []).some(s => s.sessionId === PARENT)) + expect(anchorHeldSeparately).toBe(true) const rows = aggregateByPr(projects) const row = rows.find(r => r.url === PR) diff --git a/tests/session-cache-v5-adoption.test.ts b/tests/session-cache-v5-adoption.test.ts index dd10619..cdee52e 100644 --- a/tests/session-cache-v5-adoption.test.ts +++ b/tests/session-cache-v5-adoption.test.ts @@ -214,7 +214,7 @@ describe('newest-prior cache adoption (v6 then v5)', () => { expect(row!.cost).toBeCloseTo(40, 6) }) - it('prefers the newest prior file: v6 orphan is adopted, v5-only orphan is not', async () => { + it('merges prior versions: a v6-only orphan AND a v5-only orphan both survive', async () => { await loadPricing() const v6Path = join(configDir, 'projects', 'in6', 'in6.jsonl') const v5Path = join(configDir, 'projects', 'in5', 'in5.jsonl') @@ -229,7 +229,27 @@ describe('newest-prior cache adoption (v6 then v5)', () => { const range = { start: new Date('2026-07-20T00:00:00Z'), end: new Date('2026-07-20T23:59:59Z') } const urls = aggregateByPr(await parseAllSessions(range, 'claude')).map(r => r.url) - expect(urls).toContain('https://github.com/o/r/pull/60') // v6 (newest) adopted - expect(urls).not.toContain('https://github.com/o/r/pull/50') // v5 superseded by v6 + // A sparse newer file must NOT mask older-only orphans: BOTH survive. + expect(urls).toContain('https://github.com/o/r/pull/60') // from v6 + expect(urls).toContain('https://github.com/o/r/pull/50') // from v5, not masked + }) + + it('a newer version wins per source path when both hold the same path', async () => { + await loadPricing() + const samePath = join(configDir, 'projects', 'dup', 'dup.jsonl') + // v5 attributes this path to PR 500; v6 (newer) to PR 600 for the SAME path. + await writeFile(join(cacheDir, 'session-cache.v5.json'), JSON.stringify({ + version: 5, complete: true, + providers: { claude: { envFingerprint: 'v5', files: { [samePath]: expiredPrEntry('/dup', 'dup', 'https://github.com/o/r/pull/500') } } }, + })) + await writeFile(join(cacheDir, 'session-cache.v6.json'), JSON.stringify({ + version: 6, complete: true, + providers: { claude: { envFingerprint: 'v6', files: { [samePath]: expiredPrEntry('/dup', 'dup', 'https://github.com/o/r/pull/600') } } }, + })) + + const range = { start: new Date('2026-07-20T00:00:00Z'), end: new Date('2026-07-20T23:59:59Z') } + const urls = aggregateByPr(await parseAllSessions(range, 'claude')).map(r => r.url) + expect(urls).toContain('https://github.com/o/r/pull/600') // newer v6 entry wins + expect(urls).not.toContain('https://github.com/o/r/pull/500') // older v5 entry for same path superseded }) }) diff --git a/tests/sessions-subagent-fold.test.ts b/tests/sessions-subagent-fold.test.ts index f72efce..b3d8867 100644 --- a/tests/sessions-subagent-fold.test.ts +++ b/tests/sessions-subagent-fold.test.ts @@ -85,8 +85,8 @@ function child(opts: { } } -function project(sessions: SessionSummary[], name = 'p'): ProjectSummary { - return { project: name, projectPath: `/${name}`, sessions, totalCostUSD: 0, totalSavingsUSD: 0, totalApiCalls: 0, totalProxiedCostUSD: 0 } +function project(sessions: SessionSummary[], name = 'p', anchors?: SessionSummary[]): ProjectSummary { + return { project: name, projectPath: `/${name}`, sessions, totalCostUSD: 0, totalSavingsUSD: 0, totalApiCalls: 0, totalProxiedCostUSD: 0, ...(anchors ? { subagentAnchors: anchors } : {}) } } function rowFor(rows: ReturnType, url: string) { @@ -107,7 +107,9 @@ describe('buildSubagentIndex', () => { // Child lives in a DIFFERENT project than its parent (worktree cwd resolved elsewhere). project([child({ agentId: 'c1', parentId: 'P', cost: 50, firstTs: '2026-07-01T10:05:00Z', project: 'projB' })], 'projB'), ]) - expect(idx.get('P')?.map(s => s.agentId)).toEqual(['c1']) + // One key (parentSessionId, provider-prefixed), holding the single child. + expect(idx.size).toBe(1) + expect([...idx.values()].flat().map(s => s.agentId)).toEqual(['c1']) }) }) @@ -248,16 +250,22 @@ describe('MAJOR: date-range correctness', () => { expect(rowFor(rows, B)!.cost).toBeCloseTo(10, 6) // parent's in-range turn only }) - it('(a) an in-range child of a parent with no in-range turns still folds', () => { - // The parent has zero in-range turns (a 0-cost fold anchor) but carries prLinks - // and spawnPrSets; its in-range child must still reach the PR. + it('(a) an in-range child of an anchor parent (no in-range turns) folds, anchor uncounted', () => { + // The parent is a 0-cost fold ANCHOR: it carries prLinks + spawnPrSets but has + // no in-range turns, so it lives in subagentAnchors, NOT sessions. Its in-range + // child must still reach the PR, and the anchor must not inflate session counts. + const anchor = parent({ id: 'P', prLinks: [A], turns: [], last: '', first: '', agentSpawnLinks: { c1: 'toolu_x' }, spawnPrSets: { toolu_x: [A] } }) const projects = [project([ - parent({ id: 'P', prLinks: [A], turns: [], last: '', agentSpawnLinks: { c1: 'toolu_x' }, spawnPrSets: { toolu_x: [A] } }), - child({ agentId: 'c1', parentId: 'P', cost: 100, firstTs: '2026-07-20T10:05:00Z' }), - ])] + child({ agentId: 'c1', parentId: 'P', cost: 100, firstTs: '2026-07-20T10:05:00Z', last: '2026-07-20T10:30:00Z' }), + ], 'p', [anchor])] const rows = aggregateByPr(projects) expect(rowFor(rows, A)!.cost).toBeCloseTo(100, 6) - expect(prLinkedTotals(projects).subagentSessions).toBe(1) + const totals = prLinkedTotals(projects) + expect(totals.subagentSessions).toBe(1) + expect(totals.sessions).toBe(0) // the anchor is NOT counted as a PR-linked session + // The PR row's date span comes from the CHILD, not the anchor's empty timestamps. + expect(rowFor(rows, A)!.firstStarted).toBe('2026-07-20T10:05:00Z') + expect(rowFor(rows, A)!.lastEnded).toBe('2026-07-20T10:30:00Z') }) }) @@ -280,6 +288,19 @@ describe('MAJOR: timestamp fallback (epoch, end-bounded)', () => { expect(totals.unattributedCost).toBeCloseTo(40, 6) // child fell before the turn -> unattributed }) + it('a child whose spawn link was omitted (ambiguous pairing) still folds via timestamp', () => { + // The parent has NO agentSpawnLinks entry for this child (the spawn-result + // pairing was ambiguous and omitted). The child must still fold via the + // timestamp bucket, not disappear. + const projects = [project([ + parent({ id: 'P', prLinks: [A], last: '2026-07-01T12:00:00Z', turns: [turn({ cost: 10, ts: '2026-07-01T10:00:00Z', prRefs: [A] })] }), + child({ agentId: 'noLink', parentId: 'P', cost: 40, firstTs: '2026-07-01T10:30:00Z' }), + ])] + const rows = aggregateByPr(projects) + expect(rowFor(rows, A)!.cost).toBeCloseTo(50, 6) // 10 parent + 40 child via timestamp bucket + expect(prLinkedTotals(projects).subagentSessions).toBe(1) + }) + it('a child active after the parent last timestamp is UNLINKED (contributes nothing)', () => { const projects = [project([ parent({ @@ -313,24 +334,54 @@ describe('orphans and non-PR parents contribute nothing', () => { }) }) -describe('resolveSubagentAttribution is computed once and shared', () => { +describe('resolveSubagentAttribution', () => { it('resolves each PR-bearing parent to its resolved children', () => { const projects = [project([ parent({ id: 'P', prLinks: [A], turns: [turn({ cost: 10, ts: '2026-07-01T10:00:00Z', prRefs: [A] })], agentSpawnLinks: { c1: 'toolu_x' }, spawnPrSets: { toolu_x: [A] } }), child({ agentId: 'c1', parentId: 'P', cost: 100, firstTs: '2026-07-01T10:05:00Z' }), ])] - const attribution = resolveSubagentAttribution(projects) - expect(attribution.get('P')).toHaveLength(1) - expect(attribution.get('P')![0]!.prSet).toEqual([A]) - expect(attribution.get('P')![0]!.fold.cost).toBe(100) - }) - - it('memoizes by projects identity so the tree is resolved once per render', () => { - const projects = [project([ - parent({ id: 'P', prLinks: [A], turns: [turn({ cost: 10, ts: '2026-07-01T10:00:00Z', prRefs: [A] })], agentSpawnLinks: { c1: 'toolu_x' }, spawnPrSets: { toolu_x: [A] } }), - child({ agentId: 'c1', parentId: 'P', cost: 100, firstTs: '2026-07-01T10:05:00Z' }), - ])] - expect(resolveSubagentAttribution(projects)).toBe(resolveSubagentAttribution(projects)) // same array -> same object - expect(resolveSubagentAttribution(projects)).not.toBe(resolveSubagentAttribution([...projects])) // different array -> recomputed + const resolved = [...resolveSubagentAttribution(projects).values()] + expect(resolved).toHaveLength(1) + expect(resolved[0]!).toHaveLength(1) + expect(resolved[0]![0]!.prSet).toEqual([A]) + expect(resolved[0]![0]!.fold.cost).toBe(100) + }) +}) + +describe('MAJOR: id-collision contamination', () => { + it('folds a child into NEITHER of two distinct parents that share a session id', () => { + // Two DISTINCT parent sessions both have id "P" (duplicate/imported data). A + // child pointing at "P" is ambiguous: it must fold nowhere and stay standalone, + // while both parents attribute their OWN spend only. + const projects = [project([ + parent({ id: 'P', prLinks: [A], turns: [turn({ cost: 10, ts: '2026-07-01T10:00:00Z', prRefs: [A] })], agentSpawnLinks: { c1: 'toolu_x' }, spawnPrSets: { toolu_x: [A] } }), + parent({ id: 'P', prLinks: [B], turns: [turn({ cost: 20, ts: '2026-07-01T11:00:00Z', prRefs: [B] })], agentSpawnLinks: { c1: 'toolu_y' }, spawnPrSets: { toolu_y: [B] } }), + child({ agentId: 'c1', parentId: 'P', cost: 100, firstTs: '2026-07-01T10:05:00Z' }), + ])] + const rows = aggregateByPr(projects) + expect(rowFor(rows, A)!.cost).toBeCloseTo(10, 6) // parent 1 own spend only + expect(rowFor(rows, B)!.cost).toBeCloseTo(20, 6) // parent 2 own spend only + const totals = prLinkedTotals(projects) + expect(totals.subagentSessions).toBe(0) // the ambiguous child folds nowhere + expect(totals.attributedCost).toBeCloseTo(30, 6) // no child double-charge + }) +}) + +describe('MAJOR: recursion dedup is global (diamond folds once)', () => { + it('a grandchild id reachable via two direct children folds exactly once', () => { + // Parent P has two direct children c1 and c2; both point (via duplicate data) + // at a grandchild with the SAME id "agent-gc". The shared claimed-set folds it + // once, not twice. + const projects = [project([ + parent({ id: 'P', prLinks: [A], turns: [turn({ cost: 10, ts: '2026-07-01T10:00:00Z', prRefs: [A] })], agentSpawnLinks: { c1: 'x1', c2: 'x2' }, spawnPrSets: { x1: [A], x2: [A] } }), + child({ agentId: 'c1', parentId: 'P', cost: 100, firstTs: '2026-07-01T10:05:00Z' }), + child({ agentId: 'c2', parentId: 'P', cost: 100, firstTs: '2026-07-01T10:06:00Z' }), + child({ agentId: 'gc', parentId: 'agent-c1', cost: 50, firstTs: '2026-07-01T10:07:00Z' }), + { ...child({ agentId: 'gc', parentId: 'agent-c2', cost: 50, firstTs: '2026-07-01T10:08:00Z' }) }, // duplicate gc id under c2 + ])] + const rows = aggregateByPr(projects) + // 10 (parent) + 100 (c1) + 100 (c2) + 50 (gc, ONCE, not 100). + expect(rowFor(rows, A)!.cost).toBeCloseTo(260, 6) + expect(prLinkedTotals(projects).subagentSessions).toBe(3) // c1, c2, gc once }) })