From b95cb95ce484e0be71d7d90879f7db3a90fd143e Mon Sep 17 00:00:00 2001 From: iamtoruk Date: Thu, 16 Jul 2026 18:58:49 -0700 Subject: [PATCH] fix(yield): group commit attribution by canonical repo, not launch path Sessions launched from monorepo subdirectories or separate worktrees of one repository formed independent attribution groups, each awarding the same commit (double-counted productive sessions; the duplicate owner was not even flagged ambiguous). Groups now key on the realpath of git-common-dir, so all subdirs and worktrees of a repo collapse into one group while distinct repos stay separate; ambiguity semantics unchanged, now correctly spanning the whole repo. rev-parse results cached per directory. Fixes #713. Regression tests fail on the unfixed code. --- src/yield.ts | 93 ++++++++++++--- tests/yield-repo-grouping.test.ts | 188 ++++++++++++++++++++++++++++++ 2 files changed, 266 insertions(+), 15 deletions(-) create mode 100644 tests/yield-repo-grouping.test.ts diff --git a/src/yield.ts b/src/yield.ts index eeee66d..c0c2e7e 100644 --- a/src/yield.ts +++ b/src/yield.ts @@ -1,4 +1,6 @@ import { execFileSync } from 'child_process' +import { realpathSync } from 'fs' +import { resolve } from 'path' import { parseAllSessions } from './parser.js' import type { DateRange, SessionSummary } from './types.js' @@ -60,8 +62,59 @@ function runGit(args: string[], cwd: string): string | null { } } -function isGitRepo(dir: string): boolean { - return runGit(['rev-parse', '--is-inside-work-tree'], dir) === 'true' +type RepoIdentity = { + /** Canonical group key: the absolute git-common-dir (shared object store). */ + readonly key: string + /** A member directory to run `git log` from; --all spans the whole store. */ + readonly gitDir: string +} + +function canonicalPath(path: string): string { + try { + return realpathSync(path) + } catch { + return path + } +} + +/** + * Resolve a directory to its canonical repository identity, or null when it is + * not inside a git work tree. + * + * The key is the absolute `git-common-dir` (the shared object store), NOT the + * raw path: `git rev-parse --is-inside-work-tree` is true in every subdirectory + * and `git log --all` returns the same repo-wide list from any of them, so + * keying groups on the raw path would let two monorepo subdirectories — or two + * worktrees of one repo — each award the same commit. All subdirectories AND + * all worktrees of one repo share this common-dir, so they collapse to a single + * group; distinct repositories keep distinct keys. + * + * Cached per directory: resolution runs once per unique path (many sessions + * share a path), never once per session. + */ +function resolveRepoIdentity( + dir: string, + cache: Map, +): RepoIdentity | null { + const cached = cache.get(dir) + if (cached !== undefined) return cached + + let identity: RepoIdentity | null = null + // One fork answers both questions; line 1 is --is-inside-work-tree, line 2 the + // --git-common-dir (relative to `dir` for a subdir, absolute for a worktree). + const out = runGit(['rev-parse', '--is-inside-work-tree', '--git-common-dir'], dir) + if (out) { + const [insideWorkTree, commonDir] = out.split('\n') + if (insideWorkTree === 'true' && commonDir) { + // realpath canonicalizes symlinks: git reports a linked worktree's + // common-dir as a realpath but the main worktree's as ".git" relative to + // its (possibly symlinked) path, so both must be canonicalized to collapse + // to one key. + identity = { key: canonicalPath(resolve(dir, commonDir)), gitDir: dir } + } + } + cache.set(dir, identity) + return identity } function getMainBranch(cwd: string): string { @@ -252,32 +305,42 @@ export async function computeYield(range: DateRange, cwd: string, provider: stri details: [], } + const repoIdentityCache = new Map() + // Get all commits in the date range for correlation - const commits = isGitRepo(cwd) + const cwdIdentity = resolveRepoIdentity(cwd, repoIdentityCache) + const cwdCommits = cwdIdentity ? getCommitsInRange(cwd, range.start, range.end, getMainBranch(cwd)) : [] - // Group sessions by resolved repo before attributing: every project entry - // whose projectPath is missing (or not a git repo) falls back to the same - // cwd and shares one commit list, so attribution must run once per repo or - // two such entries could each award the same commit to one of their sessions. + // Group sessions by canonical repository identity before attributing so that + // each commit is awarded at most once across the whole repo. Two monorepo + // subdirectory sessions, or two worktrees of one repo, resolve to the same + // git-common-dir and share ONE group; keying on the raw path would double + // count. A project whose path is missing or not a git repo falls back to the + // cwd repo (or, when cwd is not a repo either, an empty commit list) exactly + // as before. type RepoGroup = { commits: CommitInfo[]; sessions: SessionSummary[]; projectNames: string[] } const repoGroups = new Map() for (const project of projects) { - const projectCwd = project.projectPath && isGitRepo(project.projectPath) - ? project.projectPath - : cwd + const projectIdentity = project.projectPath + ? resolveRepoIdentity(project.projectPath, repoIdentityCache) + : null + const identity = projectIdentity ?? cwdIdentity + const groupKey = identity ? identity.key : cwd - let group = repoGroups.get(projectCwd) + let group = repoGroups.get(groupKey) if (!group) { group = { - commits: projectCwd === cwd - ? commits - : getCommitsInRange(projectCwd, range.start, range.end, getMainBranch(projectCwd)), + commits: !identity + ? [] + : cwdIdentity && identity.key === cwdIdentity.key + ? cwdCommits + : getCommitsInRange(identity.gitDir, range.start, range.end, getMainBranch(identity.gitDir)), sessions: [], projectNames: [], } - repoGroups.set(projectCwd, group) + repoGroups.set(groupKey, group) } for (const session of project.sessions) { group.sessions.push(session) diff --git a/tests/yield-repo-grouping.test.ts b/tests/yield-repo-grouping.test.ts new file mode 100644 index 0000000..35f174b --- /dev/null +++ b/tests/yield-repo-grouping.test.ts @@ -0,0 +1,188 @@ +import { execFileSync } from 'node:child_process' +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest' + +import type { ProjectSummary, SessionSummary } from '../src/types.js' +import { computeYield } from '../src/yield.js' + +const { parseAllSessionsMock } = vi.hoisted(() => ({ + parseAllSessionsMock: vi.fn(), +})) + +vi.mock('../src/parser.js', () => ({ + parseAllSessions: parseAllSessionsMock, +})) + +function git(cwd: string, args: string[], env: Record = {}): string { + return execFileSync('git', args, { + cwd, + encoding: 'utf-8', + env: { ...process.env, ...env }, + }).trim() +} + +function initRepo(dir: string): void { + git(dir, ['init', '-b', 'main']) + git(dir, ['config', 'user.email', 'test@example.com']) + git(dir, ['config', 'user.name', 'Test']) +} + +function commitAt(dir: string, message: string, iso: string): void { + git(dir, ['add', '.']) + git(dir, ['commit', '-m', message], { + GIT_AUTHOR_DATE: iso, + GIT_COMMITTER_DATE: iso, + }) +} + +function makeSession(overrides: Partial): SessionSummary { + return { + sessionId: 'session', + project: 'app', + firstTimestamp: '2026-01-01T10:00:00.000Z', + lastTimestamp: '2026-01-01T11:00:00.000Z', + totalCostUSD: 1, + totalSavingsUSD: 0, + totalInputTokens: 0, + totalOutputTokens: 0, + totalReasoningTokens: 0, + totalCacheReadTokens: 0, + totalCacheWriteTokens: 0, + apiCalls: 1, + turns: [], + modelBreakdown: {}, + toolBreakdown: {}, + mcpBreakdown: {}, + bashBreakdown: {}, + categoryBreakdown: {} as SessionSummary['categoryBreakdown'], + skillBreakdown: {}, + subagentBreakdown: {}, + ...overrides, + } +} + +const range = { + start: new Date('2026-01-01T00:00:00.000Z'), + end: new Date('2026-01-02T00:00:00.000Z'), +} + +// Tighter window (span 1.5h): 10:15 -> 10:45 (+1h = 11:45). Wins any commit it shares. +const tightWindow = { + firstTimestamp: '2026-01-01T10:15:00.000Z', + lastTimestamp: '2026-01-01T10:45:00.000Z', +} +// Broader window (span 2h): 10:00 -> 11:00 (+1h = 12:00). Loses the shared commit. +const broadWindow = { + firstTimestamp: '2026-01-01T10:00:00.000Z', + lastTimestamp: '2026-01-01T11:00:00.000Z', +} + +describe('yield repo grouping by canonical repository identity (issue #713)', () => { + it('credits one commit once across two monorepo subdirectory sessions', async () => { + const repoDir = await mkdtemp(join(tmpdir(), 'codeburn-yield-monorepo-')) + try { + initRepo(repoDir) + // Two package subdirectories of ONE repo. `git rev-parse --is-inside-work-tree` + // is true in both, and `git log --all` returns the same repo-wide commit + // list from either, so keying groups by the raw subdir path double-credits. + await writeFile(join(repoDir, 'a.txt'), 'a\n') + await writeFile(join(repoDir, 'b.txt'), 'b\n') + commitAt(repoDir, 'feat: shipped once', '2026-01-01T10:30:00Z') + + const subA = join(repoDir, 'packages', 'a') + const subB = join(repoDir, 'packages', 'b') + await mkdir(subA, { recursive: true }) + await mkdir(subB, { recursive: true }) + + const sessionA = makeSession({ sessionId: 'sub-a', project: 'pkg-a', ...tightWindow, totalCostUSD: 5 }) + const sessionB = makeSession({ sessionId: 'sub-b', project: 'pkg-b', ...broadWindow, totalCostUSD: 3 }) + + parseAllSessionsMock.mockResolvedValue([ + { project: 'pkg-a', projectPath: subA, sessions: [sessionA] } as ProjectSummary, + { project: 'pkg-b', projectPath: subB, sessions: [sessionB] } as ProjectSummary, + ]) + + const summary = await computeYield(range, repoDir) + + const productive = summary.details.filter(d => d.category === 'productive') + expect(productive.map(d => d.sessionId)).toEqual(['sub-a']) + expect(productive[0]!.commitCount).toBe(1) + + const detailB = summary.details.find(d => d.sessionId === 'sub-b')! + expect(detailB.category).toBe('ambiguous') + expect(detailB.commitCount).toBe(0) + expect(summary.ambiguous).toEqual({ cost: 3, sessions: 1 }) + } finally { + await rm(repoDir, { recursive: true, force: true }) + } + }) + + it('credits one commit once across two worktrees of one repo', async () => { + const repoDir = await mkdtemp(join(tmpdir(), 'codeburn-yield-worktree-')) + let wtDir = '' + try { + initRepo(repoDir) + await writeFile(join(repoDir, 'file.txt'), 'hello\n') + commitAt(repoDir, 'feat: shipped once', '2026-01-01T10:30:00Z') + + // Linked worktree on a different branch, sharing the same object store. + wtDir = join(repoDir, '..', `${repoDir.split('/').pop()}-wt`) + git(repoDir, ['worktree', 'add', wtDir, '-b', 'feature']) + + const sessionMain = makeSession({ sessionId: 'wt-main', project: 'app', ...tightWindow, totalCostUSD: 5 }) + const sessionLinked = makeSession({ sessionId: 'wt-linked', project: 'app', ...broadWindow, totalCostUSD: 3 }) + + parseAllSessionsMock.mockResolvedValue([ + { project: 'app', projectPath: repoDir, sessions: [sessionMain] } as ProjectSummary, + { project: 'app', projectPath: wtDir, sessions: [sessionLinked] } as ProjectSummary, + ]) + + const summary = await computeYield(range, repoDir) + + const productive = summary.details.filter(d => d.category === 'productive') + expect(productive.map(d => d.sessionId)).toEqual(['wt-main']) + expect(productive[0]!.commitCount).toBe(1) + + const detailLinked = summary.details.find(d => d.sessionId === 'wt-linked')! + expect(detailLinked.category).toBe('ambiguous') + expect(detailLinked.commitCount).toBe(0) + } finally { + if (wtDir) await rm(wtDir, { recursive: true, force: true }) + await rm(repoDir, { recursive: true, force: true }) + } + }) + + it('keeps two genuinely separate repos as independent groups (no over-merge)', async () => { + const repo1 = await mkdtemp(join(tmpdir(), 'codeburn-yield-sep1-')) + const repo2 = await mkdtemp(join(tmpdir(), 'codeburn-yield-sep2-')) + try { + for (const [dir, name] of [[repo1, 'first'], [repo2, 'second']] as const) { + initRepo(dir) + await writeFile(join(dir, 'file.txt'), `${name}\n`) + commitAt(dir, `feat: ${name}`, '2026-01-01T10:30:00Z') + } + + // Identical overlapping windows: only correct because the repos are distinct. + const session1 = makeSession({ sessionId: 'repo1-session', project: 'r1', ...broadWindow, totalCostUSD: 4 }) + const session2 = makeSession({ sessionId: 'repo2-session', project: 'r2', ...broadWindow, totalCostUSD: 4 }) + + parseAllSessionsMock.mockResolvedValue([ + { project: 'r1', projectPath: repo1, sessions: [session1] } as ProjectSummary, + { project: 'r2', projectPath: repo2, sessions: [session2] } as ProjectSummary, + ]) + + const summary = await computeYield(range, repo1) + + const productive = summary.details.filter(d => d.category === 'productive') + expect(productive.map(d => d.sessionId).sort()).toEqual(['repo1-session', 'repo2-session']) + expect(productive.every(d => d.commitCount === 1)).toBe(true) + expect(summary.ambiguous).toEqual({ cost: 0, sessions: 0 }) + } finally { + await rm(repo1, { recursive: true, force: true }) + await rm(repo2, { recursive: true, force: true }) + } + }) +})