mirror of
https://github.com/AgentSeal/codeburn.git
synced 2026-07-29 10:55:35 +00:00
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.
This commit is contained in:
parent
3625b59065
commit
b95cb95ce4
2 changed files with 266 additions and 15 deletions
93
src/yield.ts
93
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<string, RepoIdentity | null>,
|
||||
): 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<string, RepoIdentity | null>()
|
||||
|
||||
// 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<string, RepoGroup>()
|
||||
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)
|
||||
|
|
|
|||
188
tests/yield-repo-grouping.test.ts
Normal file
188
tests/yield-repo-grouping.test.ts
Normal file
|
|
@ -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, string> = {}): 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>): 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 })
|
||||
}
|
||||
})
|
||||
})
|
||||
Loading…
Add table
Add a link
Reference in a new issue