sessions: round-4 hardening of subagent PR attribution

Fourth adversarial review pass.

- sessionFingerprint now covers the COMPLETE linkage-relevant payload,
  not just headline stats: a canonical (sorted-key) serialization of
  agentSpawnLinks, spawnPrSets, prRefsAtRangeStart, ambiguousSpawnAgentIds,
  parent/agent identity, and the per-turn prRefs timeline. Two records
  that share an id and headline stats but map the child to different
  spawns/PRs now fingerprint DISTINCT, so the ambiguity rule fires and
  they fold into neither, deterministically rather than order-dependent
  first-wins.
- A date/day filter recomputes prRefsAtRangeStart at the new slice
  boundary by replaying the original full turn sequence, instead of
  copying the wide range's value. A PR switch between the wide start and
  the slice start (July 1 A, July 10 B, slice July 20) now carries B, not
  a stale A; a turn exactly on the boundary stays in-slice and applies its
  own refs. The recompute selects by timestamp, so it is order-independent.
  Non-contiguous day selections are documented as treated contiguous from
  the earliest selected day (a single session-level seed cannot represent
  multiple segments; the menubar selection is a single day or a run).
- The anchor-carry path drops an anchor that duplicates a surviving
  session id, so malformed merged input cannot double-count.

Also: rebuilt filtered sessions were losing their PR/subagent-linkage
metadata (a child its parentSessionId, a parent its prLinks), which
carryLinkageFields now restores, so by-PR and folding work on any filtered
slice.

Every fix mutation-verified. A fresh real-data drive re-proves the
no-double-count identity and reconciliation to the cent for both a
lifetime scan and a day-filtered slice (anchors created, subagents fold
through the filter).
This commit is contained in:
reviewer 2026-07-21 05:15:40 +02:00
parent b16b70e4a1
commit 75d0c709f1
3 changed files with 167 additions and 12 deletions

View file

@ -3144,7 +3144,8 @@ function isSpawnParent(session: SessionSummary): boolean {
function carryLinkageFields(rebuilt: SessionSummary, original: SessionSummary): void {
if (original.everHadBranch) rebuilt.everHadBranch = true
if (original.prLinks?.length) rebuilt.prLinks = original.prLinks
if (original.prRefsAtRangeStart?.length) rebuilt.prRefsAtRangeStart = original.prRefsAtRangeStart
// prRefsAtRangeStart is NOT copied here: a narrower slice needs it recomputed at
// the new boundary (see recomputeRangeStartPrRefs), not the wide range's value.
if (original.parentSessionId) rebuilt.parentSessionId = original.parentSessionId
if (original.agentId) rebuilt.agentId = original.agentId
if (original.agentSpawnLinks) rebuilt.agentSpawnLinks = original.agentSpawnLinks
@ -3154,7 +3155,52 @@ function carryLinkageFields(rebuilt: SessionSummary, original: SessionSummary):
if (original.agentType) rebuilt.agentType = original.agentType
}
// The "PR active entering this slice", recomputed by replaying the ORIGINAL full
// turn sequence up to `sliceStartMs`, seeded from the original range-start state.
// A narrower filter must NOT reuse the wide range's range-start PR: a PR switch
// between the wide start and the slice start would otherwise be lost, mis-seeding
// both spend attribution and the subagent grace fallback. A turn exactly ON the
// boundary stays in the slice and applies its own prRefs there, so the walk stops
// strictly before it.
function recomputeRangeStartPrRefs(original: SessionSummary, sliceStartMs: number): string[] | undefined {
// The carried PR is the refs of the LATEST turn (by timestamp) strictly before the
// slice that referenced any PR; a turn exactly on the boundary is inside the slice
// and applies its own refs there. Selected by timestamp, not array position, so
// the result does not depend on turn ordering. Falls back to the original
// range-start state when nothing referenced a PR before the slice.
let current = original.prRefsAtRangeStart
let bestMs = -Infinity
for (const turn of original.turns) {
if (!turn.prRefs?.length) continue
const ts = turn.assistantCalls[0]?.timestamp
if (!ts) continue
const tMs = new Date(ts).getTime()
if (Number.isNaN(tMs) || tMs >= sliceStartMs) continue
if (tMs >= bestMs) { bestMs = tMs; current = turn.prRefs }
}
return current
}
// Apply a recomputed range-start PR state to a rebuilt session (or clear it).
function applyRecomputedRangeStart(rebuilt: SessionSummary, original: SessionSummary, sliceStartMs: number): void {
const rs = recomputeRangeStartPrRefs(original, sliceStartMs)
if (rs?.length) rebuilt.prRefsAtRangeStart = rs
else delete rebuilt.prRefsAtRangeStart
}
// Local-midnight epoch of the EARLIEST selected day. Range-start PR state is
// recomputed at this boundary. Non-contiguous day selections are treated as
// CONTIGUOUS from the earliest day: a PR switch inside an unselected gap between two
// selected days is not reflected in the carried state (a single session-level seed
// cannot represent multiple segments). The menubar day selection is a single day or
// a contiguous run, so this is exact in practice.
function earliestDayStartMs(days: Set<string>): number {
const earliest = [...days].sort()[0]
return earliest ? new Date(`${earliest}T00:00:00`).getTime() : NaN
}
export function filterProjectsByDays(projects: ProjectSummary[], days: Set<string>): ProjectSummary[] {
const sliceStartMs = earliestDayStartMs(days)
const filtered: ProjectSummary[] = []
for (const project of projects) {
const sessions: SessionSummary[] = []
@ -3163,6 +3209,7 @@ export function filterProjectsByDays(projects: ProjectSummary[], days: Set<strin
// so its surviving in-range child still resolves. The anchor contributes no
// own spend either way.
const anchors: SessionSummary[] = [...(project.subagentAnchors ?? [])]
const survivingIds = new Set<string>()
for (const session of project.sessions) {
const turns = session.turns.filter(turn => {
const ds = turnDayString(turn)
@ -3174,10 +3221,15 @@ export function filterProjectsByDays(projects: ProjectSummary[], days: Set<strin
}
const rebuilt = buildSessionSummary(session.sessionId, session.project, turns, session.mcpInventory, session.source)
carryLinkageFields(rebuilt, session)
if (!Number.isNaN(sliceStartMs)) applyRecomputedRangeStart(rebuilt, session, sliceStartMs)
survivingIds.add(session.sessionId)
sessions.push(rebuilt)
}
if (sessions.length === 0 && anchors.length === 0) continue
filtered.push(summarizeProject(project.project, project.projectPath, sessions, anchors))
// A surviving session (real, in-range spend) supersedes any anchor with the same
// id: keep the session, drop the duplicate anchor (guards malformed merged input).
const dedupedAnchors = survivingIds.size ? anchors.filter(a => !survivingIds.has(a.sessionId)) : anchors
if (sessions.length === 0 && dedupedAnchors.length === 0) continue
filtered.push(summarizeProject(project.project, project.projectPath, sessions, dedupedAnchors))
}
return filtered.sort((a, b) => b.totalCostUSD - a.totalCostUSD)
}
@ -3230,12 +3282,14 @@ export function filterProjectsByClaudeConfigSource(projects: ProjectSummary[], s
}
export function filterProjectsByDateRange(projects: ProjectSummary[], dateRange: DateRange): ProjectSummary[] {
const sliceStartMs = dateRange.start.getTime()
const filtered: ProjectSummary[] = []
for (const project of projects) {
const sessions: SessionSummary[] = []
// Carry existing anchors and convert a spawn parent whose in-range turns are all
// filtered out into one (see filterProjectsByDays).
const anchors: SessionSummary[] = [...(project.subagentAnchors ?? [])]
const survivingIds = new Set<string>()
for (const session of project.sessions) {
const turns = session.turns.filter(turn => turnIsInDateRange(turn, dateRange))
if (turns.length === 0) {
@ -3244,10 +3298,13 @@ export function filterProjectsByDateRange(projects: ProjectSummary[], dateRange:
}
const rebuilt = buildSessionSummary(session.sessionId, session.project, turns, session.mcpInventory, session.source)
carryLinkageFields(rebuilt, session)
applyRecomputedRangeStart(rebuilt, session, sliceStartMs)
survivingIds.add(session.sessionId)
sessions.push(rebuilt)
}
if (sessions.length === 0 && anchors.length === 0) continue
filtered.push(summarizeProject(project.project, project.projectPath, sessions, anchors))
const dedupedAnchors = survivingIds.size ? anchors.filter(a => !survivingIds.has(a.sessionId)) : anchors
if (sessions.length === 0 && dedupedAnchors.length === 0) continue
filtered.push(summarizeProject(project.project, project.projectPath, sessions, dedupedAnchors))
}
return filtered.sort((a, b) => b.totalCostUSD - a.totalCostUSD)
}

View file

@ -239,12 +239,36 @@ function rowSessionKey(session: SessionSummary): string {
return `${linkageProvider(session)}${KEY_SEP}${session.project}${KEY_SEP}${session.sessionId}`
}
// Sorted-key canonical serialization of a Record, so two records that map the same
// entries fingerprint EQUAL regardless of insertion order.
function canonicalRecord(rec: Record<string, string | string[]> | undefined): string {
if (!rec) return ''
return Object.keys(rec).sort().map(k => {
const v = rec[k]!
return `${k}=${Array.isArray(v) ? v.join(',') : v}`
}).join(';')
}
// Distinguishes two DIFFERENT records that happen to share a session id (duplicate
// or imported data): identical copies produce the same fingerprint and fold once,
// any difference (cost, calls, span, PR links) marks the key ambiguous so it folds
// into NEITHER parent/subtree.
// any difference marks the key ambiguous so it folds into NEITHER parent/subtree.
// The fingerprint covers EVERY field that changes fold behavior, not just headline
// stats: two parents mapping the same child to different spawns/PRs (agentSpawnLinks
// / spawnPrSets) must differ here, or the ambiguity rule never fires and a stale
// first-wins returns an order-dependent result.
function sessionFingerprint(s: SessionSummary): string {
return [s.totalCostUSD, s.apiCalls, s.firstTimestamp, s.lastTimestamp, (s.prLinks ?? []).join(',')].join(KEY_SEP)
return JSON.stringify([
s.totalCostUSD, s.apiCalls, s.firstTimestamp, s.lastTimestamp,
s.parentSessionId ?? '', s.agentId ?? '',
(s.prLinks ?? []),
(s.prRefsAtRangeStart ?? []),
[...(s.ambiguousSpawnAgentIds ?? [])].sort(),
canonicalRecord(s.agentSpawnLinks),
canonicalRecord(s.spawnPrSets),
// The per-turn PR-ref timeline: the launch turn's PR is what a child inherits,
// so two records with different prRefs sequences are distinct folds.
s.turns.map(t => (t.prRefs ?? []).join('|')).join('>'),
])
}
/// Index every sidechain (subagent) session by the parent that spawned it, keyed

View file

@ -6,6 +6,7 @@ import {
prLinkedTotals,
resolveSubagentAttribution,
} from '../src/sessions-report.js'
import { filterProjectsByDateRange, filterProjectsByDays } from '../src/parser.js'
import type { ClassifiedTurn, ParsedApiCall, ProjectSummary, SessionSummary, TokenUsage } from '../src/types.js'
const A = 'https://github.com/o/r/pull/1'
@ -367,6 +368,23 @@ describe('MAJOR: id-collision contamination', () => {
expect(totals.attributedCost).toBeCloseTo(30, 6) // no child double-charge
})
it('folds nowhere when two parents share id + headline stats but map the child to different spawns/PRs', () => {
// Same cost/calls/prLinks/turn-refs, but P1 maps c1 -> spawn x (PR A) and P2 maps
// c1 -> spawn y (PR B). A fingerprint over headline stats alone would miss this
// and first-wins by input order; the full linkage fingerprint makes it ambiguous.
const mk = (order: 'p1' | 'p2') => {
const p1 = parent({ id: 'P', prLinks: [A, B], turns: [turn({ cost: 10, ts: '2026-07-01T10:00:00Z', prRefs: [A] })], agentSpawnLinks: { c1: 'x' }, spawnPrSets: { x: [A] } })
const p2 = parent({ id: 'P', prLinks: [A, B], turns: [turn({ cost: 10, ts: '2026-07-01T10:00:00Z', prRefs: [A] })], agentSpawnLinks: { c1: 'y' }, spawnPrSets: { y: [B] } })
const c = child({ agentId: 'c1', parentId: 'P', cost: 100, firstTs: '2026-07-01T10:05:00Z' })
return [project(order === 'p1' ? [p1, p2, c] : [p2, p1, c])]
}
for (const order of ['p1', 'p2'] as const) {
const totals = prLinkedTotals(mk(order))
expect(totals.subagentSessions).toBe(0) // ambiguous identity -> child folds nowhere
expect(totals.attributedCost).toBeCloseTo(20, 6) // only the two parents own $10 turns
}
})
it('folds nowhere when a PR-bearing parent shares its id with a PR-LESS parent', () => {
// Only ONE of the two colliding parents has prLinks, but the identity is still
// ambiguous: count ALL candidates, not just PR-bearing ones.
@ -418,6 +436,50 @@ describe('MAJOR: ambiguous pairing + late child grace window', () => {
})
})
describe('MAJOR: range-start PR state is recomputed at a filter boundary', () => {
const A_TURN = () => turn({ cost: 10, ts: '2026-07-01T10:00:00Z', prRefs: [A] })
const B_TURN = () => turn({ cost: 10, ts: '2026-07-10T10:00:00Z', prRefs: [B] })
const LATE_TURN = () => turn({ cost: 10, ts: '2026-07-20T10:00:00Z' }) // no refs, carries the active PR
// The wide parse recorded PR A as active entering the range; a switch to B lands
// July 10. A slice starting July 20 must attribute the ref-less July 20 turn to B,
// NOT the stale A. Tested under both turn orderings.
const build = (turns: ClassifiedTurn[]) => [project([parent({ id: 'S', prLinks: [A, B], prRefsAtRangeStart: [A], turns })])]
it('recomputes to B (July 1 A -> July 10 B, slice July 20), both turn orders', () => {
const range = { start: new Date('2026-07-20T00:00:00Z'), end: new Date('2026-07-21T23:59:59Z') }
for (const turns of [[A_TURN(), B_TURN(), LATE_TURN()], [LATE_TURN(), B_TURN(), A_TURN()]]) {
const rows = aggregateByPr(filterProjectsByDateRange(build(turns), range))
expect(rowFor(rows, B)?.cost ?? 0).toBeCloseTo(10, 6) // ref-less July 20 turn -> B
expect(rowFor(rows, A)).toBeUndefined() // NOT the stale range-start A
}
})
it('a slice starting exactly ON the switch turn attributes that turn to the new PR', () => {
// Switch to B lands July 10 10:00; slice starts July 10 00:00. The July 10 turn is
// inside the slice and applies B; the recomputed seed (from before) is A.
const range = { start: new Date('2026-07-10T00:00:00Z'), end: new Date('2026-07-21T23:59:59Z') }
const rows = aggregateByPr(filterProjectsByDateRange(build([A_TURN(), B_TURN(), LATE_TURN()]), range))
expect(rowFor(rows, B)!.cost).toBeCloseTo(20, 6) // July 10 (B) + July 20 (carried B)
expect(rowFor(rows, A)).toBeUndefined() // July 1 A turn is out of slice
})
it('day filter recomputes the same way (menubar path)', () => {
const rows = aggregateByPr(filterProjectsByDays(build([A_TURN(), B_TURN(), LATE_TURN()]), new Set(['2026-07-20'])))
expect(rowFor(rows, B)?.cost ?? 0).toBeCloseTo(10, 6)
expect(rowFor(rows, A)).toBeUndefined()
})
it('drops an anchor that duplicates a surviving session id (malformed merged input)', () => {
// A surviving in-range session and a stray anchor carry the SAME id: the real
// session wins, the duplicate anchor is dropped.
const surviving = parent({ id: 'dup', prLinks: [A], turns: [turn({ cost: 10, ts: '2026-07-20T10:00:00Z', prRefs: [A] })], spawnPrSets: { x: [A] } })
const strayAnchor = parent({ id: 'dup', prLinks: [A], turns: [], spawnPrSets: { x: [A] } })
const proj = { ...project([surviving]), subagentAnchors: [strayAnchor] }
const filtered = filterProjectsByDays([proj], new Set(['2026-07-20']))
expect(filtered.flatMap(p => p.subagentAnchors ?? []).some(a => a.sessionId === 'dup')).toBe(false)
})
})
describe('MINOR: row session-key delimiter does not collide on names with spaces', () => {
it('counts two sessions whose space-joined keys would collide as distinct', () => {
// "a b" + "c" and "a" + "b c" both become "a b c" under a space delimiter,
@ -451,10 +513,22 @@ describe('MAJOR: recursion dedup and conflicting duplicates', () => {
}
})
it('identical duplicate ids fold exactly once', () => {
// Same id, same fingerprint (cost/span/links): one logical session, folds once.
it('a truly identical duplicate child (same parent, same record) folds exactly once', () => {
// Two copies of the SAME child under the SAME parent, identical in every
// fingerprinted field: one logical session, folds once (not doubled).
const mk = () => [project([
parent({ id: 'P', prLinks: [A], turns: [turn({ cost: 10, ts: '2026-07-01T10:00:00Z', prRefs: [A] })], agentSpawnLinks: { c1: 'x' }, spawnPrSets: { x: [A] } }),
child({ agentId: 'c1', parentId: 'P', cost: 100, firstTs: '2026-07-01T10:05:00Z' }),
child({ agentId: 'c1', parentId: 'P', cost: 100, firstTs: '2026-07-01T10:05:00Z' }), // exact duplicate
])]
expect(rowFor(aggregateByPr(mk()), A)!.cost).toBeCloseTo(110, 6) // 10 + 100 (once, not 210)
expect(prLinkedTotals(mk()).subagentSessions).toBe(1)
})
it('the SAME id under DIFFERENT parents is ambiguous even at equal cost (distinct identity)', () => {
// Different parentSessionId is part of the fingerprint, so these are NOT the
// same logical session: fold neither.
const rows = aggregateByPr(diamond(50, 50, 'c1first'))
expect(rowFor(rows, A)!.cost).toBeCloseTo(260, 6) // 10 + 100 + 100 + 50 (once)
expect(prLinkedTotals(diamond(50, 50, 'c1first')).subagentSessions).toBe(3)
expect(rowFor(rows, A)!.cost).toBeCloseTo(210, 6) // grandchild folds nowhere
})
})