diff --git a/src/main/git/status-branch-line-total-exec-contract.test.ts b/src/main/git/status-branch-line-total-exec-contract.test.ts index 13ece36da..7dff12b54 100644 --- a/src/main/git/status-branch-line-total-exec-contract.test.ts +++ b/src/main/git/status-branch-line-total-exec-contract.test.ts @@ -67,6 +67,8 @@ import { } from './status' const BOGUS_MERGE_BASE = 'deadbeef'.repeat(5) +// No fixture path here looks like test or generated code, so it is all source. +const NO_LINES = { added: 0, removed: 0 } const tempRoots: string[] = [] function git(repo: string, args: string[]): string { @@ -257,7 +259,13 @@ describe('branch line total exec budget', () => { getStatus(repo, { branchLineTotalMergeBase: mergeBase }) ]) - expect(first.branchLineTotal).toEqual({ added: 1, removed: 0, mergeBase }) + expect(first.branchLineTotal).toEqual({ + added: 1, + removed: 0, + mergeBase, + test: NO_LINES, + generated: NO_LINES + }) expect(second.branchLineTotal).toEqual(first.branchLineTotal) expect(rangedDiffCalls()).toHaveLength(1) }) @@ -280,7 +288,13 @@ describe('branch line total exec budget', () => { getStatus(repo, { branchLineTotalMergeBase: mergeBase, limit: 4096 }) ]) - expect(first.branchLineTotal).toEqual({ added: 1, removed: 0, mergeBase }) + expect(first.branchLineTotal).toEqual({ + added: 1, + removed: 0, + mergeBase, + test: NO_LINES, + generated: NO_LINES + }) expect(second.branchLineTotal).toEqual(first.branchLineTotal) // Proves the saving came from the branch-total coalescer: both passes really // ran their own `git status`, so the status lease merged nothing. @@ -306,7 +320,13 @@ describe('branch line total exec budget', () => { }) expect(second.branchLineTotal).toEqual(first.branchLineTotal) - expect(second.branchLineTotal).toEqual({ added: 1, removed: 0, mergeBase }) + expect(second.branchLineTotal).toEqual({ + added: 1, + removed: 0, + mergeBase, + test: NO_LINES, + generated: NO_LINES + }) expect(numstatCalls()).toEqual([]) }) @@ -324,7 +344,13 @@ describe('branch line total exec budget', () => { reuseLineStats: true }) - expect(result.branchLineTotal).toEqual({ added: 0, removed: 0, mergeBase: laterMergeBase }) + expect(result.branchLineTotal).toEqual({ + added: 0, + removed: 0, + mergeBase: laterMergeBase, + test: NO_LINES, + generated: NO_LINES + }) expect(rangedDiffCalls()).toHaveLength(1) }) }) diff --git a/src/main/git/status-branch-line-total-real-git.test.ts b/src/main/git/status-branch-line-total-real-git.test.ts index 80426c76e..1748efb29 100644 --- a/src/main/git/status-branch-line-total-real-git.test.ts +++ b/src/main/git/status-branch-line-total-real-git.test.ts @@ -40,6 +40,9 @@ function commitAll(repo: string, message: string): string { return git(repo, ['rev-parse', 'HEAD']) } +// No fixture path here looks like test or generated code, so it is all source. +const NO_LINES = { added: 0, removed: 0 } + /** What a plain `git diff ` reports, i.e. the number the chip must not disagree with. */ function rangedDiffTotal(repo: string, mergeBase: string): { added: number; removed: number } { let added = 0 @@ -80,7 +83,13 @@ describe('branch line total against a real repository', () => { const result = await getStatus(repo, { branchLineTotalMergeBase: mergeBase }) expect(rangedDiffTotal(repo, mergeBase)).toEqual({ added: 1, removed: 0 }) - expect(result.branchLineTotal).toEqual({ added: 1, removed: 0, mergeBase }) + expect(result.branchLineTotal).toEqual({ + added: 1, + removed: 0, + mergeBase, + test: NO_LINES, + generated: NO_LINES + }) expect(result.entries.map((entry) => [entry.area, entry.added, entry.removed])).toEqual([ ['staged', 1, 0], ['unstaged', 1, 1] @@ -97,7 +106,13 @@ describe('branch line total against a real repository', () => { const result = await getStatus(repo, { branchLineTotalMergeBase: mergeBase }) - expect(result.branchLineTotal).toEqual({ added: 0, removed: 0, mergeBase }) + expect(result.branchLineTotal).toEqual({ + added: 0, + removed: 0, + mergeBase, + test: NO_LINES, + generated: NO_LINES + }) }) it('keeps the total across a commit and updates it on the next status call after an edit', async () => { @@ -107,15 +122,33 @@ describe('branch line total against a real repository', () => { await write(repo, 'f.txt', 'a\nb\nc\n') const beforeCommit = await getStatus(repo, { branchLineTotalMergeBase: mergeBase }) - expect(beforeCommit.branchLineTotal).toEqual({ added: 2, removed: 0, mergeBase }) + expect(beforeCommit.branchLineTotal).toEqual({ + added: 2, + removed: 0, + mergeBase, + test: NO_LINES, + generated: NO_LINES + }) commitAll(repo, 'commit the edit') const afterCommit = await getStatus(repo, { branchLineTotalMergeBase: mergeBase }) - expect(afterCommit.branchLineTotal).toEqual({ added: 2, removed: 0, mergeBase }) + expect(afterCommit.branchLineTotal).toEqual({ + added: 2, + removed: 0, + mergeBase, + test: NO_LINES, + generated: NO_LINES + }) await write(repo, 'f.txt', 'a\nb\nc\nd\n') const afterSecondEdit = await getStatus(repo, { branchLineTotalMergeBase: mergeBase }) - expect(afterSecondEdit.branchLineTotal).toEqual({ added: 3, removed: 0, mergeBase }) + expect(afterSecondEdit.branchLineTotal).toEqual({ + added: 3, + removed: 0, + mergeBase, + test: NO_LINES, + generated: NO_LINES + }) }) it('counts a rename across the commit boundary once, using the post-rename path', async () => { @@ -129,7 +162,13 @@ describe('branch line total against a real repository', () => { const result = await getStatus(repo, { branchLineTotalMergeBase: mergeBase }) expect(rangedDiffTotal(repo, mergeBase)).toEqual({ added: 1, removed: 0 }) - expect(result.branchLineTotal).toEqual({ added: 1, removed: 0, mergeBase }) + expect(result.branchLineTotal).toEqual({ + added: 1, + removed: 0, + mergeBase, + test: NO_LINES, + generated: NO_LINES + }) }) it('counts an untracked-only branch from the untracked file contents', async () => { @@ -141,7 +180,13 @@ describe('branch line total against a real repository', () => { const result = await getStatus(repo, { branchLineTotalMergeBase: mergeBase }) expect(result.entries.map((entry) => entry.area)).toEqual(['untracked']) - expect(result.branchLineTotal).toEqual({ added: 3, removed: 0, mergeBase }) + expect(result.branchLineTotal).toEqual({ + added: 3, + removed: 0, + mergeBase, + test: NO_LINES, + generated: NO_LINES + }) }) it('excludes a binary-only change, matching numstat reporting it as "-"', async () => { @@ -153,7 +198,13 @@ describe('branch line total against a real repository', () => { const result = await getStatus(repo, { branchLineTotalMergeBase: mergeBase }) expect(git(repo, ['diff', '--numstat', mergeBase, '--'])).toMatch(/^-\t-\t/) - expect(result.branchLineTotal).toEqual({ added: 0, removed: 0, mergeBase }) + expect(result.branchLineTotal).toEqual({ + added: 0, + removed: 0, + mergeBase, + test: NO_LINES, + generated: NO_LINES + }) }) it('contributes nothing for a pure rename', async () => { @@ -164,7 +215,33 @@ describe('branch line total against a real repository', () => { const result = await getStatus(repo, { branchLineTotalMergeBase: mergeBase }) - expect(result.branchLineTotal).toEqual({ added: 0, removed: 0, mergeBase }) + expect(result.branchLineTotal).toEqual({ + added: 0, + removed: 0, + mergeBase, + test: NO_LINES, + generated: NO_LINES + }) + }) + + it('splits test-path and generated files out of the published total', async () => { + const repo = await createFixtureRepo() + await write(repo, 'src/a.ts', 'a\n') + const mergeBase = commitAll(repo, 'base') + await write(repo, 'src/a.ts', 'a\nb\n') + await write(repo, 'src/a.test.ts', 't1\nt2\nt3\n') + await write(repo, 'pnpm-lock.yaml', 'l1\nl2\nl3\nl4\nl5\n') + + const result = await getStatus(repo, { branchLineTotalMergeBase: mergeBase }) + + expect(rangedDiffTotal(repo, mergeBase)).toEqual({ added: 1, removed: 0 }) + expect(result.branchLineTotal).toEqual({ + added: 9, + removed: 0, + mergeBase, + test: { added: 3, removed: 0 }, + generated: { added: 5, removed: 0 } + }) }) it('includes untracked additions alongside tracked range changes', async () => { @@ -178,7 +255,13 @@ describe('branch line total against a real repository', () => { const result = await getStatus(repo, { branchLineTotalMergeBase: mergeBase }) expect(rangedDiffTotal(repo, mergeBase)).toEqual({ added: 1, removed: 0 }) - expect(result.branchLineTotal).toEqual({ added: 5, removed: 0, mergeBase }) + expect(result.branchLineTotal).toEqual({ + added: 5, + removed: 0, + mergeBase, + test: NO_LINES, + generated: NO_LINES + }) }) // Documented deviation from a pure `git diff`: the untracked half is added on @@ -198,6 +281,12 @@ describe('branch line total against a real repository', () => { ['untracked', 'gone.txt'] ]) expect(rangedDiffTotal(repo, mergeBase)).toEqual({ added: 0, removed: 3 }) - expect(result.branchLineTotal).toEqual({ added: 2, removed: 3, mergeBase }) + expect(result.branchLineTotal).toEqual({ + added: 2, + removed: 3, + mergeBase, + test: NO_LINES, + generated: NO_LINES + }) }) }) diff --git a/src/main/git/status-branch-line-total-relay-parity.test.ts b/src/main/git/status-branch-line-total-relay-parity.test.ts index e9f524072..c62d419a2 100644 --- a/src/main/git/status-branch-line-total-relay-parity.test.ts +++ b/src/main/git/status-branch-line-total-relay-parity.test.ts @@ -26,7 +26,13 @@ const execFileAsync = promisify(execFile) // flip.txt 0 (added in the branch commit, removed again in the worktree) // moved.txt 0 (pure rename) // fresh.txt +3 (untracked) -const EXPECTED_TOTAL = { added: 6, removed: 0 } +// No fixture path here looks like test or generated code, so it is all source. +const EXPECTED_TOTAL = { + added: 6, + removed: 0, + test: { added: 0, removed: 0 }, + generated: { added: 0, removed: 0 } +} // Summing the per-area status rows instead would give this — the wrong answer // the shared module exists to avoid. const AREA_ROW_SUM = { added: 5, removed: 2 } diff --git a/src/relay/git-status-branch-line-total.test.ts b/src/relay/git-status-branch-line-total.test.ts index ceb04351e..ed20750d2 100644 --- a/src/relay/git-status-branch-line-total.test.ts +++ b/src/relay/git-status-branch-line-total.test.ts @@ -24,6 +24,8 @@ const OTHER_MERGE_BASE = 'fedcba9876543210fedcba9876543210fedcba98' // Untracked additions ride on top of the ranged diff, so the fixture needs a real file. const UNTRACKED_FILE = 'notes.md' const UNTRACKED_LINES = 4 +// No fixture path here looks like test or generated code, so it is all source. +const NO_LINES = { added: 0, removed: 0 } const STATUS_OUTPUT = [ '# branch.oid 1111111111111111111111111111111111111111', '# branch.head (detached)', @@ -166,7 +168,9 @@ describe('getStatusOp branch line total', () => { expect(result.branchLineTotal).toEqual({ added: 3 + UNTRACKED_LINES, removed: 2, - mergeBase + mergeBase, + test: NO_LINES, + generated: NO_LINES }) expect(rangedDiffCalls(git.mock.calls)).toEqual([ ['-c', 'core.quotePath=false', 'diff', '-z', '--numstat', '-M', mergeBase, '--'] @@ -352,7 +356,9 @@ describe('getStatusOp branch line total', () => { expect(first.branchLineTotal).toEqual({ added: 12 + UNTRACKED_LINES, removed: 5, - mergeBase: MERGE_BASE + mergeBase: MERGE_BASE, + test: NO_LINES, + generated: NO_LINES }) expect(reused.branchLineTotal).toEqual(first.branchLineTotal) expect(rangedDiffCalls(git.mock.calls)).toHaveLength(1) @@ -391,7 +397,9 @@ describe('getStatusOp branch line total', () => { expect(first.branchLineTotal).toEqual({ added: 12 + UNTRACKED_LINES, removed: 5, - mergeBase: MERGE_BASE + mergeBase: MERGE_BASE, + test: NO_LINES, + generated: NO_LINES }) }) @@ -411,7 +419,9 @@ describe('getStatusOp branch line total', () => { expect(moved.branchLineTotal).toEqual({ added: 12 + UNTRACKED_LINES, removed: 5, - mergeBase: OTHER_MERGE_BASE + mergeBase: OTHER_MERGE_BASE, + test: NO_LINES, + generated: NO_LINES }) expect(rangedDiffCalls(git.mock.calls)).toHaveLength(2) }) @@ -466,7 +476,9 @@ describe('getStatusOp branch line total', () => { expect(result.branchLineTotal).toEqual({ added: UNTRACKED_LINES, removed: 0, - mergeBase: MERGE_BASE + mergeBase: MERGE_BASE, + test: NO_LINES, + generated: NO_LINES }) expect(Number.isFinite(result.branchLineTotal?.added)).toBe(true) expect(Number.isFinite(result.branchLineTotal?.removed)).toBe(true) diff --git a/src/renderer/src/components/right-sidebar/SourceControl.branch-line-total.test.tsx b/src/renderer/src/components/right-sidebar/SourceControl.branch-line-total.test.tsx index c9a7f24be..9f2529973 100644 --- a/src/renderer/src/components/right-sidebar/SourceControl.branch-line-total.test.tsx +++ b/src/renderer/src/components/right-sidebar/SourceControl.branch-line-total.test.tsx @@ -207,6 +207,12 @@ function chip(): HTMLElement | null { return container.querySelector('[data-testid="source-control-branch-line-total"]') } +function loadingChip(): HTMLElement | null { + return container.querySelector( + '[data-testid="source-control-branch-line-total-loading"]' + ) +} + describe('SourceControl branch line total request gate', () => { it('asks for a total when the panel is visible and compare is ready', () => { renderSourceControl() @@ -277,14 +283,14 @@ describe('SourceControl branch line total chip', () => { }) renderSourceControl() - expect(chip()?.getAttribute('aria-label')).toBe('8259 additions, 670 deletions') + expect(chip()?.getAttribute('aria-label')).toBe('8259 lines added, 670 lines deleted') // Grouping is pinned to the app locale (`en`), not the runner's host locale. expect(chip()?.textContent).toBe('+8,259-670') }) it('drops a total whose fork point has since moved', () => { // Why: status and branch compare refresh on different cadences, so a total - // can outlive the merge base it measured. Hidden beats a stale number. + // can outlive the merge base it measured. Stale digits must not render. resetState({ gitBranchLineTotalByWorktree: { [mocks.activeWorktree.id]: { added: 8259, removed: 670, mergeBase: 'stale-merge-base' } @@ -293,6 +299,7 @@ describe('SourceControl branch line total chip', () => { renderSourceControl() expect(chip()).toBeNull() + expect(loadingChip()).toBeNull() }) it('drops a published total while branch compare has no ready summary', () => { @@ -307,12 +314,27 @@ describe('SourceControl branch line total chip', () => { renderSourceControl() expect(chip()).toBeNull() + expect(loadingChip()).toBeNull() }) - it('renders nothing when no total was published', () => { - renderSourceControl() + // Why: a pending total and one this host will never send look identical from + // here, so a pulsing placeholder would keep pulsing forever on an old host, + // after a hard failure, or during the ranged-diff cooldown. + it('shows no placeholder while the total is still pending', () => { + vi.useFakeTimers() + try { + renderSourceControl() - expect(chip()).toBeNull() + expect(chip()).toBeNull() + act(() => { + vi.advanceTimersByTime(20_000) + }) + expect(chip()).toBeNull() + expect(loadingChip()).toBeNull() + expect(container.innerHTML).not.toContain('animate-pulse') + } finally { + vi.useRealTimers() + } }) it('renders nothing for an exact zero total', () => { @@ -324,6 +346,7 @@ describe('SourceControl branch line total chip', () => { renderSourceControl() expect(chip()).toBeNull() + expect(loadingChip()).toBeNull() }) it('omits the zero half of a one-sided total', () => { @@ -335,6 +358,6 @@ describe('SourceControl branch line total chip', () => { renderSourceControl() expect(chip()?.textContent).toBe('+42') - expect(chip()?.getAttribute('aria-label')).toBe('42 additions') + expect(chip()?.getAttribute('aria-label')).toBe('42 lines added') }) }) diff --git a/src/renderer/src/components/right-sidebar/source-control-branch-context-row.test.tsx b/src/renderer/src/components/right-sidebar/source-control-branch-context-row.test.tsx index 963e4e00a..9ccad4f3a 100644 --- a/src/renderer/src/components/right-sidebar/source-control-branch-context-row.test.tsx +++ b/src/renderer/src/components/right-sidebar/source-control-branch-context-row.test.tsx @@ -11,6 +11,12 @@ vi.mock('@/components/ui/tooltip', () => ({ TooltipTrigger: ({ children }: { children: ReactNode }) => <>{children} })) +vi.mock('@/components/ui/hover-card', () => ({ + HoverCard: ({ children }: { children: ReactNode }) => <>{children}, + HoverCardContent: ({ children }: { children: ReactNode }) => <>{children}, + HoverCardTrigger: ({ children }: { children: ReactNode }) => <>{children} +})) + const readySummary: GitBranchCompareSummary = { baseRef: 'refs/remotes/origin/FRONT-192-ZisVoucherStrip', baseOid: 'base', @@ -241,7 +247,7 @@ describe('SourceControlBranchContextRow branch line total', () => { expect(markup).toContain('+8,259') expect(markup).toContain('-670') // Label reads raw digits; the grouped spans are decoration. - expect(markup).toContain('aria-label="8259 additions, 670 deletions"') + expect(markup).toContain('aria-label="8259 lines added, 670 lines deleted"') expect(markup).toContain('tabular-nums') expect(markup).toContain('text-[color:var(--git-decoration-added)]') expect(markup).toContain('text-[color:var(--git-decoration-deleted)]') @@ -286,12 +292,12 @@ describe('SourceControlBranchContextRow branch line total', () => { const addedOnly = renderWithLineTotal({ added: 42, removed: 0, mergeBase: 'base' }) expect(addedOnly).toContain('>+42<') expect(addedOnly).not.toContain('>-0<') - expect(addedOnly).toContain('aria-label="42 additions"') + expect(addedOnly).toContain('aria-label="42 lines added"') const removedOnly = renderWithLineTotal({ added: 0, removed: 7, mergeBase: 'base' }) expect(removedOnly).toContain('>-7<') expect(removedOnly).not.toContain('>+0<') - expect(removedOnly).toContain('aria-label="7 deletions"') + expect(removedOnly).toContain('aria-label="7 lines deleted"') }) it('hides the chip when both counts are zero', () => { @@ -302,10 +308,13 @@ describe('SourceControlBranchContextRow branch line total', () => { expect(markup).not.toContain('>-0<') }) - it('hides the chip when the total is absent', () => { + it('renders nothing at all when the total is absent', () => { + // Why: a null total is equally "still computing" and "this host will never + // send one", so there is no placeholder to show — the slot stays empty. for (const total of [null, undefined]) { const markup = renderWithLineTotal(total) expect(markup).not.toContain('data-testid="source-control-branch-line-total"') + expect(markup).not.toContain('animate-pulse') expect(markup).not.toContain('NaN') } }) @@ -382,3 +391,156 @@ describe('SourceControlBranchContextRow branch line total', () => { } }) }) + +// The hover-card mock at the top renders content inline, so these assert the +// breakdown copy without driving a real hover. +describe('SourceControlBranchContextRow line total test split', () => { + it('breaks the panel down into test and non-test halves', () => { + const markup = renderWithLineTotal({ + added: 243, + removed: 149, + mergeBase: 'base', + test: { added: 120, removed: 40 } + }) + + expect(markup).toContain('Code breakdown') + expect(markup).toContain('Lines of code') + expect(markup).toContain('data-testid="source-control-branch-line-total-breakdown"') + expect(markup).toContain('Non-test') + expect(markup).toContain('>+123<') + expect(markup).toContain('>-109<') + expect(markup).toContain('Tests') + expect(markup).toContain('>+120<') + expect(markup).toContain('>-40<') + // Non-test first so the primary share leads the panel. + expect(markup.indexOf('Non-test')).toBeLessThan(markup.indexOf('Tests')) + }) + + it('spells the split into the label so the summary is announced closed', () => { + const markup = renderWithLineTotal({ + added: 243, + removed: 149, + mergeBase: 'base', + test: { added: 120, removed: 40 } + }) + + expect(markup).toContain( + 'aria-label="243 lines added, 149 lines deleted — test code: 120 lines added, 40 lines deleted"' + ) + }) + + it('keeps a zero test share visible rather than hiding the row', () => { + const markup = renderWithLineTotal({ + added: 243, + removed: 149, + mergeBase: 'base', + test: { added: 0, removed: 0 } + }) + + expect(markup).toContain('>+0<') + expect(markup).toContain('>-0<') + expect(markup).toContain('>+243<') + expect(markup).toContain('>-149<') + // Panel keeps the zero row; the spoken label stays the main totals only. + expect(markup).toContain('aria-label="243 lines added, 149 lines deleted"') + expect(markup).not.toContain('test code:') + }) + + // A host predating the split omits the field; inventing a 0% test share there + // would be a confidently wrong claim. + it('renders exactly as before when the host reported no split', () => { + const withoutSplit = renderWithLineTotal({ added: 243, removed: 149, mergeBase: 'base' }) + + expect(withoutSplit).not.toContain('Non-test') + expect(withoutSplit).not.toContain('Code breakdown') + expect(withoutSplit).toContain('aria-label="243 lines added, 149 lines deleted"') + }) + + it('labels the remainder Source once generated is known, and shows that row', () => { + const markup = renderWithLineTotal({ + added: 243, + removed: 149, + mergeBase: 'base', + test: { added: 20, removed: 10 }, + generated: { added: 100, removed: 50 } + }) + + expect(markup).toContain('Source') + expect(markup).not.toContain('Non-test') + expect(markup).toContain('Generated') + expect(markup).toContain('>+123<') // 243-20-100 + expect(markup).toContain('>-89<') // 149-10-50 + expect(markup).toContain('>+100<') + expect(markup).toContain('>-50<') + expect(markup).toContain( + 'aria-label="243 lines added, 149 lines deleted — test code: 20 lines added, 10 lines deleted — generated: 100 lines added, 50 lines deleted"' + ) + }) + + // The remainder here still contains tests, so calling it "Source" would claim + // a split the host never sent. + it('labels the remainder Non-generated when the host omitted the test field', () => { + const markup = renderWithLineTotal({ + added: 243, + removed: 149, + mergeBase: 'base', + generated: { added: 100, removed: 50 } + }) + + expect(markup).toContain('Non-generated') + expect(markup).toContain('>Generated<') + expect(markup).not.toContain('>Source<') + expect(markup).not.toContain('Tests') + expect(markup).toContain('>+143<') // 243-100 + expect(markup).toContain('>-99<') // 149-50 + }) + + // "No tests in this branch" is worth stating; "nothing was generated" is the + // normal case, so that row is dropped rather than shown as +0 -0. + it('drops the generated row and its announcement when nothing was generated', () => { + const markup = renderWithLineTotal({ + added: 243, + removed: 149, + mergeBase: 'base', + test: { added: 120, removed: 40 }, + generated: { added: 0, removed: 0 } + }) + + expect(markup).not.toContain('Generated') + expect(markup).not.toContain('generated:') + // The host did classify and found nothing, so the remainder is still Source. + expect(markup).toContain('Source') + expect(markup).toContain('Tests') + expect(markup).toContain('>+123<') + }) + + it('orders the rows source, tests, generated', () => { + const markup = renderWithLineTotal({ + added: 1243, + removed: 149, + mergeBase: 'base', + test: { added: 120, removed: 40 }, + generated: { added: 1000, removed: 0 } + }) + + expect(markup.indexOf('Source')).toBeLessThan(markup.indexOf('Tests')) + expect(markup.indexOf('Tests')).toBeLessThan(markup.indexOf('Generated')) + expect(markup).toContain('>+1,000<') + }) + + // The three rows must account for every line, or the panel contradicts the chip. + it('keeps the three rows summing back to the chip total', () => { + const markup = renderWithLineTotal({ + added: 500, + removed: 200, + mergeBase: 'base', + test: { added: 150, removed: 60 }, + generated: { added: 300, removed: 100 } + }) + + expect(markup).toContain('>+50<') // 500-150-300 + expect(markup).toContain('>-40<') // 200-60-100 + expect(markup).toContain('>+150<') + expect(markup).toContain('>+300<') + }) +}) diff --git a/src/renderer/src/components/right-sidebar/source-control-branch-line-total-chip.tsx b/src/renderer/src/components/right-sidebar/source-control-branch-line-total-chip.tsx index d7256ac0e..6db2d9aac 100644 --- a/src/renderer/src/components/right-sidebar/source-control-branch-line-total-chip.tsx +++ b/src/renderer/src/components/right-sidebar/source-control-branch-line-total-chip.tsx @@ -1,5 +1,6 @@ import React, { useMemo } from 'react' import type { GitBranchLineTotal } from '../../../../shared/git-status-types' +import { HoverCard, HoverCardContent, HoverCardTrigger } from '@/components/ui/hover-card' import { getIntlLocale, translate } from '@/i18n/i18n' // Why: raw digits, not the grouped display string — screen readers announce @@ -8,26 +9,106 @@ function buildAccessibleLabel(added: number, removed: number): string { if (added > 0 && removed > 0) { return translate( 'auto.components.right.sidebar.source.control.branch.line.total.chip.daa8e8e59b', - '{{value0}} additions, {{value1}} deletions', + '{{value0}} lines added, {{value1}} lines deleted', { value0: added, value1: removed } ) } if (added > 0) { return translate( 'auto.components.right.sidebar.source.control.branch.line.total.chip.8a9b97b666', - '{{value0}} additions', + '{{value0}} lines added', { value0: added } ) } return translate( 'auto.components.right.sidebar.source.control.branch.line.total.chip.52c366d88d', - '{{value0}} deletions', + '{{value0}} lines deleted', { value0: removed } ) } -// Absent, incomplete and genuinely-empty all render as nothing: no `+0 -0`, no -// spinner, no reserved width. Not clickable — `openBranchAllDiffs` is narrower. +// The chip takes no focus; the split still reaches assistive tech through the +// label rather than the hover-only panel. +function appendTestSplitToLabel(label: string, testAdded: number, testRemoved: number): string { + return translate( + 'auto.components.right.sidebar.source.control.branch.line.total.chip.4c1f70ba92', + '{{value0}} — test code: {{value1}} lines added, {{value2}} lines deleted', + { value0: label, value1: testAdded, value2: testRemoved } + ) +} + +function appendGeneratedSplitToLabel(label: string, added: number, removed: number): string { + return translate( + 'auto.components.right.sidebar.source.control.branch.line.total.chip.7f3e1a9c24', + '{{value0}} — generated: {{value1}} lines added, {{value2}} lines deleted', + { value0: label, value1: added, value2: removed } + ) +} + +type LineTotalSplitRow = { key: string; label: string; added: number; removed: number } + +function LineCountPair({ + added, + removed, + locale +}: { + added: number + removed: number + locale: string +}): React.JSX.Element { + return ( + + + +{added.toLocaleString(locale)} + + + -{removed.toLocaleString(locale)} + + + ) +} + +function CodeBreakdownPanel({ + rows, + locale +}: { + rows: LineTotalSplitRow[] + locale: string +}): React.JSX.Element { + return ( +
+
+
+ {translate( + 'auto.components.right.sidebar.source.control.branch.line.total.chip.a1b2c3d4e5', + 'Code breakdown' + )} +
+
+ {translate( + 'auto.components.right.sidebar.source.control.branch.line.total.chip.b2c3d4e5f6', + 'Lines of code' + )} +
+
+
+ {rows.map((row) => ( +
+ {row.label} + +
+ ))} +
+
+ ) +} + +// Genuinely-empty (`+0 -0`), not-yet-published and unknown-after-timeout all +// render as nothing: no reserved width, no placeholder. There is no loading +// state because a null total is indistinguishable from permanent absence (old +// host, failed diff, cooldown), and the store now keeps the last published +// total across a soft miss so the slot rarely empties once filled. +// Not clickable — `openBranchAllDiffs` is narrower. export const SourceControlBranchLineTotalChip = React.memo( function SourceControlBranchLineTotalChip({ branchLineTotal @@ -38,12 +119,93 @@ export const SourceControlBranchLineTotalChip = React.memo( const removed = branchLineTotal?.removed ?? 0 const hasAdded = added > 0 const hasRemoved = removed > 0 + // Why: hosts predating either split omit the field; inventing a zero share + // there would be confidently wrong, so the breakdown is dropped per field. + const testTotal = branchLineTotal?.test + const testAdded = testTotal?.added + const testRemoved = testTotal?.removed + const generatedTotal = branchLineTotal?.generated + const generatedAdded = generatedTotal?.added + const generatedRemoved = generatedTotal?.removed // Full precision and app-locale-aware; a status tick that leaves the counts // alone must not rebuild these strings. const locale = getIntlLocale() const addedText = useMemo(() => added.toLocaleString(locale), [added, locale]) const removedText = useMemo(() => removed.toLocaleString(locale), [removed, locale]) - const accessibleLabel = useMemo(() => buildAccessibleLabel(added, removed), [added, removed]) + const accessibleLabel = useMemo(() => { + let label = buildAccessibleLabel(added, removed) + // Zero test share still shows in the hover panel; don't announce noise. + if (testAdded != null && testRemoved != null && (testAdded > 0 || testRemoved > 0)) { + label = appendTestSplitToLabel(label, testAdded, testRemoved) + } + if ( + generatedAdded != null && + generatedRemoved != null && + (generatedAdded > 0 || generatedRemoved > 0) + ) { + label = appendGeneratedSplitToLabel(label, generatedAdded, generatedRemoved) + } + return label + }, [added, removed, testAdded, testRemoved, generatedAdded, generatedRemoved]) + const splitRows = useMemo(() => { + // Why: the fields ship together today but the type is independent so a + // host can gain one before the other; either alone is enough to render. + const hasTest = testAdded != null && testRemoved != null + const hasGenerated = generatedAdded != null && generatedRemoved != null + if (!hasTest && !hasGenerated) { + return [] + } + + const rows: LineTotalSplitRow[] = [ + { + key: 'source', + // Only "Source" once both splits are known — with one missing the + // remainder still contains the other bucket, so name what was taken out. + label: + hasTest && hasGenerated + ? translate( + 'auto.components.right.sidebar.source.control.branch.line.total.chip.c8d5b21e07', + 'Source' + ) + : hasTest + ? translate( + 'auto.components.right.sidebar.source.control.branch.line.total.chip.9e4a3c5081', + 'Non-test' + ) + : translate( + 'auto.components.right.sidebar.source.control.branch.line.total.chip.3d7e9b1042', + 'Non-generated' + ), + added: added - (testAdded ?? 0) - (generatedAdded ?? 0), + removed: removed - (testRemoved ?? 0) - (generatedRemoved ?? 0) + } + ] + if (hasTest) { + rows.push({ + key: 'test', + label: translate( + 'auto.components.right.sidebar.source.control.branch.line.total.chip.6b2d0f14a7', + 'Tests' + ), + added: testAdded, + removed: testRemoved + }) + } + // Why: "no tests in this branch" is worth showing as +0 -0; "nothing was + // generated" is the normal case, so that row is dropped instead. + if (hasGenerated && (generatedAdded > 0 || generatedRemoved > 0)) { + rows.push({ + key: 'generated', + label: translate( + 'auto.components.right.sidebar.source.control.branch.line.total.chip.7a04c6f8b3', + 'Generated' + ), + added: generatedAdded, + removed: generatedRemoved + }) + } + return rows + }, [added, removed, testAdded, testRemoved, generatedAdded, generatedRemoved]) if (!hasAdded && !hasRemoved) { return null @@ -51,12 +213,18 @@ export const SourceControlBranchLineTotalChip = React.memo( // Why: no fixed `ch` width — that clips at 5+ digits; `tabular-nums` alone // keeps digits from jittering between refreshes. - return ( + // `cursor-help` signals hover detail without implying a click target. + const hasBreakdown = splitRows.length > 0 + const chip = ( {hasAdded ? ( ) + + if (!hasBreakdown) { + return chip + } + + return ( + + {chip} + + + + + ) } ) diff --git a/src/renderer/src/components/right-sidebar/source-control-header-toolbar-identity.test.tsx b/src/renderer/src/components/right-sidebar/source-control-header-toolbar-identity.test.tsx index 4147569b7..fac0f0b6d 100644 --- a/src/renderer/src/components/right-sidebar/source-control-header-toolbar-identity.test.tsx +++ b/src/renderer/src/components/right-sidebar/source-control-header-toolbar-identity.test.tsx @@ -132,7 +132,7 @@ describe('SourceControlHeaderToolbar branch identity', () => { it('threads the branch line total down to the base-ref line', () => { const markup = renderToolbar({ branchLineTotal: { added: 24, removed: 3, mergeBase: 'base' } }) - expect(markup).toContain('aria-label="24 additions, 3 deletions"') + expect(markup).toContain('aria-label="24 lines added, 3 lines deleted"') expect(markup).toContain('+24') expect(markup).toContain('-3') }) diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 962bf2dbe..568e139eb 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -11359,9 +11359,18 @@ "line": { "total": { "chip": { - "daa8e8e59b": "{{value0}} additions, {{value1}} deletions", - "8a9b97b666": "{{value0}} additions", - "52c366d88d": "{{value0}} deletions" + "daa8e8e59b": "{{value0}} lines added, {{value1}} lines deleted", + "8a9b97b666": "{{value0}} lines added", + "52c366d88d": "{{value0}} lines deleted", + "4c1f70ba92": "{{value0}} — test code: {{value1}} lines added, {{value2}} lines deleted", + "6b2d0f14a7": "Tests", + "9e4a3c5081": "Non-test", + "3d7e9b1042": "Non-generated", + "a1b2c3d4e5": "Code breakdown", + "b2c3d4e5f6": "Lines of code", + "7f3e1a9c24": "{{value0}} — generated: {{value1}} lines added, {{value2}} lines deleted", + "c8d5b21e07": "Source", + "7a04c6f8b3": "Generated" } } } diff --git a/src/renderer/src/store/slices/editor-branch-line-total.test.ts b/src/renderer/src/store/slices/editor-branch-line-total.test.ts index c4f885409..2b7132085 100644 --- a/src/renderer/src/store/slices/editor-branch-line-total.test.ts +++ b/src/renderer/src/store/slices/editor-branch-line-total.test.ts @@ -69,7 +69,7 @@ describe('createEditorSlice branch line total', () => { }) }) - it('clears a stored total when a later status omits the field', () => { + it('keeps the published total when a later status omits the field', () => { const store = createEditorStore() store .getState() @@ -77,12 +77,25 @@ describe('createEditorSlice branch line total', () => { 'wt-1', status({ branchLineTotal: { added: 24, removed: 3, mergeBase: MERGE_BASE } }) ) + const published = store.getState().gitBranchLineTotalByWorktree['wt-1'] - // Why: an omitted field means "not known exact" — an old server, a failed - // numstat, or a timeout. Keeping the old number renders a confident lie. + // Why: an omitted field is "not computed on this pass" — a soft-deadline + // miss or a cooldown — and the host keeps backfilling its cache. Clearing + // it blanked the chip mid-poll and then flashed it back. store.getState().setGitStatus('wt-1', status()) - expect(store.getState().gitBranchLineTotalByWorktree['wt-1']).toBeUndefined() + expect(store.getState().gitBranchLineTotalByWorktree['wt-1']).toBe(published) + }) + + it('does not produce a new state object when a status merely omits the total', () => { + const store = createEditorStore() + const tick = status({ branchLineTotal: { added: 24, removed: 3, mergeBase: MERGE_BASE } }) + store.getState().setGitStatus('wt-1', tick) + const before = store.getState() + + store.getState().setGitStatus('wt-1', { ...tick, branchLineTotal: undefined }) + + expect(store.getState()).toBe(before) }) it('clears a stored total when the listing hits the entry cap', () => { @@ -132,7 +145,7 @@ describe('createEditorSlice branch line total', () => { status({ branchLineTotal: { added: 1, removed: 1, mergeBase: 'other' } }) ) - store.getState().setGitStatus('wt-1', status()) + store.getState().setGitStatus('wt-1', status({ didHitLimit: true, statusLength: 2 })) expect(store.getState().gitBranchLineTotalByWorktree['wt-1']).toBeUndefined() expect(store.getState().gitBranchLineTotalByWorktree['wt-2']).toEqual({ @@ -203,6 +216,69 @@ describe('createEditorSlice branch line total', () => { }) }) + // Moving a line from a source file into a test file leaves the totals + // identical; without the split in the equality check the hover would keep + // showing the old share. + it('produces a new state object when only the test split changed', () => { + const store = createEditorStore() + const tick = status({ + branchLineTotal: { + added: 24, + removed: 3, + mergeBase: MERGE_BASE, + test: { added: 4, removed: 0 } + } + }) + store.getState().setGitStatus('wt-1', tick) + const before = store.getState() + + store.getState().setGitStatus('wt-1', { + ...tick, + branchLineTotal: { + added: 24, + removed: 3, + mergeBase: MERGE_BASE, + test: { added: 5, removed: 1 } + } + }) + + expect(store.getState()).not.toBe(before) + expect(store.getState().gitBranchLineTotalByWorktree['wt-1']?.test).toEqual({ + added: 5, + removed: 1 + }) + }) + + it('produces a new state object when only the generated split changed', () => { + const store = createEditorStore() + const tick = status({ + branchLineTotal: { + added: 24, + removed: 3, + mergeBase: MERGE_BASE, + generated: { added: 8, removed: 0 } + } + }) + store.getState().setGitStatus('wt-1', tick) + const before = store.getState() + + store.getState().setGitStatus('wt-1', { + ...tick, + branchLineTotal: { + added: 24, + removed: 3, + mergeBase: MERGE_BASE, + generated: { added: 12, removed: 1 } + } + }) + + expect(store.getState()).not.toBe(before) + expect(store.getState().gitBranchLineTotalByWorktree['wt-1']?.generated).toEqual({ + added: 12, + removed: 1 + }) + }) + it('leaves other worktree entries referentially stable when one total changes', () => { const store = createEditorStore() store diff --git a/src/renderer/src/store/slices/editor.ts b/src/renderer/src/store/slices/editor.ts index e3123db9c..971ab20cb 100644 --- a/src/renderer/src/store/slices/editor.ts +++ b/src/renderer/src/store/slices/editor.ts @@ -4193,15 +4193,29 @@ export const createEditorSlice: StateCreator = (s const hugeUnchanged = (prevHuge?.limit ?? null) === (nextHuge?.limit ?? null) const prevBranchLineTotal = s.gitBranchLineTotalByWorktree[worktreeId] ?? null - // Why: an omitted field means "not known exact"; keeping the old total would render a confidently wrong chip. - const nextBranchLineTotal = status.branchLineTotal ?? null + // Why: an omitted field means "not computed on this pass" — soft-deadline + // miss, cooldown, old host — not "zero", so dropping it blanks a published + // chip between polls. Staleness is handled where it can be: the host + // carries its cache forward, and SourceControl hides any total whose + // mergeBase no longer matches the fork point. A capped listing skips the + // ranged diff outright, so nothing will refresh it — clear it there. + const nextBranchLineTotal = status.didHitLimit + ? null + : (status.branchLineTotal ?? prevBranchLineTotal) const branchLineTotalUnchanged = prevBranchLineTotal === nextBranchLineTotal || (prevBranchLineTotal !== null && nextBranchLineTotal !== null && prevBranchLineTotal.added === nextBranchLineTotal.added && prevBranchLineTotal.removed === nextBranchLineTotal.removed && - prevBranchLineTotal.mergeBase === nextBranchLineTotal.mergeBase) + prevBranchLineTotal.mergeBase === nextBranchLineTotal.mergeBase && + (prevBranchLineTotal.test?.added ?? null) === (nextBranchLineTotal.test?.added ?? null) && + (prevBranchLineTotal.test?.removed ?? null) === + (nextBranchLineTotal.test?.removed ?? null) && + (prevBranchLineTotal.generated?.added ?? null) === + (nextBranchLineTotal.generated?.added ?? null) && + (prevBranchLineTotal.generated?.removed ?? null) === + (nextBranchLineTotal.generated?.removed ?? null)) const prevStatusHead = s.gitStatusHeadByWorktree[worktreeId] const nextStatusHead = getKnownGitHead(status.head) diff --git a/src/shared/generated-code-path.test.ts b/src/shared/generated-code-path.test.ts new file mode 100644 index 000000000..6af7617cf --- /dev/null +++ b/src/shared/generated-code-path.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it } from 'vitest' +import { isGeneratedCodePath } from './generated-code-path' + +describe('isGeneratedCodePath', () => { + it('recognizes dependency lockfiles across ecosystems', () => { + for (const filePath of [ + 'package-lock.json', + 'pnpm-lock.yaml', + 'yarn.lock', + 'bun.lockb', + 'Cargo.lock', + 'poetry.lock', + 'uv.lock', + 'Gemfile.lock', + 'composer.lock', + 'go.sum', + 'flake.lock', + 'mobile/pubspec.lock', + 'services/api/Pipfile.lock' + ]) { + expect(isGeneratedCodePath(filePath), filePath).toBe(true) + } + }) + + it('recognizes tool-stamped filename suffixes', () => { + for (const filePath of [ + 'api/service.pb.go', + // grpc-gateway / protoc-gen-validate stack extra segments onto `.pb.` + 'api/service.pb.gw.go', + 'api/service.pb.validate.go', + 'api/service_pb2.py', + 'api/service_pb2_grpc.py', + 'src/schema.gen.ts', + 'internal/bindings_generated.go', + 'src/Api.generated.ts', + 'Forms/Main.Designer.cs', + 'lib/model.g.dart', + 'lib/model.freezed.dart', + 'public/app.min.js', + 'public/app.js.map', + 'src/components/__snapshots__/Chip.tsx.snap' + ]) { + expect(isGeneratedCodePath(filePath), filePath).toBe(true) + } + }) + + it('recognizes generated directories on either separator', () => { + expect(isGeneratedCodePath('dist/renderer/index.js')).toBe(true) + expect(isGeneratedCodePath('src/__generated__/schema.ts')).toBe(true) + expect(isGeneratedCodePath('vendor/github.com/pkg/errors/errors.go')).toBe(true) + expect(isGeneratedCodePath('src\\__pycache__\\mod.pyc')).toBe(true) + }) + + it('leaves hand-written paths alone, including the ambiguous directory names', () => { + for (const filePath of [ + 'src/shared/git-branch-line-total.ts', + // Excluded on purpose: all common as authored source directories. + 'build/scripts/release.ts', + 'target/tracker.rs', + 'src/out/renderer.ts', + 'cmd/bin/main.go', + // Substrings, not whole segments or suffixes. + 'src/distributed/queue.ts', + 'src/vendors/stripe.ts', + 'src/generator/emit.ts', + 'docs/generated-code-policy.md', + 'src/shared/generated-code-path.ts' + ]) { + expect(isGeneratedCodePath(filePath), filePath).toBe(false) + } + }) +}) diff --git a/src/shared/generated-code-path.ts b/src/shared/generated-code-path.ts new file mode 100644 index 000000000..b15dd0204 --- /dev/null +++ b/src/shared/generated-code-path.ts @@ -0,0 +1,50 @@ +/** + * Path-only heuristic for "was this file written by a tool?", used to carve + * machine-authored lines out of the branch total. A regenerated lockfile or + * protobuf stub can dwarf every hand-written line in a branch, which is what + * makes a bare `+8,259` misleading. + * + * Conservative on purpose: a false positive understates the real work, so only + * names that are unambiguous across ecosystems are listed. Matched against the + * raw path with anchored regexes, for the reasons in `test-code-path.ts`. + */ + +// Deliberately excludes `build`, `target`, `out` and `bin` — all common as +// hand-written source directories. +const GENERATED_DIRECTORY = + /(?:^|[/\\])(?:__generated__|__pycache__|\.next|\.nuxt|coverage|dist|generated|node_modules|vendor)[/\\]/i + +// Every dependency lockfile is regenerated wholesale, so its diff is churn +// rather than authored change. Kept separate from the suffix alternation below +// rather than merged into it: measured over 12.5k paths, two narrow anchored +// regexes reject a non-match faster than one wide alternation. +const LOCKFILE = + /(?:^|[/\\])(?:bun\.lockb?|cargo\.lock|composer\.lock|flake\.lock|gemfile\.lock|go\.sum|gradle\.lockfile|mix\.lock|npm-shrinkwrap\.json|package-lock\.json|packages\.lock\.json|pipfile\.lock|pnpm-lock\.yaml|poetry\.lock|pubspec\.lock|uv\.lock|yarn\.lock)$/i + +// The suffixes tools stamp onto their own output. +const GENERATED_BASENAME = new RegExp( + `(?:${[ + /\.(?:generated|designer)\.[^./\\]+/, // Foo.generated.ts, Form.Designer.cs + // Trailing segments are optional so plugin output keeps matching: + // service.pb.go, service_pb2.py, service.pb.gw.go, service.pb.validate.go. + /[._](?:pb|pb2|pb2_grpc)\.(?:[^./\\]+\.)*[^./\\]+/, + /[._]gen\.[^./\\]+/, // schema.gen.ts, mock_gen.go + /_generated\.[^./\\]+/, // bindings_generated.go + /\.(?:g|freezed)\.dart/, // model.g.dart + /\.min\.(?:js|css|mjs)/, + /\.(?:js|css|mjs)\.map/, // source maps + /\.snap/ // vitest/jest snapshots, rewritten by `-u` + ] + .map((pattern) => pattern.source) + .join('|')})$`, + 'i' +) + +/** Accepts POSIX and Windows separators — git reports `/`, callers may not. */ +export function isGeneratedCodePath(filePath: string): boolean { + return ( + GENERATED_BASENAME.test(filePath) || + LOCKFILE.test(filePath) || + GENERATED_DIRECTORY.test(filePath) + ) +} diff --git a/src/shared/git-branch-line-total.test.ts b/src/shared/git-branch-line-total.test.ts index 054c4fc68..1716da811 100644 --- a/src/shared/git-branch-line-total.test.ts +++ b/src/shared/git-branch-line-total.test.ts @@ -15,6 +15,7 @@ import { import type { GitLineStats } from './git-uncommitted-line-stats' const MERGE_BASE = 'a1b2c3d4e5f60718293a4b5c6d7e8f90a1b2c3d4' +const NO_LINES = { added: 0, removed: 0 } const OTHER_MERGE_BASE = '0f1e2d3c4b5a69788796a5b4c3d2e1f00f1e2d3c' function statsMap(entries: Record): ReadonlyMap { @@ -58,7 +59,7 @@ describe('sumGitBranchLineTotal', () => { }), untracked: statsMap({}) }) - ).toEqual({ added: 4, removed: 1, mergeBase: MERGE_BASE }) + ).toEqual({ added: 4, removed: 1, mergeBase: MERGE_BASE, test: NO_LINES, generated: NO_LINES }) }) it('counts a half-binary entry on the side git did report', () => { @@ -68,7 +69,7 @@ describe('sumGitBranchLineTotal', () => { tracked: statsMap({ 'odd.txt': { added: 3 } }), untracked: statsMap({ 'new.bin': {} }) }) - ).toEqual({ added: 3, removed: 0, mergeBase: MERGE_BASE }) + ).toEqual({ added: 3, removed: 0, mergeBase: MERGE_BASE, test: NO_LINES, generated: NO_LINES }) }) it('adds untracked additions on top of the tracked range and echoes the merge base', () => { @@ -78,7 +79,7 @@ describe('sumGitBranchLineTotal', () => { tracked: statsMap({ 'src/a.ts': { added: 10, removed: 2 } }), untracked: statsMap({ 'src/new.ts': { added: 7, removed: 0 }, 'src/also.ts': { added: 1 } }) }) - ).toEqual({ added: 18, removed: 2, mergeBase: MERGE_BASE }) + ).toEqual({ added: 18, removed: 2, mergeBase: MERGE_BASE, test: NO_LINES, generated: NO_LINES }) }) it('returns an all-zero total for a pure rename rather than omitting it', () => { @@ -88,7 +89,71 @@ describe('sumGitBranchLineTotal', () => { tracked: statsMap({ 'g.txt': { added: 0, removed: 0 } }), untracked: statsMap({}) }) - ).toEqual({ added: 0, removed: 0, mergeBase: MERGE_BASE }) + ).toEqual({ added: 0, removed: 0, mergeBase: MERGE_BASE, test: NO_LINES, generated: NO_LINES }) + }) + + it('splits out the test-file share of both halves, untracked files included', () => { + expect( + sumGitBranchLineTotal({ + mergeBase: MERGE_BASE, + tracked: statsMap({ + 'src/a.ts': { added: 10, removed: 2 }, + 'src/a.test.ts': { added: 30, removed: 5 }, + 'e2e/login.ts': { added: 4, removed: 1 } + }), + untracked: statsMap({ 'src/b.spec.ts': { added: 6 }, 'src/b.ts': { added: 2 } }) + }) + ).toEqual({ + added: 52, + removed: 8, + mergeBase: MERGE_BASE, + test: { added: 40, removed: 6 }, + generated: NO_LINES + }) + }) + + it('splits generated lines out of both the total and the test share', () => { + expect( + sumGitBranchLineTotal({ + mergeBase: MERGE_BASE, + tracked: statsMap({ + 'src/a.ts': { added: 10, removed: 2 }, + 'src/a.test.ts': { added: 30, removed: 5 }, + 'pnpm-lock.yaml': { added: 900, removed: 400 }, + 'api/service.pb.go': { added: 120, removed: 0 }, + // Generated wins the overlap: a snapshot is both, and counts as churn. + 'src/__snapshots__/a.tsx.snap': { added: 50, removed: 20 } + }), + untracked: statsMap({ 'dist/bundle.js': { added: 5000 } }) + }) + ).toEqual({ + added: 6110, + removed: 427, + mergeBase: MERGE_BASE, + test: { added: 30, removed: 5 }, + generated: { added: 6070, removed: 420 } + }) + }) + + it('keeps the three buckets disjoint so they sum back to the total', () => { + const total = sumGitBranchLineTotal({ + mergeBase: MERGE_BASE, + tracked: statsMap({ + 'src/a.ts': { added: 10, removed: 2 }, + 'src/a.test.ts': { added: 30, removed: 5 }, + 'go.sum': { added: 7, removed: 3 }, + 'tests/fixtures/repo.json': { added: 4, removed: 1 } + }), + untracked: statsMap({ 'src/b.ts': { added: 2 } }) + }) + + const source = { + added: total.added - total.test!.added - total.generated!.added, + removed: total.removed - total.test!.removed - total.generated!.removed + } + expect(source).toEqual({ added: 12, removed: 2 }) + expect(total.test).toEqual({ added: 34, removed: 6 }) + expect(total.generated).toEqual({ added: 7, removed: 3 }) }) }) @@ -178,7 +243,13 @@ describe('computeGitBranchLineTotal', () => { untrackedPaths: ['new.txt'], runDiffNumstat: async () => '4\t1\tsrc/a.ts\0' }) - ).resolves.toEqual({ added: 7, removed: 1, mergeBase: MERGE_BASE }) + ).resolves.toEqual({ + added: 7, + removed: 1, + mergeBase: MERGE_BASE, + test: NO_LINES, + generated: NO_LINES + }) }) it('omits the total when the ranged numstat fails instead of publishing untracked-only zeros', async () => { @@ -218,8 +289,8 @@ describe('computeGitBranchLineTotal', () => { release() expect(await both).toEqual([ - { added: 4, removed: 1, mergeBase: MERGE_BASE }, - { added: 4, removed: 1, mergeBase: MERGE_BASE } + { added: 4, removed: 1, mergeBase: MERGE_BASE, test: NO_LINES, generated: NO_LINES }, + { added: 4, removed: 1, mergeBase: MERGE_BASE, test: NO_LINES, generated: NO_LINES } ]) expect(runDiffNumstat).toHaveBeenCalledTimes(1) }) @@ -365,7 +436,13 @@ describe('computeGitBranchLineTotal ranged-diff cooldown', () => { } } - const TOTAL = { added: 10, removed: 2, mergeBase: MERGE_BASE } + const TOTAL = { + added: 10, + removed: 2, + mergeBase: MERGE_BASE, + test: NO_LINES, + generated: NO_LINES + } beforeEach(() => { nowMs = 0 diff --git a/src/shared/git-branch-line-total.ts b/src/shared/git-branch-line-total.ts index 270b28704..cfd841a46 100644 --- a/src/shared/git-branch-line-total.ts +++ b/src/shared/git-branch-line-total.ts @@ -5,6 +5,8 @@ import { parseNumstat, type GitLineStats } from './git-uncommitted-line-stats' +import { isGeneratedCodePath } from './generated-code-path' +import { isTestCodePath } from './test-code-path' export type { GitBranchLineTotal } @@ -87,17 +89,37 @@ export function sumGitBranchLineTotal(input: { }): GitBranchLineTotal { let added = 0 let removed = 0 - for (const stats of input.tracked.values()) { - // Binary files parse to undefined in numstat and contribute nothing, matching - // the per-file rows. - added += stats.added ?? 0 - removed += stats.removed ?? 0 + let testAdded = 0 + let testRemoved = 0 + let generatedAdded = 0 + let generatedRemoved = 0 + for (const source of [input.tracked, input.untracked]) { + for (const [filePath, stats] of source) { + // Binary files parse to undefined in numstat and contribute nothing, matching + // the per-file rows. + const fileAdded = stats.added ?? 0 + const fileRemoved = stats.removed ?? 0 + added += fileAdded + removed += fileRemoved + // Generated wins the overlap (a snapshot is both): the point of the bucket + // is to separate authored lines from churn, and a regenerated file is churn + // wherever it lives. + if (isGeneratedCodePath(filePath)) { + generatedAdded += fileAdded + generatedRemoved += fileRemoved + } else if (isTestCodePath(filePath)) { + testAdded += fileAdded + testRemoved += fileRemoved + } + } } - for (const stats of input.untracked.values()) { - added += stats.added ?? 0 - removed += stats.removed ?? 0 + return { + added, + removed, + mergeBase: input.mergeBase, + test: { added: testAdded, removed: testRemoved }, + generated: { added: generatedAdded, removed: generatedRemoved } } - return { added, removed, mergeBase: input.mergeBase } } // Why: one shared exec per (host, worktree, mergeBase). The renderer's own diff --git a/src/shared/git-status-types.ts b/src/shared/git-status-types.ts index 7b6c6e12c..23b57aebb 100644 --- a/src/shared/git-status-types.ts +++ b/src/shared/git-status-types.ts @@ -59,6 +59,10 @@ export type GitBranchLineTotal = { added: number removed: number mergeBase: string + // Path-heuristic buckets of added/removed; generated wins on overlap. Optional + // for pre-split hosts (absent = unknown, not zero). + test?: { added: number; removed: number } + generated?: { added: number; removed: number } } export type GitStatusResult = { diff --git a/src/shared/test-code-path.test.ts b/src/shared/test-code-path.test.ts new file mode 100644 index 000000000..778158c47 --- /dev/null +++ b/src/shared/test-code-path.test.ts @@ -0,0 +1,61 @@ +import { describe, expect, it } from 'vitest' +import { isTestCodePath } from './test-code-path' + +describe('isTestCodePath', () => { + it('recognizes the per-ecosystem filename conventions', () => { + for (const filePath of [ + 'src/renderer/src/components/Chip.test.tsx', + 'src/shared/git-status.spec.js', + 'internal/server/handler_test.go', + 'app/models/user_spec.rb', + 'tools/test_migration.py', + 'src/main/java/com/orca/GitTest.java', + 'src/Orca.Core/GitStatusTests.cs', + 'src/UserTest.kt', + 'AppTests.swift', + 'FooSpec.rb', + 'python/conftest.py', + 'src/components/__snapshots__/Chip.tsx.snap' + ]) { + expect(isTestCodePath(filePath), filePath).toBe(true) + } + }) + + it('recognizes test directories anywhere in the path, on either separator', () => { + expect(isTestCodePath('src/__tests__/git-status.ts')).toBe(true) + expect(isTestCodePath('tests/fixtures/repo.json')).toBe(true) + expect(isTestCodePath('e2e/login.ts')).toBe(true) + expect(isTestCodePath('src\\__mocks__\\fs.ts')).toBe(true) + }) + + it('matches whole segments, so production paths that merely contain the word do not count', () => { + for (const filePath of [ + 'src/latest/index.ts', + 'src/contest/leaderboard.ts', + 'src/testing-library-setup.ts', + 'docs/protest.md', + 'src/shared/git-status-types.ts', + // A `specs/` folder is usually specifications, not tests. + 'src/cli/specs/account.ts', + // Case-insensitive "…test.ext" suffixes hit ordinary type names. + 'src/Contest.java', + 'src/Latest.kt', + 'src/MyContest.php', + 'packages/spec.rb', + // The `test_` prefix is pytest's; outside Python it hits fixtures and pages. + 'src/pages/test_page.tsx', + 'fixtures/test_data.json' + ]) { + expect(isTestCodePath(filePath), filePath).toBe(false) + } + // ...but a real test inside one still matches on its basename. + expect(isTestCodePath('src/cli/specs/account.test.ts')).toBe(true) + expect(isTestCodePath('spec/models/user_spec.rb')).toBe(true) + }) + + it('handles a bare filename and an empty path without throwing', () => { + expect(isTestCodePath('chip.test.ts')).toBe(true) + expect(isTestCodePath('chip.ts')).toBe(false) + expect(isTestCodePath('')).toBe(false) + }) +}) diff --git a/src/shared/test-code-path.ts b/src/shared/test-code-path.ts new file mode 100644 index 000000000..2b9a6e1b7 --- /dev/null +++ b/src/shared/test-code-path.ts @@ -0,0 +1,43 @@ +// Path-only test heuristic for branch line-total buckets. Conservative and +// anchored on the raw path (no lowercasing/splitting per file). + +// Whole path segments only, so `src/latest/x.ts` and `contest/` don't count. +// The trailing separator is what keeps a *file* named `test.ts` out of here. +// `spec/` is RSpec's; plural `specs/` is left out because it far more often +// holds specifications (this repo's `src/cli/specs/` is production code), and a +// real test inside one still matches on its `.spec.`/`_spec.` basename. +const TEST_DIRECTORY = + /(?:^|[/\\])(?:__mocks__|__snapshots__|__tests__|cypress|e2e|spec|tests?|testdata)[/\\]/i + +// One alternative per ecosystem convention; `[^./\\]` keeps each anchored match +// inside the basename. Joined into a single regex so V8 walks the path once. +const TEST_BASENAME = new RegExp( + `(?:${[ + /\.(?:test|spec)\.[^./\\]+/, // Chip.test.tsx, git-status.spec.js + /[-_](?:test|spec)s?\.[^./\\]+/, // handler_test.go, user_spec.rb + // `.py` only: the bare `test_` prefix is pytest's convention, and widening it + // to any extension swept in fixtures and pages (test_data.json, test_page.tsx). + // A production helper that happens to be named `test_client.py` is + // indistinguishable from a real module here — pytest collects it too. + /(?:^|[/\\])test_[^./\\]*\.py/, + /(?:^|[/\\])conftest\.py/, + /\.snap/ + ] + .map((pattern) => pattern.source) + .join('|')})$`, + 'i' +) + +// Case-sensitive on purpose: a case-insensitive `test`/`tests`/`spec` suffix +// falsely matches ordinary type names like Contest.java, Latest.kt, MyContest.php. +// JVM/C#/Swift/PHP/Ruby test types use a capital T/S in the conventional suffix. +const TEST_TYPE_NAME_SUFFIX = /(?:Test|Tests|Spec)\.(?:java|kt|kts|scala|groovy|cs|swift|php|rb)$/ + +/** Accepts POSIX and Windows separators — git reports `/`, callers may not. */ +export function isTestCodePath(filePath: string): boolean { + return ( + TEST_DIRECTORY.test(filePath) || + TEST_BASENAME.test(filePath) || + TEST_TYPE_NAME_SUFFIX.test(filePath) + ) +}