From 0b0c74649bfdfcf1a807c32103422fb5c0ff33b7 Mon Sep 17 00:00:00 2001 From: Liang Hu Date: Sat, 25 Jul 2026 17:48:24 -0400 Subject: [PATCH 01/10] feat: compare task coverage with base branch --- electron/ipc/channel-manifest.json | 1 + electron/ipc/git.test.ts | 60 +++++ electron/ipc/git.ts | 84 +++++-- electron/ipc/register.ts | 4 + electron/ipc/shared-types.ts | 2 + electron/preload.cjs | 1 + src/components/ChangedFilesList.tsx | 258 ++++++++++++++++++--- src/components/MergeDialog.tsx | 7 + src/components/MergeReadinessPanel.test.ts | 58 ++++- src/components/MergeReadinessPanel.tsx | 3 + src/components/TaskChangedFilesSection.tsx | 2 + src/components/merge-readiness.ts | 44 ++++ src/lib/coverage-comparison.test.ts | 160 +++++++++++++ src/lib/coverage-comparison.ts | 144 ++++++++++++ 14 files changed, 786 insertions(+), 42 deletions(-) create mode 100644 src/lib/coverage-comparison.test.ts create mode 100644 src/lib/coverage-comparison.ts diff --git a/electron/ipc/channel-manifest.json b/electron/ipc/channel-manifest.json index d4f545d0..1d9316aa 100644 --- a/electron/ipc/channel-manifest.json +++ b/electron/ipc/channel-manifest.json @@ -18,6 +18,7 @@ "GetFileDiffFromBranch": "get_file_diff_from_branch", "GetGitignoredDirs": "get_gitignored_dirs", "ListImportableWorktrees": "list_importable_worktrees", + "GetBranchWorktreePath": "get_branch_worktree_path", "GetWorktreeStatus": "get_worktree_status", "CheckMergeStatus": "check_merge_status", "MergeTask": "merge_task", diff --git a/electron/ipc/git.test.ts b/electron/ipc/git.test.ts index be6137c6..2974d90c 100644 --- a/electron/ipc/git.test.ts +++ b/electron/ipc/git.test.ts @@ -59,6 +59,7 @@ import { getFileDiff, getUncommittedChangedFiles, checkMergeStatus, + getBranchWorktreePath, listImportableWorktrees, mergeTask, } from './git.js'; @@ -667,6 +668,29 @@ describe('getChangedFiles (worktree-based, merge-base diff)', () => { expect(files[0].status).toBe('A'); }); + it('should preserve the original path for renamed files', async () => { + const calls: string[][] = []; + setupMock( + calls, + buildWorktreeMockHandler({ + committedRawNumstat: [ + ':100644 100644 aaa111 bbb222 R100\tsrc/old-name.ts\tsrc/new-name.ts', + '0\t0\tsrc/{old-name.ts => new-name.ts}', + ].join('\n'), + }), + ); + + const files = await getChangedFiles(uniqueWorktreePath(), 'main'); + + expect(files).toEqual([ + expect.objectContaining({ + path: 'src/new-name.ts', + previous_path: 'src/old-name.ts', + status: 'R', + }), + ]); + }); + it('should return multiple committed files', async () => { const calls: string[][] = []; setupMock( @@ -1419,6 +1443,42 @@ describe('listImportableWorktrees', () => { }); }); +describe('getBranchWorktreePath', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('returns only an existing worktree on the requested branch', async () => { + const calls: string[][] = []; + setupMock(calls, (args, cb) => { + if (args[0] === 'worktree' && args[1] === 'list') { + return cb( + null, + [ + 'worktree /repo', + 'HEAD aaa111', + 'branch refs/heads/main', + '', + 'worktree /repo-task', + 'HEAD bbb222', + 'branch refs/heads/task/coverage', + '', + ].join('\n'), + '', + ); + } + return cb(new Error(`unexpected git call: ${args.join(' ')}`), '', ''); + }); + + await expect(getBranchWorktreePath('/repo', 'main')).resolves.toBe('/repo'); + await expect(getBranchWorktreePath('/repo', 'missing')).resolves.toBeNull(); + expect(calls).toEqual([ + ['worktree', 'list', '--porcelain'], + ['worktree', 'list', '--porcelain'], + ]); + }); +}); + // --------------------------------------------------------------------------- // checkMergeStatus — main-ahead count uses cherry-pick filtering so rebased // patch-equivalent commits in main don't trigger a needless rebase prompt. diff --git a/electron/ipc/git.ts b/electron/ipc/git.ts index 4bc4d176..b88d1dd7 100644 --- a/electron/ipc/git.ts +++ b/electron/ipc/git.ts @@ -533,18 +533,42 @@ async function detectRepoLockKey(p: string): Promise { function normalizeStatusPath(raw: string): string { const trimmed = raw.trim(); if (!trimmed) return ''; - // Handle rename/copy "old -> new" - const destination = trimmed.split(' -> ').pop()?.trim() ?? trimmed; - return destination.replace(/^"|"$/g, '').replace(/\\(.)/g, '$1'); + return trimmed.replace(/^"|"$/g, '').replace(/\\(.)/g, '$1'); +} + +function parseNumstatPath(raw: string): { path: string; previousPath?: string } { + const trimmed = raw.trim(); + const arrowIndex = trimmed.indexOf(' => '); + if (arrowIndex < 0) return { path: normalizeStatusPath(trimmed) }; + + const openBrace = trimmed.lastIndexOf('{', arrowIndex); + const closeBrace = trimmed.indexOf('}', arrowIndex); + if (openBrace >= 0 && closeBrace > arrowIndex) { + const prefix = trimmed.slice(0, openBrace); + const suffix = trimmed.slice(closeBrace + 1); + const previousPath = `${prefix}${trimmed.slice(openBrace + 1, arrowIndex)}${suffix}`; + const destinationPath = `${prefix}${trimmed.slice(arrowIndex + 4, closeBrace)}${suffix}`; + return { + path: normalizeStatusPath(destinationPath), + previousPath: normalizeStatusPath(previousPath), + }; + } + + return { + path: normalizeStatusPath(trimmed.slice(arrowIndex + 4)), + previousPath: normalizeStatusPath(trimmed.slice(0, arrowIndex)), + }; } /** Parse combined `git diff --raw --numstat` output into status and numstat maps. */ function parseDiffRawNumstat(output: string): { statusMap: Map; numstatMap: Map; + previousPathMap: Map; } { const statusMap = new Map(); const numstatMap = new Map(); + const previousPathMap = new Map(); for (const line of output.split('\n')) { if (line.startsWith(':')) { @@ -555,6 +579,10 @@ function parseDiffRawNumstat(output: string): { const rawPath = parts[parts.length - 1]; const p = normalizeStatusPath(rawPath); if (p) statusMap.set(p, statusLetter); + if ((statusLetter === 'R' || statusLetter === 'C') && parts.length >= 3) { + const previousPath = normalizeStatusPath(parts[parts.length - 2]); + if (p && previousPath) previousPathMap.set(p, previousPath); + } } continue; } @@ -565,18 +593,21 @@ function parseDiffRawNumstat(output: string): { const removed = parseInt(parts[1], 10); if (!isNaN(added) && !isNaN(removed)) { const rawPath = parts[parts.length - 1]; - const p = normalizeStatusPath(rawPath); + const parsedPath = parseNumstatPath(rawPath); + const p = parsedPath.path; if (p) numstatMap.set(p, [added, removed]); + if (p && parsedPath.previousPath) previousPathMap.set(p, parsedPath.previousPath); } } } - return { statusMap, numstatMap }; + return { statusMap, numstatMap, previousPathMap }; } export function changedFilesFromMaps(opts: { statusMap: Map; numstatMap: Map; + previousPathMap?: Map; committed: boolean | ((filePath: string) => boolean); sort?: boolean; }): ChangedFile[] { @@ -589,6 +620,7 @@ export function changedFilesFromMaps(opts: { seen.add(p); files.push({ path: p, + previous_path: opts.previousPathMap?.get(p), lines_added: added, lines_removed: removed, status: opts.statusMap.get(p) ?? 'M', @@ -600,6 +632,7 @@ export function changedFilesFromMaps(opts: { if (seen.has(p)) continue; files.push({ path: p, + previous_path: opts.previousPathMap?.get(p), lines_added: 0, lines_removed: 0, status, @@ -1097,8 +1130,11 @@ export async function getChangedFiles( /* empty */ } - const { statusMap: finalStatusMap, numstatMap: finalNumstatMap } = - parseDiffRawNumstat(finalDiffStr); + const { + statusMap: finalStatusMap, + numstatMap: finalNumstatMap, + previousPathMap: finalPreviousPathMap, + } = parseDiffRawNumstat(finalDiffStr); // git diff --raw --numstat — tracked uncommitted changes (HEAD vs working tree). // Compares HEAD tree directly to the working tree, so it does not need the index @@ -1131,6 +1167,7 @@ export async function getChangedFiles( const files = changedFilesFromMaps({ statusMap: finalStatusMap, numstatMap: finalNumstatMap, + previousPathMap: finalPreviousPathMap, committed: isCommitted, sort: false, }); @@ -1293,8 +1330,14 @@ export async function getUncommittedChangedFiles(worktreePath: string): Promise< /* empty */ } - const { statusMap, numstatMap } = parseDiffRawNumstat(diffStr); - const files = changedFilesFromMaps({ statusMap, numstatMap, committed: false, sort: false }); + const { statusMap, numstatMap, previousPathMap } = parseDiffRawNumstat(diffStr); + const files = changedFilesFromMaps({ + statusMap, + numstatMap, + previousPathMap, + committed: false, + sort: false, + }); const seen = new Set(files.map((file) => file.path)); files.push(...(await getUntrackedChangedFiles(worktreePath, seen))); @@ -1540,6 +1583,21 @@ export async function listImportableWorktrees(projectRoot: string): Promise< return filtered; } +/** Resolve an already checked-out local branch without creating or switching worktrees. */ +export async function getBranchWorktreePath( + projectRoot: string, + branchName: string, +): Promise { + const { stdout } = await exec('git', ['worktree', 'list', '--porcelain'], { + cwd: projectRoot, + maxBuffer: MAX_BUFFER, + }); + const match = parseWorktreeList(stdout).find( + (entry) => !entry.detached && entry.branchName === branchName, + ); + return match?.path ?? null; +} + /** Stage all changes and commit in a worktree. */ export async function commitAll(worktreePath: string, message: string): Promise { await exec('git', ['add', '-A'], { cwd: worktreePath }); @@ -1756,9 +1814,9 @@ export async function getChangedFilesFromBranch( return []; } - const { statusMap, numstatMap } = parseDiffRawNumstat(diffStr); + const { statusMap, numstatMap, previousPathMap } = parseDiffRawNumstat(diffStr); - return changedFilesFromMaps({ statusMap, numstatMap, committed: true }); + return changedFilesFromMaps({ statusMap, numstatMap, previousPathMap, committed: true }); } export async function getFileDiffFromBranch( @@ -1984,9 +2042,9 @@ export async function getCommitChangedFiles( } } - const { statusMap, numstatMap } = parseDiffRawNumstat(diffStr); + const { statusMap, numstatMap, previousPathMap } = parseDiffRawNumstat(diffStr); - return changedFilesFromMaps({ statusMap, numstatMap, committed: true }); + return changedFilesFromMaps({ statusMap, numstatMap, previousPathMap, committed: true }); } export async function getCommitDiffs(worktreePath: string, commitHash: string): Promise { diff --git a/electron/ipc/register.ts b/electron/ipc/register.ts index aae2d88a..c85d92de 100644 --- a/electron/ipc/register.ts +++ b/electron/ipc/register.ts @@ -54,6 +54,7 @@ import { getFileDiffFromBranch, getWorktreeStatus, listImportableWorktrees, + getBranchWorktreePath, commitAll, discardUncommitted, checkMergeStatus, @@ -603,6 +604,9 @@ export function registerAllHandlers(win: BrowserWindow): void { ipcMain.handle(IPC.ListImportableWorktrees, (_e, args) => { return listImportableWorktrees(projectRootArg(args)); }); + ipcMain.handle(IPC.GetBranchWorktreePath, (_e, args) => { + return getBranchWorktreePath(projectRootArg(args), branchNameArg(args)); + }); ipcMain.handle(IPC.GetWorktreeStatus, (_e, args) => { const worktreePath = worktreePathArg(args); return getWorktreeStatus(worktreePath, optionalBaseBranch(args)); diff --git a/electron/ipc/shared-types.ts b/electron/ipc/shared-types.ts index 9ef93cc6..8e53cd09 100644 --- a/electron/ipc/shared-types.ts +++ b/electron/ipc/shared-types.ts @@ -29,6 +29,8 @@ export interface CreateTaskResult { export interface ChangedFile { path: string; + /** Original path when Git reports a rename or copy. */ + previous_path?: string; lines_added: number; lines_removed: number; status: string; diff --git a/electron/preload.cjs b/electron/preload.cjs index d0126c6c..1ce97c81 100644 --- a/electron/preload.cjs +++ b/electron/preload.cjs @@ -22,6 +22,7 @@ const ALLOWED_CHANNELS = new Set([ 'get_file_diff_from_branch', 'get_gitignored_dirs', 'list_importable_worktrees', + 'get_branch_worktree_path', 'get_worktree_status', 'check_merge_status', 'merge_task', diff --git a/src/components/ChangedFilesList.tsx b/src/components/ChangedFilesList.tsx index f44ca2e8..94dce452 100644 --- a/src/components/ChangedFilesList.tsx +++ b/src/components/ChangedFilesList.tsx @@ -6,6 +6,13 @@ import { sf } from '../lib/fontScale'; import { getStatusColor } from '../lib/status-colors'; import { openFileInEditor } from '../lib/shell'; import { buildFileTree, flattenVisibleTree } from '../lib/file-tree'; +import { + buildCoverageComparison, + formatCoverageDelta, + type CoverageComparison, + type CoverageFileComparison, + type CoverageValue, +} from '../lib/coverage-comparison'; import { type CommitSelection, isCommitHashSelection, @@ -30,6 +37,8 @@ interface ChangedFilesListProps { branchName?: string | null; /** Base branch for diff comparison (e.g. 'main', 'develop'). Undefined = auto-detect. */ baseBranch?: string; + /** Reports coverage changes when task or base coverage data refreshes. */ + onCoverageComparisonChange?: (comparison: CoverageComparison | null) => void; /** * Selection mode for the file list: * - undefined/null: all changes (committed + uncommitted) @@ -42,15 +51,16 @@ interface ChangedFilesListProps { const SOURCE_FILE_RE = /\.(?:[cm]?[jt]sx?)$/i; const TEST_FILE_RE = /\.(?:test|spec)\.(?:[cm]?[jt]sx?)$/i; -export function isCoverageEligible(file: ChangedFile): boolean { +function isCoverageCandidate(file: ChangedFile): boolean { return ( - file.status !== 'D' && - SOURCE_FILE_RE.test(file.path) && - !TEST_FILE_RE.test(file.path) && - !file.path.endsWith('.d.ts') + SOURCE_FILE_RE.test(file.path) && !TEST_FILE_RE.test(file.path) && !file.path.endsWith('.d.ts') ); } +export function isCoverageEligible(file: ChangedFile): boolean { + return file.status !== 'D' && isCoverageCandidate(file); +} + export function coverageFooterLabel( hasCoverageArtifact: boolean, touchedCoveragePct: number | null, @@ -99,37 +109,117 @@ function coverageBadgeTitle(summary: CoverageFileSummary): string { return `Lines ${summary.lines.pct}% · Branches ${summary.branches.pct}% · Functions ${summary.functions.pct}% · Statements ${summary.statements.pct}%`; } +function coverageValueLabel(value: CoverageValue): string { + if (value.state === 'available') return `${value.pct}%`; + if (value.state === 'no-executable-lines') return 'no lines'; + if (value.state === 'file-not-present') return 'not present'; + return 'no report'; +} + +function deltaColor(delta: number): string { + if (delta > 0) return theme.success; + if (delta < 0) return theme.error; + return theme.fgMuted; +} + +function comparisonBadge( + comparison: CoverageFileComparison, +): { label: string; color: string; title: string } | null { + const taskLabel = coverageValueLabel(comparison.task); + const baseLabel = coverageValueLabel(comparison.base); + const renameDetail = + comparison.kind === 'renamed' ? ` (${comparison.basePath} → ${comparison.path})` : ''; + + if (comparison.kind === 'deleted') { + if (comparison.base.state === 'no-report') return null; + return { + label: comparison.base.state === 'available' ? `del ${baseLabel}` : 'deleted', + color: theme.fgMuted, + title: `Deleted file${renameDetail}. Base: ${baseLabel}; task: not present.`, + }; + } + + if (comparison.task.state === 'no-executable-lines') { + return { + label: 'no lines', + color: theme.fgMuted, + title: `Task: no executable lines; base: ${baseLabel}${renameDetail}.`, + }; + } + + if (comparison.task.state === 'no-report' && comparison.base.state !== 'no-report') { + return { + label: 'no report', + color: theme.fgMuted, + title: `No task coverage report; base: ${baseLabel}${renameDetail}.`, + }; + } + + if (comparison.task.state !== 'available') return null; + + if (comparison.delta !== null) { + return { + label: `${taskLabel} ${formatCoverageDelta(comparison.delta)}`, + color: deltaColor(comparison.delta), + title: `Task: ${taskLabel}; base: ${baseLabel}; delta: ${formatCoverageDelta(comparison.delta)}${renameDetail}.`, + }; + } + + const kindLabel = + comparison.kind === 'new' ? ' new' : comparison.kind === 'renamed' ? ' renamed' : ''; + return { + label: `${taskLabel}${kindLabel}`, + color: coverageColor(comparison.task.pct ?? 0), + title: `Task: ${taskLabel}; base: ${baseLabel}${renameDetail}.`, + }; +} + function FileCoverageBadge(props: { file: ChangedFile; selectedCommit?: CommitSelection; summary?: CoverageFileSummary; + comparison?: CoverageFileComparison; hasCoverageArtifact: boolean; }) { - const isEligible = () => - !isCommitHashSelection(props.selectedCommit) && isCoverageEligible(props.file); - const summary = () => (isEligible() ? props.summary : undefined); + const isCandidate = () => + !isCommitHashSelection(props.selectedCommit) && isCoverageCandidate(props.file); + const summary = () => (isCandidate() ? props.summary : undefined); + const badge = () => + isCandidate() && props.comparison ? comparisonBadge(props.comparison) : null; return ( <> - - {(coverageSummary) => ( + + {(coverageBadge) => ( - {coverageSummary.lines.pct}% + {coverageBadge.label} )} - + { + if (!projectRoot) return null; + const baseRoot = await invoke(IPC.GetBranchWorktreePath, { + projectRoot, + branchName: baseBranch, + }).catch(() => null); + if (!baseRoot || baseRoot === taskRoot) return null; + return baseRoot; +} + function OpenInEditorButton(props: { worktreePath: string; filePath: string; @@ -189,6 +293,7 @@ function OpenInEditorButton(props: { export function ChangedFilesList(props: ChangedFilesListProps) { const [files, setFiles] = createSignal([]); const [coverage, setCoverage] = createSignal(null); + const [baseCoverage, setBaseCoverage] = createSignal(null); const [canOpenFilesInEditor, setCanOpenFilesInEditor] = createSignal(false); const [selectedIndex, setSelectedIndex] = createSignal(-1); const [collapsed, setCollapsed] = createSignal>(new Set()); @@ -198,7 +303,14 @@ export function ChangedFilesList(props: ChangedFilesListProps) { const visibleRows = createMemo(() => flattenVisibleTree(tree(), collapsed())); const coverageFiles = createMemo(() => coverage()?.files ?? {}); const hasCoverageArtifact = createMemo(() => coverage() !== null); + const hasBaseCoverageArtifact = createMemo(() => baseCoverage() !== null); + const coverageCandidateFiles = createMemo(() => + files().filter((file) => isCoverageCandidate(file)), + ); const eligibleFiles = createMemo(() => files().filter((file) => isCoverageEligible(file))); + const coverageComparison = createMemo(() => + buildCoverageComparison(coverage(), baseCoverage(), coverageCandidateFiles()), + ); const coveredEligibleFiles = createMemo(() => eligibleFiles().filter((file) => Boolean(coverageFiles()[file.path])), ); @@ -224,6 +336,44 @@ export function ChangedFilesList(props: ChangedFilesListProps) { if (totalLines === 0) return null; return Math.round((coveredLines / totalLines) * 100); }); + const aggregateCoverageLabel = createMemo(() => { + const comparison = coverageComparison().aggregate; + if (!hasBaseCoverageArtifact()) return null; + const delta = comparison.delta === null ? '' : ` (${formatCoverageDelta(comparison.delta)})`; + return `base ${coverageValueLabel(comparison.base)} → task ${coverageValueLabel(comparison.task)}${delta}`; + }); + const aggregateCoverageTitle = createMemo(() => { + const comparison = coverageComparison(); + const taskReport = coverage(); + const baseReport = baseCoverage(); + if (!baseReport) return ''; + const lines = [ + `Base: ${coverageValueLabel(comparison.aggregate.base)} (${baseReport.reportPath}).`, + taskReport + ? `Task: ${coverageValueLabel(comparison.aggregate.task)} (${taskReport.reportPath}).` + : 'Task: no coverage report.', + ]; + if (comparison.aggregate.delta !== null) { + lines.push(`Delta: ${formatCoverageDelta(comparison.aggregate.delta)}.`); + } + if (comparison.impactedUnchangedFiles.length > 0) { + const impacted = comparison.impactedUnchangedFiles + .slice(0, 3) + .map((file) => `${file.path} ${formatCoverageDelta(file.delta)}`) + .join(', '); + lines.push( + `${comparison.impactedUnchangedFiles.length} materially impacted unchanged file${comparison.impactedUnchangedFiles.length === 1 ? '' : 's'}: ${impacted}.`, + ); + } + return lines.join(' '); + }); + + createEffect(() => { + if (!props.onCoverageComparisonChange) return; + props.onCoverageComparisonChange( + isCommitHashSelection(props.selectedCommit) ? null : coverageComparison(), + ); + }); function toggleDir(path: string) { const isCollapsing = !collapsed().has(path); @@ -443,26 +593,51 @@ export function ChangedFilesList(props: ChangedFilesListProps) { createEffect(() => { const repoRoot = props.worktreePath; + const projectRoot = props.projectRoot; + const baseBranch = props.baseBranch; const selection = props.selectedCommit; if (!repoRoot || isCommitHashSelection(selection)) { - setCoverage(null); + batch(() => { + setCoverage(null); + setBaseCoverage(null); + }); return; } if (!props.isActive) return; let cancelled = false; let inFlight = false; + const baseBranchPromise = baseBranch + ? Promise.resolve(baseBranch) + : projectRoot + ? invoke(IPC.GetMainBranch, { projectRoot }) + : Promise.resolve(null); async function refresh() { if (inFlight) return; inFlight = true; try { - const result = await invoke(IPC.GetCoverageSummary, { - repoRoot, - reportPath: props.coverageReportPath, - }); - if (!cancelled) setCoverage(result); - } catch { - if (!cancelled) setCoverage(null); + const resolvedBaseBranch = await baseBranchPromise.catch(() => null); + const baseRoot = resolvedBaseBranch + ? await resolveBaseCoverageRoot(projectRoot, resolvedBaseBranch, repoRoot) + : null; + const [taskResult, baseResult] = await Promise.all([ + invoke(IPC.GetCoverageSummary, { + repoRoot, + reportPath: props.coverageReportPath, + }).catch(() => null), + baseRoot + ? invoke(IPC.GetCoverageSummary, { + repoRoot: baseRoot, + reportPath: props.coverageReportPath, + }).catch(() => null) + : Promise.resolve(null), + ]); + if (!cancelled) { + batch(() => { + setCoverage(taskResult); + setBaseCoverage(baseResult); + }); + } } finally { inFlight = false; } @@ -610,6 +785,7 @@ export function ChangedFilesList(props: ChangedFilesListProps) { file={file} selectedCommit={props.selectedCommit} summary={coverageFiles()[row().node.path]} + comparison={coverageComparison().files[row().node.path]} hasCoverageArtifact={hasCoverageArtifact()} /> )} @@ -658,7 +834,11 @@ export function ChangedFilesList(props: ChangedFilesListProps) { 'flex-wrap': 'wrap', }} > - 0}> + 0 + } + >
- + + {(label) => ( + + {label()} + + )} + + - + + 0}> + + ↕ {coverageComparison().impactedUnchangedFiles.length} other + +
diff --git a/src/components/MergeDialog.tsx b/src/components/MergeDialog.tsx index 9b169153..b2e8c1b5 100644 --- a/src/components/MergeDialog.tsx +++ b/src/components/MergeDialog.tsx @@ -14,6 +14,7 @@ import { ChangedFilesList } from './ChangedFilesList'; import { MergeReadinessPanel } from './MergeReadinessPanel'; import { buildMergeReadiness } from './merge-readiness'; import { theme, bannerStyle } from '../lib/theme'; +import type { CoverageComparison } from '../lib/coverage-comparison'; import type { Task } from '../store/types'; import type { ChangedFile, MergeStatus, WorktreeStatus } from '../ipc/types'; @@ -34,6 +35,7 @@ export function MergeDialog(props: MergeDialogProps) { const [rebasing, setRebasing] = createSignal(false); const [rebaseError, setRebaseError] = createSignal(''); const [rebaseSuccess, setRebaseSuccess] = createSignal(false); + const [coverageComparison, setCoverageComparison] = createSignal(null); const resourceSource = () => props.open ? { path: props.task.worktreePath, baseBranch: props.task.baseBranch } : null; @@ -91,6 +93,7 @@ export function MergeDialog(props: MergeDialogProps) { worktreeStatusLoading: worktreeStatus.loading, verification: props.task.verification, prChecks: getPrChecks(props.task.id), + coverage: coverageComparison(), }); createEffect(() => { @@ -103,6 +106,7 @@ export function MergeDialog(props: MergeDialogProps) { setRebaseSuccess(false); setMerging(false); setRebasing(false); + setCoverageComparison(null); // Drop the previous open's cached data so accessors return undefined // during refetch — otherwise unguarded reads (uncommitted-changes // warning, branch-mismatch banner) flash the stale snapshot until the @@ -444,10 +448,13 @@ export function MergeDialog(props: MergeDialogProps) { >
{/* Imported worktrees are user-owned — never offer to delete them or their branch. */} diff --git a/src/components/MergeReadinessPanel.test.ts b/src/components/MergeReadinessPanel.test.ts index b16bf7f6..021d45c2 100644 --- a/src/components/MergeReadinessPanel.test.ts +++ b/src/components/MergeReadinessPanel.test.ts @@ -44,6 +44,7 @@ describe('buildMergeReadiness', () => { expect(readiness.checks).toEqual([ expect.objectContaining({ label: 'Merge safety', status: 'pass' }), expect.objectContaining({ label: 'Verification', status: 'pass' }), + expect.objectContaining({ label: 'Coverage', status: 'neutral' }), expect.objectContaining({ label: 'PR checks', status: 'neutral' }), ]); }); @@ -153,7 +154,7 @@ describe('buildMergeReadiness', () => { expect(readiness.checks[1]).toEqual( expect.objectContaining({ status: 'warning', detail: 'test failed — 2 tests failed' }), ); - expect(readiness.checks[2]).toEqual( + expect(readiness.checks[3]).toEqual( expect.objectContaining({ status: 'warning', detail: '1 pending, 2 passing.' }), ); }); @@ -166,13 +167,61 @@ describe('buildMergeReadiness', () => { ); expect(readiness.overall).toBe('attention'); - expect(readiness.checks[2]).toEqual( + expect(readiness.checks[3]).toEqual( expect.objectContaining({ status: 'warning', detail: '1 pending, 2 passing, 1 failing.', }), ); }); + + it('reports attention for aggregate coverage regression and impacted unchanged files', () => { + const readiness = buildMergeReadiness( + input({ + coverage: { + aggregate: { + task: { state: 'available', pct: 78 }, + base: { state: 'available', pct: 82 }, + delta: -4, + }, + files: {}, + impactedUnchangedFiles: [{ path: 'src/shared.ts', taskPct: 70, basePct: 80, delta: -10 }], + }, + }), + ); + + expect(readiness.overall).toBe('attention'); + expect(readiness.checks[2]).toEqual( + expect.objectContaining({ + label: 'Coverage', + status: 'warning', + detail: 'Base 82% → task 78% (-4pp). 1 unchanged file also regressed.', + }), + ); + }); + + it('keeps an unchanged-file regression visible when aggregate coverage improves', () => { + const readiness = buildMergeReadiness( + input({ + coverage: { + aggregate: { + task: { state: 'available', pct: 84 }, + base: { state: 'available', pct: 82 }, + delta: 2, + }, + files: {}, + impactedUnchangedFiles: [{ path: 'src/shared.ts', taskPct: 70, basePct: 80, delta: -10 }], + }, + }), + ); + + expect(readiness.checks[2]).toEqual( + expect.objectContaining({ + status: 'warning', + detail: 'Base 82% → task 84% (+2pp). 1 unchanged file also regressed.', + }), + ); + }); }); describe('MergeReadinessPanel', () => { @@ -185,6 +234,8 @@ describe('MergeReadinessPanel', () => { expect(html).toContain('Merge safety'); expect(html).toContain('Verification'); expect(html).toContain('2 checks passed.'); + expect(html).toContain('Coverage'); + expect(html).toContain('No task coverage report.'); expect(html).toContain('PR checks'); expect(html).toContain('No PR checks available.'); expect(html).toContain( @@ -199,5 +250,8 @@ describe('MergeReadinessPanel', () => { expect(html).toContain( 'title="Uses checks reported for a detected GitHub pull request. Pull requests are optional, and unavailable check data is neutral."', ); + expect(html).toContain( + 'title="Compares existing task and base-branch coverage reports. Opening the dialog never runs tests or modifies either worktree."', + ); }); }); diff --git a/src/components/MergeReadinessPanel.tsx b/src/components/MergeReadinessPanel.tsx index 547d5a69..cd803334 100644 --- a/src/components/MergeReadinessPanel.tsx +++ b/src/components/MergeReadinessPanel.tsx @@ -23,6 +23,9 @@ function checkHelp(label: string): string | undefined { if (label === 'PR checks') { return 'Uses checks reported for a detected GitHub pull request. Pull requests are optional, and unavailable check data is neutral.'; } + if (label === 'Coverage') { + return 'Compares existing task and base-branch coverage reports. Opening the dialog never runs tests or modifies either worktree.'; + } return undefined; } diff --git a/src/components/TaskChangedFilesSection.tsx b/src/components/TaskChangedFilesSection.tsx index 137c26f0..77e20163 100644 --- a/src/components/TaskChangedFilesSection.tsx +++ b/src/components/TaskChangedFilesSection.tsx @@ -151,6 +151,8 @@ export function TaskChangedFilesSection(props: TaskChangedFilesSectionProps) {
file.delta < 0); + const impactedDetail = + regressedUnchanged.length > 0 + ? ` ${countLabel(regressedUnchanged.length, 'unchanged file')} also regressed.` + : ''; + return { + label: 'Coverage', + status: aggregate.delta < 0 || regressedUnchanged.length > 0 ? 'warning' : 'pass', + detail: `Base ${aggregate.base.pct}% → task ${taskPct}% (${formatCoverageDelta(aggregate.delta)}).${impactedDetail}`, + }; +} + export function buildMergeReadiness(input: MergeReadinessInput): MergeReadiness { const checks = [ mergeSafetyCheck(input), verificationCheck(input.verification), + coverageCheck(input.coverage), prCheck(input.prChecks), ]; const overall = checks.some((check) => check.status === 'blocked') diff --git a/src/lib/coverage-comparison.test.ts b/src/lib/coverage-comparison.test.ts new file mode 100644 index 00000000..287c0846 --- /dev/null +++ b/src/lib/coverage-comparison.test.ts @@ -0,0 +1,160 @@ +import { describe, expect, it } from 'vitest'; +import type { + ChangedFile, + CoverageFileSummary, + CoverageMetricSummary, + CoverageSummary, +} from '../ipc/types'; +import { + buildCoverageComparison, + formatCoverageDelta, + MATERIAL_COVERAGE_DELTA, +} from './coverage-comparison'; + +function metric(pct: number, total = 100): CoverageMetricSummary { + return { + total, + covered: total === 0 ? 0 : Math.round((pct / 100) * total), + skipped: 0, + pct, + }; +} + +function file(path: string, pct: number, total = 100): CoverageFileSummary { + const value = metric(pct, total); + return { + path, + lines: value, + statements: value, + functions: value, + branches: value, + }; +} + +function report(totalPct: number, files: CoverageFileSummary[]): CoverageSummary { + const total = metric(totalPct); + return { + format: 'istanbul-summary', + generatedAt: '2026-07-25T00:00:00.000Z', + reportPath: '/repo/coverage/coverage-summary.json', + totals: { + lines: total, + statements: total, + functions: total, + branches: total, + }, + files: Object.fromEntries(files.map((entry) => [entry.path, entry])), + }; +} + +function changed(path: string, status = 'M', previousPath?: string): ChangedFile { + return { + path, + previous_path: previousPath, + lines_added: 1, + lines_removed: 1, + status, + committed: true, + }; +} + +describe('buildCoverageComparison', () => { + it('calculates aggregate and per-file positive, negative, and zero deltas', () => { + const base = report(80, [ + file('src/up.ts', 70), + file('src/down.ts', 90), + file('src/same.ts', 75), + ]); + const task = report(82.25, [ + file('src/up.ts', 80), + file('src/down.ts', 85), + file('src/same.ts', 75), + ]); + + const result = buildCoverageComparison(task, base, [ + changed('src/up.ts'), + changed('src/down.ts'), + changed('src/same.ts'), + ]); + + expect(result.aggregate.delta).toBe(2.25); + expect(result.files['src/up.ts'].delta).toBe(10); + expect(result.files['src/down.ts'].delta).toBe(-5); + expect(result.files['src/same.ts'].delta).toBe(0); + }); + + it('defines new, deleted, and renamed file behavior', () => { + const base = report(80, [file('src/deleted.ts', 70), file('src/old-name.ts', 60)]); + const task = report(82, [file('src/new.ts', 90), file('src/new-name.ts', 75)]); + + const result = buildCoverageComparison(task, base, [ + changed('src/new.ts', 'A'), + changed('src/deleted.ts', 'D'), + changed('src/new-name.ts', 'R', 'src/old-name.ts'), + ]); + + expect(result.files['src/new.ts']).toMatchObject({ + kind: 'new', + task: { state: 'available', pct: 90 }, + base: { state: 'file-not-present', pct: null }, + delta: null, + }); + expect(result.files['src/deleted.ts']).toMatchObject({ + kind: 'deleted', + task: { state: 'file-not-present', pct: null }, + base: { state: 'available', pct: 70 }, + delta: null, + }); + expect(result.files['src/new-name.ts']).toMatchObject({ + kind: 'renamed', + basePath: 'src/old-name.ts', + delta: 15, + }); + }); + + it('distinguishes no report, file absence, and no executable lines', () => { + const noLines = file('src/no-lines.ts', 100, 0); + const task = report(80, [noLines]); + + const noBase = buildCoverageComparison(task, null, [changed('src/no-lines.ts')]); + expect(noBase.aggregate.base.state).toBe('no-report'); + expect(noBase.files['src/no-lines.ts'].task.state).toBe('no-executable-lines'); + expect(noBase.files['src/no-lines.ts'].base.state).toBe('no-report'); + + const missing = buildCoverageComparison(task, report(75, []), [changed('src/missing.ts')]); + expect(missing.files['src/missing.ts'].task.state).toBe('file-not-present'); + expect(missing.files['src/missing.ts'].base.state).toBe('file-not-present'); + }); + + it('reports materially impacted unchanged files without duplicating changed paths', () => { + const base = report(80, [ + file('src/changed.ts', 80), + file('src/regressed.ts', 90), + file('src/noise.ts', 80), + ]); + const task = report(78, [ + file('src/changed.ts', 70), + file('src/regressed.ts', 82), + file('src/noise.ts', 80 + MATERIAL_COVERAGE_DELTA / 2), + ]); + + const result = buildCoverageComparison(task, base, [changed('src/changed.ts')]); + + expect(result.impactedUnchangedFiles).toEqual([ + { + path: 'src/regressed.ts', + taskPct: 82, + basePct: 90, + delta: -8, + }, + ]); + }); +}); + +describe('formatCoverageDelta', () => { + it('formats signed percentage-point values', () => { + expect(formatCoverageDelta(2.345)).toBe('+2.35pp'); + expect(formatCoverageDelta(-1.2)).toBe('-1.2pp'); + expect(formatCoverageDelta(0)).toBe('0pp'); + }); +}); diff --git a/src/lib/coverage-comparison.ts b/src/lib/coverage-comparison.ts new file mode 100644 index 00000000..77e00e10 --- /dev/null +++ b/src/lib/coverage-comparison.ts @@ -0,0 +1,144 @@ +import type { ChangedFile, CoverageSummary } from '../ipc/types'; + +export type CoverageValueState = + | 'available' + | 'no-report' + | 'file-not-present' + | 'no-executable-lines'; + +export interface CoverageValue { + state: CoverageValueState; + pct: number | null; +} + +export type CoverageFileChangeKind = 'changed' | 'new' | 'deleted' | 'renamed'; + +export interface CoverageFileComparison { + path: string; + basePath: string; + kind: CoverageFileChangeKind; + task: CoverageValue; + base: CoverageValue; + delta: number | null; +} + +export interface ImpactedCoverageFile { + path: string; + taskPct: number; + basePct: number; + delta: number; +} + +export interface CoverageComparison { + aggregate: { + task: CoverageValue; + base: CoverageValue; + delta: number | null; + }; + files: Record; + impactedUnchangedFiles: ImpactedCoverageFile[]; +} + +export const MATERIAL_COVERAGE_DELTA = 1; + +function roundPercentage(value: number): number { + return Math.round(value * 100) / 100; +} + +function aggregateValue(summary: CoverageSummary | null): CoverageValue { + if (!summary) return { state: 'no-report', pct: null }; + if (summary.totals.lines.total === 0) { + return { state: 'no-executable-lines', pct: null }; + } + return { state: 'available', pct: summary.totals.lines.pct }; +} + +function fileValue( + summary: CoverageSummary | null, + filePath: string, + forceMissing = false, +): CoverageValue { + if (!summary) return { state: 'no-report', pct: null }; + const file = forceMissing ? undefined : summary.files[filePath]; + if (!file) return { state: 'file-not-present', pct: null }; + if (file.lines.total === 0) return { state: 'no-executable-lines', pct: null }; + return { state: 'available', pct: file.lines.pct }; +} + +function coverageDelta(task: CoverageValue, base: CoverageValue): number | null { + if (task.state !== 'available' || base.state !== 'available') return null; + return roundPercentage((task.pct ?? 0) - (base.pct ?? 0)); +} + +function changeKind(file: ChangedFile): CoverageFileChangeKind { + if (file.status === 'D') return 'deleted'; + if (file.status === 'R') return 'renamed'; + if (file.status === 'A' || file.status === '?' || file.status === 'C') return 'new'; + return 'changed'; +} + +export function formatCoverageDelta(delta: number): string { + const rounded = roundPercentage(delta); + if (rounded > 0) return `+${rounded}pp`; + return `${rounded}pp`; +} + +export function buildCoverageComparison( + taskSummary: CoverageSummary | null, + baseSummary: CoverageSummary | null, + changedFiles: ChangedFile[], +): CoverageComparison { + const taskAggregate = aggregateValue(taskSummary); + const baseAggregate = aggregateValue(baseSummary); + const files: Record = {}; + const changedPaths = new Set(); + + for (const file of changedFiles) { + const kind = changeKind(file); + const basePath = file.previous_path ?? file.path; + changedPaths.add(file.path); + changedPaths.add(basePath); + + const task = fileValue(taskSummary, file.path, kind === 'deleted'); + const base = fileValue(baseSummary, basePath, kind === 'new'); + files[file.path] = { + path: file.path, + basePath, + kind, + task, + base, + delta: coverageDelta(task, base), + }; + } + + const impactedUnchangedFiles: ImpactedCoverageFile[] = []; + if (taskSummary && baseSummary) { + for (const [filePath, taskFile] of Object.entries(taskSummary.files)) { + if (changedPaths.has(filePath)) continue; + const baseFile = baseSummary.files[filePath]; + if (!baseFile || taskFile.lines.total === 0 || baseFile.lines.total === 0) continue; + const delta = roundPercentage(taskFile.lines.pct - baseFile.lines.pct); + if (Math.abs(delta) < MATERIAL_COVERAGE_DELTA) continue; + impactedUnchangedFiles.push({ + path: filePath, + taskPct: taskFile.lines.pct, + basePct: baseFile.lines.pct, + delta, + }); + } + } + + impactedUnchangedFiles.sort( + (a, b) => Math.abs(b.delta) - Math.abs(a.delta) || a.path.localeCompare(b.path), + ); + + return { + aggregate: { + task: taskAggregate, + base: baseAggregate, + delta: coverageDelta(taskAggregate, baseAggregate), + }, + files, + impactedUnchangedFiles, + }; +} From e711e3eb17d8cb93f3130f53a3a070c93649e43a Mon Sep 17 00:00:00 2001 From: Liang Hu Date: Mon, 27 Jul 2026 07:55:27 -0400 Subject: [PATCH 02/10] fix: make coverage comparison preserve unavailable states --- electron/ipc/git.test.ts | 21 ++++++++++ electron/ipc/git.ts | 5 ++- src/components/ChangedFilesList.test.ts | 11 ++++++ src/components/ChangedFilesList.tsx | 29 +++++++++++--- src/components/MergeReadinessPanel.test.ts | 18 ++++++++- src/components/merge-readiness.ts | 6 ++- src/lib/coverage-comparison.test.ts | 46 +++++++++++++++++++++- src/lib/coverage-comparison.ts | 30 ++++++++------ 8 files changed, 144 insertions(+), 22 deletions(-) diff --git a/electron/ipc/git.test.ts b/electron/ipc/git.test.ts index 2974d90c..76f1b417 100644 --- a/electron/ipc/git.test.ts +++ b/electron/ipc/git.test.ts @@ -691,6 +691,27 @@ describe('getChangedFiles (worktree-based, merge-base diff)', () => { ]); }); + it('should preserve a literal arrow in an ordinary modified filename', async () => { + const calls: string[][] = []; + setupMock( + calls, + buildWorktreeMockHandler({ + committedRawNumstat: rawNumstatEntry('ordinary => modified.txt', 3, 1), + }), + ); + + const files = await getChangedFiles(uniqueWorktreePath(), 'main'); + + expect(files).toEqual([ + expect.objectContaining({ + path: 'ordinary => modified.txt', + lines_added: 3, + lines_removed: 1, + status: 'M', + }), + ]); + }); + it('should return multiple committed files', async () => { const calls: string[][] = []; setupMock( diff --git a/electron/ipc/git.ts b/electron/ipc/git.ts index b88d1dd7..87b46af7 100644 --- a/electron/ipc/git.ts +++ b/electron/ipc/git.ts @@ -593,7 +593,10 @@ function parseDiffRawNumstat(output: string): { const removed = parseInt(parts[1], 10); if (!isNaN(added) && !isNaN(removed)) { const rawPath = parts[parts.length - 1]; - const parsedPath = parseNumstatPath(rawPath); + const normalizedPath = normalizeStatusPath(rawPath); + const parsedPath = statusMap.has(normalizedPath) + ? { path: normalizedPath } + : parseNumstatPath(rawPath); const p = parsedPath.path; if (p) numstatMap.set(p, [added, removed]); if (p && parsedPath.previousPath) previousPathMap.set(p, parsedPath.previousPath); diff --git a/src/components/ChangedFilesList.test.ts b/src/components/ChangedFilesList.test.ts index fef489b8..0b9490d9 100644 --- a/src/components/ChangedFilesList.test.ts +++ b/src/components/ChangedFilesList.test.ts @@ -6,6 +6,7 @@ import { filesFooterLabel, filesFooterTitle, isCoverageEligible, + shouldShowCoverageFooter, } from './ChangedFilesList'; function changedFile(overrides: Partial): ChangedFile { @@ -69,6 +70,16 @@ describe('coverageFooterLabel', () => { }); }); +describe('shouldShowCoverageFooter', () => { + it('shows comparison results for test-only changes when reports are loaded', () => { + expect(shouldShowCoverageFooter(undefined, 0, true, true)).toBe(true); + }); + + it('hides coverage details for a single-commit selection', () => { + expect(shouldShowCoverageFooter('abc123', 1, true, true)).toBe(false); + }); +}); + describe('filesFooterLabel', () => { it('shows only total files when everything is committed', () => { expect(filesFooterLabel(7, 0)).toBe('▤ 7'); diff --git a/src/components/ChangedFilesList.tsx b/src/components/ChangedFilesList.tsx index 94dce452..4cb29361 100644 --- a/src/components/ChangedFilesList.tsx +++ b/src/components/ChangedFilesList.tsx @@ -61,6 +61,18 @@ export function isCoverageEligible(file: ChangedFile): boolean { return file.status !== 'D' && isCoverageCandidate(file); } +export function shouldShowCoverageFooter( + selectedCommit: CommitSelection | undefined, + coverageCandidateCount: number, + hasCoverageArtifact: boolean, + hasBaseCoverageArtifact: boolean, +): boolean { + return ( + !isCommitHashSelection(selectedCommit) && + (coverageCandidateCount > 0 || hasCoverageArtifact || hasBaseCoverageArtifact) + ); +} + export function coverageFooterLabel( hasCoverageArtifact: boolean, touchedCoveragePct: number | null, @@ -309,7 +321,7 @@ export function ChangedFilesList(props: ChangedFilesListProps) { ); const eligibleFiles = createMemo(() => files().filter((file) => isCoverageEligible(file))); const coverageComparison = createMemo(() => - buildCoverageComparison(coverage(), baseCoverage(), coverageCandidateFiles()), + buildCoverageComparison(coverage(), baseCoverage(), files()), ); const coveredEligibleFiles = createMemo(() => eligibleFiles().filter((file) => Boolean(coverageFiles()[file.path])), @@ -359,7 +371,11 @@ export function ChangedFilesList(props: ChangedFilesListProps) { if (comparison.impactedUnchangedFiles.length > 0) { const impacted = comparison.impactedUnchangedFiles .slice(0, 3) - .map((file) => `${file.path} ${formatCoverageDelta(file.delta)}`) + .map((file) => + file.delta === null + ? `${file.path} ${coverageValueLabel(file.base)} → ${coverageValueLabel(file.task)}` + : `${file.path} ${formatCoverageDelta(file.delta)}`, + ) .join(', '); lines.push( `${comparison.impactedUnchangedFiles.length} materially impacted unchanged file${comparison.impactedUnchangedFiles.length === 1 ? '' : 's'}: ${impacted}.`, @@ -835,9 +851,12 @@ export function ChangedFilesList(props: ChangedFilesListProps) { }} > 0 - } + when={shouldShowCoverageFooter( + props.selectedCommit, + coverageCandidateFiles().length, + hasCoverageArtifact(), + hasBaseCoverageArtifact(), + )} >
{ delta: -4, }, files: {}, - impactedUnchangedFiles: [{ path: 'src/shared.ts', taskPct: 70, basePct: 80, delta: -10 }], + impactedUnchangedFiles: [ + { + path: 'src/shared.ts', + task: { state: 'available', pct: 70 }, + base: { state: 'available', pct: 80 }, + delta: -10, + }, + ], }, }), ); @@ -210,7 +217,14 @@ describe('buildMergeReadiness', () => { delta: 2, }, files: {}, - impactedUnchangedFiles: [{ path: 'src/shared.ts', taskPct: 70, basePct: 80, delta: -10 }], + impactedUnchangedFiles: [ + { + path: 'src/shared.ts', + task: { state: 'available', pct: 70 }, + base: { state: 'available', pct: 80 }, + delta: -10, + }, + ], }, }), ); diff --git a/src/components/merge-readiness.ts b/src/components/merge-readiness.ts index 85e52db0..1003e4f9 100644 --- a/src/components/merge-readiness.ts +++ b/src/components/merge-readiness.ts @@ -179,7 +179,11 @@ function coverageCheck(coverage?: CoverageComparison | null): MergeReadinessChec }; } - const regressedUnchanged = coverage.impactedUnchangedFiles.filter((file) => file.delta < 0); + const regressedUnchanged = coverage.impactedUnchangedFiles.filter( + (file) => + file.base.state === 'available' && + (file.task.state !== 'available' || (file.delta !== null && file.delta < 0)), + ); const impactedDetail = regressedUnchanged.length > 0 ? ` ${countLabel(regressedUnchanged.length, 'unchanged file')} also regressed.` diff --git a/src/lib/coverage-comparison.test.ts b/src/lib/coverage-comparison.test.ts index 287c0846..1fc51774 100644 --- a/src/lib/coverage-comparison.test.ts +++ b/src/lib/coverage-comparison.test.ts @@ -143,12 +143,54 @@ describe('buildCoverageComparison', () => { expect(result.impactedUnchangedFiles).toEqual([ { path: 'src/regressed.ts', - taskPct: 82, - basePct: 90, + task: { state: 'available', pct: 82 }, + base: { state: 'available', pct: 90 }, delta: -8, }, ]); }); + + it('retains base-only unchanged files as unavailable task coverage', () => { + const base = report(80, [file('src/base-only.ts', 40)]); + const task = report(85, []); + + const result = buildCoverageComparison(task, base, []); + + expect(result.impactedUnchangedFiles).toEqual([ + { + path: 'src/base-only.ts', + task: { state: 'file-not-present', pct: null }, + base: { state: 'available', pct: 40 }, + delta: null, + }, + ]); + }); + + it('retains available-to-no-lines transitions for unchanged files', () => { + const base = report(80, [file('src/no-lines-now.ts', 75)]); + const task = report(85, [file('src/no-lines-now.ts', 100, 0)]); + + const result = buildCoverageComparison(task, base, []); + + expect(result.impactedUnchangedFiles).toEqual([ + { + path: 'src/no-lines-now.ts', + task: { state: 'no-executable-lines', pct: null }, + base: { state: 'available', pct: 75 }, + delta: null, + }, + ]); + }); + + it('does not classify non-source Git-changed paths as unchanged coverage impacts', () => { + const base = report(80, [file('src/example.test.ts', 90)]); + const task = report(80, [file('src/example.test.ts', 60)]); + + const result = buildCoverageComparison(task, base, [changed('src/example.test.ts')]); + + expect(result.files['src/example.test.ts'].delta).toBe(-30); + expect(result.impactedUnchangedFiles).toEqual([]); + }); }); describe('formatCoverageDelta', () => { diff --git a/src/lib/coverage-comparison.ts b/src/lib/coverage-comparison.ts index 77e00e10..e9fcb39e 100644 --- a/src/lib/coverage-comparison.ts +++ b/src/lib/coverage-comparison.ts @@ -24,9 +24,9 @@ export interface CoverageFileComparison { export interface ImpactedCoverageFile { path: string; - taskPct: number; - basePct: number; - delta: number; + task: CoverageValue; + base: CoverageValue; + delta: number | null; } export interface CoverageComparison { @@ -113,23 +113,31 @@ export function buildCoverageComparison( const impactedUnchangedFiles: ImpactedCoverageFile[] = []; if (taskSummary && baseSummary) { - for (const [filePath, taskFile] of Object.entries(taskSummary.files)) { + const reportPaths = new Set([ + ...Object.keys(taskSummary.files), + ...Object.keys(baseSummary.files), + ]); + for (const filePath of reportPaths) { if (changedPaths.has(filePath)) continue; - const baseFile = baseSummary.files[filePath]; - if (!baseFile || taskFile.lines.total === 0 || baseFile.lines.total === 0) continue; - const delta = roundPercentage(taskFile.lines.pct - baseFile.lines.pct); - if (Math.abs(delta) < MATERIAL_COVERAGE_DELTA) continue; + const task = fileValue(taskSummary, filePath); + const base = fileValue(baseSummary, filePath); + const delta = coverageDelta(task, base); + if (task.state === base.state && delta === null) continue; + if (delta !== null && Math.abs(delta) < MATERIAL_COVERAGE_DELTA) continue; impactedUnchangedFiles.push({ path: filePath, - taskPct: taskFile.lines.pct, - basePct: baseFile.lines.pct, + task, + base, delta, }); } } impactedUnchangedFiles.sort( - (a, b) => Math.abs(b.delta) - Math.abs(a.delta) || a.path.localeCompare(b.path), + (a, b) => + Number(b.delta === null) - Number(a.delta === null) || + Math.abs(b.delta ?? 0) - Math.abs(a.delta ?? 0) || + a.path.localeCompare(b.path), ); return { From af12493b9815444c9dace4d22c21dd6a9f385f43 Mon Sep 17 00:00:00 2001 From: Liang Hu Date: Mon, 27 Jul 2026 13:08:48 -0400 Subject: [PATCH 03/10] fix: use canonical coverage comparison inventory --- src/components/ChangedFilesList.test.ts | 30 ++++++++++++ src/components/ChangedFilesList.tsx | 65 +++++++++++++++++++------ src/lib/coverage-comparison.test.ts | 14 ++++++ src/lib/coverage-comparison.ts | 15 +++++- 4 files changed, 107 insertions(+), 17 deletions(-) diff --git a/src/components/ChangedFilesList.test.ts b/src/components/ChangedFilesList.test.ts index 0b9490d9..99607020 100644 --- a/src/components/ChangedFilesList.test.ts +++ b/src/components/ChangedFilesList.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest'; import type { ChangedFile } from '../ipc/types'; import { + buildCoverageComparisonForSelection, coverageFooterLabel, coverageFooterTitle, filesFooterLabel, @@ -80,6 +81,35 @@ describe('shouldShowCoverageFooter', () => { }); }); +describe('buildCoverageComparisonForSelection', () => { + it.each(['pending', 'failed'])( + 'suppresses comparison output while the canonical changed-file inventory is %s', + () => { + expect(buildCoverageComparisonForSelection(undefined, null, null, null)).toBeNull(); + }, + ); + + it('uses the canonical whole-task inventory for an uncommitted-only display selection', () => { + const committed = changedFile({ path: 'src/committed.ts', committed: true }); + const localOnly = changedFile({ path: 'src/local-only.ts', committed: false }); + + const result = buildCoverageComparisonForSelection('uncommitted', null, null, [ + committed, + localOnly, + ]); + + expect(Object.keys(result?.files ?? {})).toEqual([committed.path, localOnly.path]); + }); + + it('suppresses whole-task comparison for a single-commit selection', () => { + expect( + buildCoverageComparisonForSelection('abc123', null, null, [ + changedFile({ path: 'src/example.ts' }), + ]), + ).toBeNull(); + }); +}); + describe('filesFooterLabel', () => { it('shows only total files when everything is committed', () => { expect(filesFooterLabel(7, 0)).toBe('▤ 7'); diff --git a/src/components/ChangedFilesList.tsx b/src/components/ChangedFilesList.tsx index 4cb29361..c20f9668 100644 --- a/src/components/ChangedFilesList.tsx +++ b/src/components/ChangedFilesList.tsx @@ -7,7 +7,7 @@ import { getStatusColor } from '../lib/status-colors'; import { openFileInEditor } from '../lib/shell'; import { buildFileTree, flattenVisibleTree } from '../lib/file-tree'; import { - buildCoverageComparison, + buildCoverageComparisonIfReady, formatCoverageDelta, type CoverageComparison, type CoverageFileComparison, @@ -73,6 +73,16 @@ export function shouldShowCoverageFooter( ); } +export function buildCoverageComparisonForSelection( + selectedCommit: CommitSelection | undefined, + taskSummary: CoverageSummary | null, + baseSummary: CoverageSummary | null, + comparisonFiles: ChangedFile[] | null, +): CoverageComparison | null { + if (isCommitHashSelection(selectedCommit)) return null; + return buildCoverageComparisonIfReady(taskSummary, baseSummary, comparisonFiles); +} + export function coverageFooterLabel( hasCoverageArtifact: boolean, touchedCoveragePct: number | null, @@ -304,6 +314,7 @@ function OpenInEditorButton(props: { export function ChangedFilesList(props: ChangedFilesListProps) { const [files, setFiles] = createSignal([]); + const [comparisonFiles, setComparisonFiles] = createSignal(null); const [coverage, setCoverage] = createSignal(null); const [baseCoverage, setBaseCoverage] = createSignal(null); const [canOpenFilesInEditor, setCanOpenFilesInEditor] = createSignal(false); @@ -321,7 +332,12 @@ export function ChangedFilesList(props: ChangedFilesListProps) { ); const eligibleFiles = createMemo(() => files().filter((file) => isCoverageEligible(file))); const coverageComparison = createMemo(() => - buildCoverageComparison(coverage(), baseCoverage(), files()), + buildCoverageComparisonForSelection( + props.selectedCommit, + coverage(), + baseCoverage(), + comparisonFiles(), + ), ); const coveredEligibleFiles = createMemo(() => eligibleFiles().filter((file) => Boolean(coverageFiles()[file.path])), @@ -349,7 +365,8 @@ export function ChangedFilesList(props: ChangedFilesListProps) { return Math.round((coveredLines / totalLines) * 100); }); const aggregateCoverageLabel = createMemo(() => { - const comparison = coverageComparison().aggregate; + const comparison = coverageComparison()?.aggregate; + if (!comparison) return null; if (!hasBaseCoverageArtifact()) return null; const delta = comparison.delta === null ? '' : ` (${formatCoverageDelta(comparison.delta)})`; return `base ${coverageValueLabel(comparison.base)} → task ${coverageValueLabel(comparison.task)}${delta}`; @@ -358,7 +375,7 @@ export function ChangedFilesList(props: ChangedFilesListProps) { const comparison = coverageComparison(); const taskReport = coverage(); const baseReport = baseCoverage(); - if (!baseReport) return ''; + if (!baseReport || !comparison) return ''; const lines = [ `Base: ${coverageValueLabel(comparison.aggregate.base)} (${baseReport.reportPath}).`, taskReport @@ -386,9 +403,7 @@ export function ChangedFilesList(props: ChangedFilesListProps) { createEffect(() => { if (!props.onCoverageComparisonChange) return; - props.onCoverageComparisonChange( - isCommitHashSelection(props.selectedCommit) ? null : coverageComparison(), - ); + props.onCoverageComparisonChange(coverageComparison()); }); function toggleDir(path: string) { @@ -508,6 +523,7 @@ export function ChangedFilesList(props: ChangedFilesListProps) { let inFlight = false; let usingBranchFallback = false; setCanOpenFilesInEditor(false); + setComparisonFiles(null); async function refresh() { if (inFlight) return; @@ -534,6 +550,16 @@ export function ChangedFilesList(props: ChangedFilesListProps) { } if (uncommittedOnly && path) { + const comparisonRequest = invoke(IPC.GetChangedFiles, { + worktreePath: path, + baseBranch, + }) + .then((result) => { + if (!cancelled) setComparisonFiles(result); + }) + .catch(() => { + if (!cancelled) setComparisonFiles(null); + }); try { const result = await invoke(IPC.GetUncommittedChangedFiles, { worktreePath: path, @@ -548,6 +574,7 @@ export function ChangedFilesList(props: ChangedFilesListProps) { setCanOpenFilesInEditor(false); } } + await comparisonRequest; return; } @@ -560,11 +587,15 @@ export function ChangedFilesList(props: ChangedFilesListProps) { }); if (!cancelled) { setFiles(result); + setComparisonFiles(result); setCanOpenFilesInEditor(true); } return; } catch { - if (!cancelled) setCanOpenFilesInEditor(false); + if (!cancelled) { + setComparisonFiles(null); + setCanOpenFilesInEditor(false); + } // Worktree may not exist — try branch fallback below } } @@ -579,11 +610,15 @@ export function ChangedFilesList(props: ChangedFilesListProps) { baseBranch, }); if (!cancelled) { - setFiles(uncommittedOnly ? result.filter((f) => !f.committed) : result); + setFiles(result); + setComparisonFiles(result); setCanOpenFilesInEditor(false); } } catch { - if (!cancelled) setCanOpenFilesInEditor(false); + if (!cancelled) { + setComparisonFiles(null); + setCanOpenFilesInEditor(false); + } // Branch may no longer exist } } @@ -801,7 +836,7 @@ export function ChangedFilesList(props: ChangedFilesListProps) { file={file} selectedCommit={props.selectedCommit} summary={coverageFiles()[row().node.path]} - comparison={coverageComparison().files[row().node.path]} + comparison={coverageComparison()?.files[row().node.path]} hasCoverageArtifact={hasCoverageArtifact()} /> )} @@ -872,9 +907,9 @@ export function ChangedFilesList(props: ChangedFilesListProps) { title={aggregateCoverageTitle()} style={{ color: - coverageComparison().aggregate.delta === null + coverageComparison()?.aggregate.delta === null ? theme.fgMuted - : deltaColor(coverageComparison().aggregate.delta ?? 0), + : deltaColor(coverageComparison()?.aggregate.delta ?? 0), 'font-weight': '600', }} > @@ -932,12 +967,12 @@ export function ChangedFilesList(props: ChangedFilesListProps) { ∅ {missingCoverageCount()} - 0}> + 0}> - ↕ {coverageComparison().impactedUnchangedFiles.length} other + ↕ {coverageComparison()?.impactedUnchangedFiles.length} other
diff --git a/src/lib/coverage-comparison.test.ts b/src/lib/coverage-comparison.test.ts index 1fc51774..4dd47551 100644 --- a/src/lib/coverage-comparison.test.ts +++ b/src/lib/coverage-comparison.test.ts @@ -191,6 +191,20 @@ describe('buildCoverageComparison', () => { expect(result.files['src/example.test.ts'].delta).toBe(-30); expect(result.impactedUnchangedFiles).toEqual([]); }); + + it.each(['toString', 'constructor', '__proto__'])( + 'treats a missing report entry named %s as absent instead of reading Object.prototype', + (path) => { + const result = buildCoverageComparison(report(80, []), report(80, []), [changed(path)]); + + expect(Object.getPrototypeOf(result.files)).toBeNull(); + expect(result.files[path]).toMatchObject({ + path, + task: { state: 'file-not-present', pct: null }, + base: { state: 'file-not-present', pct: null }, + }); + }, + ); }); describe('formatCoverageDelta', () => { diff --git a/src/lib/coverage-comparison.ts b/src/lib/coverage-comparison.ts index e9fcb39e..913cd526 100644 --- a/src/lib/coverage-comparison.ts +++ b/src/lib/coverage-comparison.ts @@ -59,7 +59,8 @@ function fileValue( forceMissing = false, ): CoverageValue { if (!summary) return { state: 'no-report', pct: null }; - const file = forceMissing ? undefined : summary.files[filePath]; + const file = + forceMissing || !Object.hasOwn(summary.files, filePath) ? undefined : summary.files[filePath]; if (!file) return { state: 'file-not-present', pct: null }; if (file.lines.total === 0) return { state: 'no-executable-lines', pct: null }; return { state: 'available', pct: file.lines.pct }; @@ -90,7 +91,7 @@ export function buildCoverageComparison( ): CoverageComparison { const taskAggregate = aggregateValue(taskSummary); const baseAggregate = aggregateValue(baseSummary); - const files: Record = {}; + const files = Object.create(null) as Record; const changedPaths = new Set(); for (const file of changedFiles) { @@ -150,3 +151,13 @@ export function buildCoverageComparison( impactedUnchangedFiles, }; } + +export function buildCoverageComparisonIfReady( + taskSummary: CoverageSummary | null, + baseSummary: CoverageSummary | null, + changedFiles: ChangedFile[] | null, +): CoverageComparison | null { + return changedFiles === null + ? null + : buildCoverageComparison(taskSummary, baseSummary, changedFiles); +} From 5aa4d4faf51b5a5555e5f7625c56c2a144ed264c Mon Sep 17 00:00:00 2001 From: Liang Hu Date: Tue, 28 Jul 2026 09:05:35 -0400 Subject: [PATCH 04/10] fix: validate coverage comparison freshness --- electron/ipc/channel-manifest.json | 1 + electron/ipc/git.test.ts | 28 +++ electron/ipc/git.ts | 41 ++-- electron/ipc/register.ts | 4 + electron/preload.cjs | 1 + src/components/ChangedFilesList.test.ts | 41 ---- src/components/ChangedFilesList.tsx | 218 +++++++++++++-------- src/components/MergeReadinessPanel.test.ts | 71 +++++++ src/components/merge-readiness.ts | 18 +- src/lib/coverage-comparison.test.ts | 28 +++ src/lib/coverage-comparison.ts | 26 ++- 11 files changed, 327 insertions(+), 150 deletions(-) diff --git a/electron/ipc/channel-manifest.json b/electron/ipc/channel-manifest.json index 1d9316aa..cadacaa3 100644 --- a/electron/ipc/channel-manifest.json +++ b/electron/ipc/channel-manifest.json @@ -19,6 +19,7 @@ "GetGitignoredDirs": "get_gitignored_dirs", "ListImportableWorktrees": "list_importable_worktrees", "GetBranchWorktreePath": "get_branch_worktree_path", + "GetMergeBaseTimestamp": "get_merge_base_timestamp", "GetWorktreeStatus": "get_worktree_status", "CheckMergeStatus": "check_merge_status", "MergeTask": "merge_task", diff --git a/electron/ipc/git.test.ts b/electron/ipc/git.test.ts index 76f1b417..43b4da1b 100644 --- a/electron/ipc/git.test.ts +++ b/electron/ipc/git.test.ts @@ -60,6 +60,7 @@ import { getUncommittedChangedFiles, checkMergeStatus, getBranchWorktreePath, + getMergeBaseTimestamp, listImportableWorktrees, mergeTask, } from './git.js'; @@ -443,6 +444,7 @@ function uniqueWorktreePath(): string { */ function buildWorktreeMockHandler(opts: { mergeBase?: string; + mergeBaseTimestamp?: string; finalRawNumstat?: string; committedRawNumstat?: string; uncommittedRawNumstat?: string; @@ -592,6 +594,11 @@ function buildWorktreeMockHandler(opts: { return; } + if (cmd === 'show' && args.includes('--format=%cI')) { + cb(null, `${opts.mergeBaseTimestamp ?? ''}\n`, ''); + return; + } + // git status --porcelain if (cmd === 'status' && args.includes('--porcelain')) { cb(null, opts.statusPorcelain ?? '', ''); @@ -1500,6 +1507,27 @@ describe('getBranchWorktreePath', () => { }); }); +describe('getMergeBaseTimestamp', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('returns the picked merge-base commit time as ISO UTC', async () => { + const calls: string[][] = []; + setupMock( + calls, + buildWorktreeMockHandler({ + mergeBaseTimestamp: '2026-07-25T12:00:00-04:00', + }), + ); + + await expect(getMergeBaseTimestamp(uniqueWorktreePath(), 'main')).resolves.toBe( + '2026-07-25T16:00:00.000Z', + ); + expect(calls).toContainEqual(['show', '-s', '--format=%cI', MERGE_BASE]); + }); +}); + // --------------------------------------------------------------------------- // checkMergeStatus — main-ahead count uses cherry-pick filtering so rebased // patch-equivalent commits in main don't trigger a needless rebase prompt. diff --git a/electron/ipc/git.ts b/electron/ipc/git.ts index 87b46af7..d31c37b6 100644 --- a/electron/ipc/git.ts +++ b/electron/ipc/git.ts @@ -536,28 +536,21 @@ function normalizeStatusPath(raw: string): string { return trimmed.replace(/^"|"$/g, '').replace(/\\(.)/g, '$1'); } -function parseNumstatPath(raw: string): { path: string; previousPath?: string } { +function parseNumstatPath(raw: string): string { const trimmed = raw.trim(); const arrowIndex = trimmed.indexOf(' => '); - if (arrowIndex < 0) return { path: normalizeStatusPath(trimmed) }; + if (arrowIndex < 0) return normalizeStatusPath(trimmed); const openBrace = trimmed.lastIndexOf('{', arrowIndex); const closeBrace = trimmed.indexOf('}', arrowIndex); if (openBrace >= 0 && closeBrace > arrowIndex) { const prefix = trimmed.slice(0, openBrace); const suffix = trimmed.slice(closeBrace + 1); - const previousPath = `${prefix}${trimmed.slice(openBrace + 1, arrowIndex)}${suffix}`; const destinationPath = `${prefix}${trimmed.slice(arrowIndex + 4, closeBrace)}${suffix}`; - return { - path: normalizeStatusPath(destinationPath), - previousPath: normalizeStatusPath(previousPath), - }; + return normalizeStatusPath(destinationPath); } - return { - path: normalizeStatusPath(trimmed.slice(arrowIndex + 4)), - previousPath: normalizeStatusPath(trimmed.slice(0, arrowIndex)), - }; + return normalizeStatusPath(trimmed.slice(arrowIndex + 4)); } /** Parse combined `git diff --raw --numstat` output into status and numstat maps. */ @@ -594,12 +587,8 @@ function parseDiffRawNumstat(output: string): { if (!isNaN(added) && !isNaN(removed)) { const rawPath = parts[parts.length - 1]; const normalizedPath = normalizeStatusPath(rawPath); - const parsedPath = statusMap.has(normalizedPath) - ? { path: normalizedPath } - : parseNumstatPath(rawPath); - const p = parsedPath.path; + const p = statusMap.has(normalizedPath) ? normalizedPath : parseNumstatPath(rawPath); if (p) numstatMap.set(p, [added, removed]); - if (p && parsedPath.previousPath) previousPathMap.set(p, parsedPath.previousPath); } } } @@ -1601,6 +1590,26 @@ export async function getBranchWorktreePath( return match?.path ?? null; } +/** Return the task branch's merge-base commit time without mutating a worktree. */ +export async function getMergeBaseTimestamp( + worktreePath: string, + baseBranch?: string, +): Promise { + try { + const branch = baseBranch ?? (await detectMainBranch(worktreePath)); + const head = await pinHead(worktreePath); + const picked = await pickMergeBase(worktreePath, branch, head); + const mergeBase = picked?.sha ?? head; + const { stdout } = await exec('git', ['show', '-s', '--format=%cI', mergeBase], { + cwd: worktreePath, + }); + const timestamp = new Date(stdout.trim()); + return Number.isNaN(timestamp.getTime()) ? null : timestamp.toISOString(); + } catch { + return null; + } +} + /** Stage all changes and commit in a worktree. */ export async function commitAll(worktreePath: string, message: string): Promise { await exec('git', ['add', '-A'], { cwd: worktreePath }); diff --git a/electron/ipc/register.ts b/electron/ipc/register.ts index c85d92de..0f581fae 100644 --- a/electron/ipc/register.ts +++ b/electron/ipc/register.ts @@ -55,6 +55,7 @@ import { getWorktreeStatus, listImportableWorktrees, getBranchWorktreePath, + getMergeBaseTimestamp, commitAll, discardUncommitted, checkMergeStatus, @@ -607,6 +608,9 @@ export function registerAllHandlers(win: BrowserWindow): void { ipcMain.handle(IPC.GetBranchWorktreePath, (_e, args) => { return getBranchWorktreePath(projectRootArg(args), branchNameArg(args)); }); + ipcMain.handle(IPC.GetMergeBaseTimestamp, (_e, args) => { + return getMergeBaseTimestamp(worktreePathArg(args), optionalBaseBranch(args)); + }); ipcMain.handle(IPC.GetWorktreeStatus, (_e, args) => { const worktreePath = worktreePathArg(args); return getWorktreeStatus(worktreePath, optionalBaseBranch(args)); diff --git a/electron/preload.cjs b/electron/preload.cjs index 1ce97c81..48044ade 100644 --- a/electron/preload.cjs +++ b/electron/preload.cjs @@ -23,6 +23,7 @@ const ALLOWED_CHANNELS = new Set([ 'get_gitignored_dirs', 'list_importable_worktrees', 'get_branch_worktree_path', + 'get_merge_base_timestamp', 'get_worktree_status', 'check_merge_status', 'merge_task', diff --git a/src/components/ChangedFilesList.test.ts b/src/components/ChangedFilesList.test.ts index 99607020..fef489b8 100644 --- a/src/components/ChangedFilesList.test.ts +++ b/src/components/ChangedFilesList.test.ts @@ -1,13 +1,11 @@ import { describe, expect, it } from 'vitest'; import type { ChangedFile } from '../ipc/types'; import { - buildCoverageComparisonForSelection, coverageFooterLabel, coverageFooterTitle, filesFooterLabel, filesFooterTitle, isCoverageEligible, - shouldShowCoverageFooter, } from './ChangedFilesList'; function changedFile(overrides: Partial): ChangedFile { @@ -71,45 +69,6 @@ describe('coverageFooterLabel', () => { }); }); -describe('shouldShowCoverageFooter', () => { - it('shows comparison results for test-only changes when reports are loaded', () => { - expect(shouldShowCoverageFooter(undefined, 0, true, true)).toBe(true); - }); - - it('hides coverage details for a single-commit selection', () => { - expect(shouldShowCoverageFooter('abc123', 1, true, true)).toBe(false); - }); -}); - -describe('buildCoverageComparisonForSelection', () => { - it.each(['pending', 'failed'])( - 'suppresses comparison output while the canonical changed-file inventory is %s', - () => { - expect(buildCoverageComparisonForSelection(undefined, null, null, null)).toBeNull(); - }, - ); - - it('uses the canonical whole-task inventory for an uncommitted-only display selection', () => { - const committed = changedFile({ path: 'src/committed.ts', committed: true }); - const localOnly = changedFile({ path: 'src/local-only.ts', committed: false }); - - const result = buildCoverageComparisonForSelection('uncommitted', null, null, [ - committed, - localOnly, - ]); - - expect(Object.keys(result?.files ?? {})).toEqual([committed.path, localOnly.path]); - }); - - it('suppresses whole-task comparison for a single-commit selection', () => { - expect( - buildCoverageComparisonForSelection('abc123', null, null, [ - changedFile({ path: 'src/example.ts' }), - ]), - ).toBeNull(); - }); -}); - describe('filesFooterLabel', () => { it('shows only total files when everything is committed', () => { expect(filesFooterLabel(7, 0)).toBe('▤ 7'); diff --git a/src/components/ChangedFilesList.tsx b/src/components/ChangedFilesList.tsx index c20f9668..d21c9f6b 100644 --- a/src/components/ChangedFilesList.tsx +++ b/src/components/ChangedFilesList.tsx @@ -7,7 +7,7 @@ import { getStatusColor } from '../lib/status-colors'; import { openFileInEditor } from '../lib/shell'; import { buildFileTree, flattenVisibleTree } from '../lib/file-tree'; import { - buildCoverageComparisonIfReady, + buildCoverageComparison, formatCoverageDelta, type CoverageComparison, type CoverageFileComparison, @@ -61,26 +61,31 @@ export function isCoverageEligible(file: ChangedFile): boolean { return file.status !== 'D' && isCoverageCandidate(file); } -export function shouldShowCoverageFooter( - selectedCommit: CommitSelection | undefined, - coverageCandidateCount: number, - hasCoverageArtifact: boolean, - hasBaseCoverageArtifact: boolean, -): boolean { - return ( - !isCommitHashSelection(selectedCommit) && - (coverageCandidateCount > 0 || hasCoverageArtifact || hasBaseCoverageArtifact) - ); +function sameChangedFiles(left: ChangedFile[] | null, right: ChangedFile[] | null): boolean { + if (left === right) return true; + if (!left || !right || left.length !== right.length) return false; + return left.every((file, index) => { + const other = right[index]; + return ( + file.path === other.path && + file.previous_path === other.previous_path && + file.lines_added === other.lines_added && + file.lines_removed === other.lines_removed && + file.status === other.status && + file.committed === other.committed + ); + }); } -export function buildCoverageComparisonForSelection( - selectedCommit: CommitSelection | undefined, - taskSummary: CoverageSummary | null, - baseSummary: CoverageSummary | null, - comparisonFiles: ChangedFile[] | null, -): CoverageComparison | null { - if (isCommitHashSelection(selectedCommit)) return null; - return buildCoverageComparisonIfReady(taskSummary, baseSummary, comparisonFiles); +function sameCoverageSummary(left: CoverageSummary | null, right: CoverageSummary | null): boolean { + return ( + left === right || + (left !== null && + right !== null && + left.format === right.format && + left.reportPath === right.reportPath && + left.generatedAt === right.generatedAt) + ); } export function coverageFooterLabel( @@ -317,6 +322,7 @@ export function ChangedFilesList(props: ChangedFilesListProps) { const [comparisonFiles, setComparisonFiles] = createSignal(null); const [coverage, setCoverage] = createSignal(null); const [baseCoverage, setBaseCoverage] = createSignal(null); + const [mergeBaseAt, setMergeBaseAt] = createSignal(null); const [canOpenFilesInEditor, setCanOpenFilesInEditor] = createSignal(false); const [selectedIndex, setSelectedIndex] = createSignal(-1); const [collapsed, setCollapsed] = createSignal>(new Set()); @@ -327,18 +333,19 @@ export function ChangedFilesList(props: ChangedFilesListProps) { const coverageFiles = createMemo(() => coverage()?.files ?? {}); const hasCoverageArtifact = createMemo(() => coverage() !== null); const hasBaseCoverageArtifact = createMemo(() => baseCoverage() !== null); - const coverageCandidateFiles = createMemo(() => - files().filter((file) => isCoverageCandidate(file)), - ); const eligibleFiles = createMemo(() => files().filter((file) => isCoverageEligible(file))); - const coverageComparison = createMemo(() => - buildCoverageComparisonForSelection( - props.selectedCommit, - coverage(), - baseCoverage(), - comparisonFiles(), - ), + const showCoverageFooter = createMemo( + () => + !isCommitHashSelection(props.selectedCommit) && + (files().some((file) => isCoverageCandidate(file)) || + hasCoverageArtifact() || + hasBaseCoverageArtifact()), ); + const coverageComparison = createMemo(() => { + const inventory = comparisonFiles(); + if (!inventory || isCommitHashSelection(props.selectedCommit)) return null; + return buildCoverageComparison(coverage(), baseCoverage(), inventory, mergeBaseAt()); + }); const coveredEligibleFiles = createMemo(() => eligibleFiles().filter((file) => Boolean(coverageFiles()[file.path])), ); @@ -377,14 +384,22 @@ export function ChangedFilesList(props: ChangedFilesListProps) { const baseReport = baseCoverage(); if (!baseReport || !comparison) return ''; const lines = [ - `Base: ${coverageValueLabel(comparison.aggregate.base)} (${baseReport.reportPath}).`, + `Base: ${coverageValueLabel(comparison.aggregate.base)} (${baseReport.reportPath}, updated ${baseReport.generatedAt ?? 'unknown time'}).`, taskReport - ? `Task: ${coverageValueLabel(comparison.aggregate.task)} (${taskReport.reportPath}).` + ? `Task: ${coverageValueLabel(comparison.aggregate.task)} (${taskReport.reportPath}, updated ${taskReport.generatedAt ?? 'unknown time'}).` : 'Task: no coverage report.', ]; if (comparison.aggregate.delta !== null) { lines.push(`Delta: ${formatCoverageDelta(comparison.aggregate.delta)}.`); } + if (comparison.baseline) { + lines.push(`Task merge-base: ${comparison.baseline.mergeBaseAt}.`); + if (comparison.baseline.stale) { + lines.push( + 'The base report predates the merge-base, so merge readiness ignores the delta.', + ); + } + } if (comparison.impactedUnchangedFiles.length > 0) { const impacted = comparison.impactedUnchangedFiles .slice(0, 3) @@ -511,6 +526,13 @@ export function ChangedFilesList(props: ChangedFilesListProps) { // wouldn't otherwise activate the task and trigger a fetch. Polling at 5s // (matching git status) is still gated on isActive to avoid running git // pipelines for every off-screen task. + createEffect(() => { + void props.worktreePath; + void props.baseBranch; + void props.selectedCommit; + setComparisonFiles(null); + }); + createEffect(() => { const path = props.worktreePath; const projectRoot = props.projectRoot; @@ -519,11 +541,11 @@ export function ChangedFilesList(props: ChangedFilesListProps) { const selection = props.selectedCommit; const singleCommitHash = isCommitHashSelection(selection) ? selection : null; const uncommittedOnly = isUncommittedSelection(selection); + const comparisonEnabled = hasCoverageArtifact() || hasBaseCoverageArtifact(); let cancelled = false; let inFlight = false; let usingBranchFallback = false; setCanOpenFilesInEditor(false); - setComparisonFiles(null); async function refresh() { if (inFlight) return; @@ -537,7 +559,7 @@ export function ChangedFilesList(props: ChangedFilesListProps) { commitHash: singleCommitHash, }); if (!cancelled) { - setFiles(result); + setFiles((current) => (sameChangedFiles(current, result) ? current : result)); setCanOpenFilesInEditor(true); } } catch { @@ -550,22 +572,29 @@ export function ChangedFilesList(props: ChangedFilesListProps) { } if (uncommittedOnly && path) { - const comparisonRequest = invoke(IPC.GetChangedFiles, { - worktreePath: path, - baseBranch, - }) - .then((result) => { - if (!cancelled) setComparisonFiles(result); - }) - .catch(() => { - if (!cancelled) setComparisonFiles(null); - }); + if (!comparisonEnabled && !cancelled) setComparisonFiles(null); + const comparisonRequest = comparisonEnabled + ? invoke(IPC.GetChangedFiles, { + worktreePath: path, + baseBranch, + }) + .then((result) => { + if (!cancelled) { + setComparisonFiles((current) => + sameChangedFiles(current, result) ? current : result, + ); + } + }) + .catch(() => { + if (!cancelled) setComparisonFiles(null); + }) + : Promise.resolve(); try { const result = await invoke(IPC.GetUncommittedChangedFiles, { worktreePath: path, }); if (!cancelled) { - setFiles(result); + setFiles((current) => (sameChangedFiles(current, result) ? current : result)); setCanOpenFilesInEditor(true); } } catch { @@ -586,8 +615,10 @@ export function ChangedFilesList(props: ChangedFilesListProps) { baseBranch, }); if (!cancelled) { - setFiles(result); - setComparisonFiles(result); + setFiles((current) => (sameChangedFiles(current, result) ? current : result)); + setComparisonFiles((current) => + sameChangedFiles(current, result) ? current : result, + ); setCanOpenFilesInEditor(true); } return; @@ -610,8 +641,15 @@ export function ChangedFilesList(props: ChangedFilesListProps) { baseBranch, }); if (!cancelled) { - setFiles(result); - setComparisonFiles(result); + const displayedFiles = uncommittedOnly + ? result.filter((file) => !file.committed) + : result; + setFiles((current) => + sameChangedFiles(current, displayedFiles) ? current : displayedFiles, + ); + setComparisonFiles((current) => + sameChangedFiles(current, result) ? current : result, + ); setCanOpenFilesInEditor(false); } } catch { @@ -651,42 +689,67 @@ export function ChangedFilesList(props: ChangedFilesListProps) { batch(() => { setCoverage(null); setBaseCoverage(null); + setMergeBaseAt(null); }); return; } if (!props.isActive) return; let cancelled = false; let inFlight = false; - const baseBranchPromise = baseBranch - ? Promise.resolve(baseBranch) - : projectRoot - ? invoke(IPC.GetMainBranch, { projectRoot }) - : Promise.resolve(null); + let cachedMergeBase: + | { + taskGeneratedAt: string; + value: string | null; + } + | undefined; async function refresh() { if (inFlight) return; inFlight = true; try { - const resolvedBaseBranch = await baseBranchPromise.catch(() => null); - const baseRoot = resolvedBaseBranch - ? await resolveBaseCoverageRoot(projectRoot, resolvedBaseBranch, repoRoot) - : null; - const [taskResult, baseResult] = await Promise.all([ - invoke(IPC.GetCoverageSummary, { - repoRoot, - reportPath: props.coverageReportPath, - }).catch(() => null), - baseRoot - ? invoke(IPC.GetCoverageSummary, { - repoRoot: baseRoot, - reportPath: props.coverageReportPath, - }).catch(() => null) - : Promise.resolve(null), - ]); + const taskResult = await invoke(IPC.GetCoverageSummary, { + repoRoot, + reportPath: props.coverageReportPath, + }).catch(() => null); + let baseResult: CoverageSummary | null = null; + let mergeBaseResult: string | null = null; + if (taskResult) { + const resolvedBaseBranch = baseBranch + ? baseBranch + : projectRoot + ? await invoke(IPC.GetMainBranch, { projectRoot }).catch(() => null) + : null; + const baseRoot = resolvedBaseBranch + ? await resolveBaseCoverageRoot(projectRoot, resolvedBaseBranch, repoRoot) + : null; + if (resolvedBaseBranch) { + if (!cachedMergeBase || cachedMergeBase.taskGeneratedAt !== taskResult.generatedAt) { + cachedMergeBase = { + taskGeneratedAt: taskResult.generatedAt, + value: await invoke(IPC.GetMergeBaseTimestamp, { + worktreePath: repoRoot, + baseBranch: resolvedBaseBranch, + }).catch(() => null), + }; + } + mergeBaseResult = cachedMergeBase.value; + } + if (baseRoot) { + baseResult = await invoke(IPC.GetCoverageSummary, { + repoRoot: baseRoot, + reportPath: props.coverageReportPath, + }).catch(() => null); + } + } if (!cancelled) { batch(() => { - setCoverage(taskResult); - setBaseCoverage(baseResult); + setCoverage((current) => + sameCoverageSummary(current, taskResult) ? current : taskResult, + ); + setBaseCoverage((current) => + sameCoverageSummary(current, baseResult) ? current : baseResult, + ); + setMergeBaseAt(mergeBaseResult); }); } } finally { @@ -885,14 +948,7 @@ export function ChangedFilesList(props: ChangedFilesListProps) { 'flex-wrap': 'wrap', }} > - +
)} - + - + { }), ); }); + + it('does not warn for aggregate drift below the materiality threshold', () => { + const readiness = buildMergeReadiness( + input({ + coverage: { + aggregate: { + task: { state: 'available', pct: 81.99 }, + base: { state: 'available', pct: 82 }, + delta: -0.01, + }, + files: {}, + impactedUnchangedFiles: [], + }, + }), + ); + + expect(readiness.overall).toBe('ready'); + expect(readiness.checks[2]).toEqual( + expect.objectContaining({ + status: 'pass', + detail: 'Base 82% → task 81.99% (-0.01pp).', + }), + ); + }); + + it('warns when aggregate coverage reaches the materiality threshold', () => { + const readiness = buildMergeReadiness( + input({ + coverage: { + aggregate: { + task: { state: 'available', pct: 81 }, + base: { state: 'available', pct: 82 }, + delta: -1, + }, + files: {}, + impactedUnchangedFiles: [], + }, + }), + ); + + expect(readiness.checks[2]).toEqual(expect.objectContaining({ status: 'warning' })); + }); + + it('keeps a stale base report neutral even when its delta is negative', () => { + const readiness = buildMergeReadiness( + input({ + coverage: { + aggregate: { + task: { state: 'available', pct: 78 }, + base: { state: 'available', pct: 82 }, + delta: -4, + }, + files: {}, + impactedUnchangedFiles: [], + baseline: { + mergeBaseAt: '2026-07-26T00:00:00.000Z', + stale: true, + }, + }, + }), + ); + + expect(readiness.overall).toBe('ready'); + expect(readiness.checks[2]).toEqual( + expect.objectContaining({ + status: 'neutral', + detail: + 'Base coverage report predates the task merge-base; regenerate it before comparing.', + }), + ); + }); }); describe('MergeReadinessPanel', () => { diff --git a/src/components/merge-readiness.ts b/src/components/merge-readiness.ts index 1003e4f9..8a398268 100644 --- a/src/components/merge-readiness.ts +++ b/src/components/merge-readiness.ts @@ -1,5 +1,9 @@ import type { MergeStatus, PrChecksOverall, WorktreeStatus } from '../ipc/types'; -import { formatCoverageDelta, type CoverageComparison } from '../lib/coverage-comparison'; +import { + formatCoverageDelta, + MATERIAL_COVERAGE_DELTA, + type CoverageComparison, +} from '../lib/coverage-comparison'; import type { SubtaskVerification } from '../store/types'; export type MergeReadinessCheckStatus = 'pass' | 'warning' | 'blocked' | 'checking' | 'neutral'; @@ -178,6 +182,13 @@ function coverageCheck(coverage?: CoverageComparison | null): MergeReadinessChec detail: `Task ${taskPct}%; the base report has no executable lines.`, }; } + if (coverage.baseline?.stale) { + return { + label: 'Coverage', + status: 'neutral', + detail: 'Base coverage report predates the task merge-base; regenerate it before comparing.', + }; + } const regressedUnchanged = coverage.impactedUnchangedFiles.filter( (file) => @@ -190,7 +201,10 @@ function coverageCheck(coverage?: CoverageComparison | null): MergeReadinessChec : ''; return { label: 'Coverage', - status: aggregate.delta < 0 || regressedUnchanged.length > 0 ? 'warning' : 'pass', + status: + aggregate.delta <= -MATERIAL_COVERAGE_DELTA || regressedUnchanged.length > 0 + ? 'warning' + : 'pass', detail: `Base ${aggregate.base.pct}% → task ${taskPct}% (${formatCoverageDelta(aggregate.delta)}).${impactedDetail}`, }; } diff --git a/src/lib/coverage-comparison.test.ts b/src/lib/coverage-comparison.test.ts index 4dd47551..b020ebff 100644 --- a/src/lib/coverage-comparison.test.ts +++ b/src/lib/coverage-comparison.test.ts @@ -192,6 +192,34 @@ describe('buildCoverageComparison', () => { expect(result.impactedUnchangedFiles).toEqual([]); }); + it('marks a base report older than the task merge-base as stale', () => { + const result = buildCoverageComparison( + report(82, []), + report(80, []), + [], + '2026-07-26T00:00:00.000Z', + ); + + expect(result.baseline).toEqual({ + mergeBaseAt: '2026-07-26T00:00:00.000Z', + stale: true, + }); + }); + + it('accepts a base report generated after the task merge-base', () => { + const result = buildCoverageComparison( + report(82, []), + report(80, []), + [], + '2026-07-24T00:00:00.000Z', + ); + + expect(result.baseline).toEqual({ + mergeBaseAt: '2026-07-24T00:00:00.000Z', + stale: false, + }); + }); + it.each(['toString', 'constructor', '__proto__'])( 'treats a missing report entry named %s as absent instead of reading Object.prototype', (path) => { diff --git a/src/lib/coverage-comparison.ts b/src/lib/coverage-comparison.ts index 913cd526..f5a706de 100644 --- a/src/lib/coverage-comparison.ts +++ b/src/lib/coverage-comparison.ts @@ -37,6 +37,10 @@ export interface CoverageComparison { }; files: Record; impactedUnchangedFiles: ImpactedCoverageFile[]; + baseline?: { + mergeBaseAt: string; + stale: boolean; + }; } export const MATERIAL_COVERAGE_DELTA = 1; @@ -88,6 +92,7 @@ export function buildCoverageComparison( taskSummary: CoverageSummary | null, baseSummary: CoverageSummary | null, changedFiles: ChangedFile[], + mergeBaseAt?: string | null, ): CoverageComparison { const taskAggregate = aggregateValue(taskSummary); const baseAggregate = aggregateValue(baseSummary); @@ -141,6 +146,16 @@ export function buildCoverageComparison( a.path.localeCompare(b.path), ); + const mergeBaseTime = mergeBaseAt ? Date.parse(mergeBaseAt) : Number.NaN; + const baseGeneratedTime = baseSummary ? Date.parse(baseSummary.generatedAt) : Number.NaN; + const baseline = + mergeBaseAt && Number.isFinite(mergeBaseTime) && Number.isFinite(baseGeneratedTime) + ? { + mergeBaseAt, + stale: baseGeneratedTime < mergeBaseTime, + } + : undefined; + return { aggregate: { task: taskAggregate, @@ -149,15 +164,6 @@ export function buildCoverageComparison( }, files, impactedUnchangedFiles, + baseline, }; } - -export function buildCoverageComparisonIfReady( - taskSummary: CoverageSummary | null, - baseSummary: CoverageSummary | null, - changedFiles: ChangedFile[] | null, -): CoverageComparison | null { - return changedFiles === null - ? null - : buildCoverageComparison(taskSummary, baseSummary, changedFiles); -} From 2385399660e755104264fda8e4ad570013da94d7 Mon Sep 17 00:00:00 2001 From: Liang Hu Date: Tue, 28 Jul 2026 14:09:32 -0400 Subject: [PATCH 05/10] fix(coverage): neutralize stale baseline deltas Signed-off-by: Liang Hu --- electron/ipc/git.test.ts | 16 ++++++++++++++++ electron/ipc/git.ts | 3 ++- src/components/ChangedFilesList.tsx | 14 ++++++++++---- 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/electron/ipc/git.test.ts b/electron/ipc/git.test.ts index 43b4da1b..0748d987 100644 --- a/electron/ipc/git.test.ts +++ b/electron/ipc/git.test.ts @@ -1526,6 +1526,22 @@ describe('getMergeBaseTimestamp', () => { ); expect(calls).toContainEqual(['show', '-s', '--format=%cI', MERGE_BASE]); }); + + it('returns null when no merge-base can be determined', async () => { + const calls: string[][] = []; + setupMock(calls, (args, cb) => { + if (args[0] === 'rev-parse' && args[1] === 'HEAD') { + return cb(null, `${HEAD_HASH}\n`, ''); + } + if (args[0] === 'rev-parse' && args[1] === '--verify') { + return cb(new Error('missing ref'), '', ''); + } + return cb(new Error(`unexpected git call: ${args.join(' ')}`), '', ''); + }); + + await expect(getMergeBaseTimestamp(uniqueWorktreePath(), 'main')).resolves.toBeNull(); + expect(calls.some((args) => args[0] === 'show')).toBe(false); + }); }); // --------------------------------------------------------------------------- diff --git a/electron/ipc/git.ts b/electron/ipc/git.ts index d31c37b6..3fbcddde 100644 --- a/electron/ipc/git.ts +++ b/electron/ipc/git.ts @@ -1599,7 +1599,8 @@ export async function getMergeBaseTimestamp( const branch = baseBranch ?? (await detectMainBranch(worktreePath)); const head = await pinHead(worktreePath); const picked = await pickMergeBase(worktreePath, branch, head); - const mergeBase = picked?.sha ?? head; + if (!picked) return null; + const mergeBase = picked.sha; const { stdout } = await exec('git', ['show', '-s', '--format=%cI', mergeBase], { cwd: worktreePath, }); diff --git a/src/components/ChangedFilesList.tsx b/src/components/ChangedFilesList.tsx index d21c9f6b..efa7fc2e 100644 --- a/src/components/ChangedFilesList.tsx +++ b/src/components/ChangedFilesList.tsx @@ -151,6 +151,7 @@ function deltaColor(delta: number): string { function comparisonBadge( comparison: CoverageFileComparison, + baselineStale: boolean, ): { label: string; color: string; title: string } | null { const taskLabel = coverageValueLabel(comparison.task); const baseLabel = coverageValueLabel(comparison.base); @@ -187,8 +188,8 @@ function comparisonBadge( if (comparison.delta !== null) { return { label: `${taskLabel} ${formatCoverageDelta(comparison.delta)}`, - color: deltaColor(comparison.delta), - title: `Task: ${taskLabel}; base: ${baseLabel}; delta: ${formatCoverageDelta(comparison.delta)}${renameDetail}.`, + color: baselineStale ? theme.fgMuted : deltaColor(comparison.delta), + title: `Task: ${taskLabel}; base: ${baseLabel}; delta: ${formatCoverageDelta(comparison.delta)}${renameDetail}.${baselineStale ? ' The base report predates the task merge-base, so this delta is informational only.' : ''}`, }; } @@ -206,13 +207,16 @@ function FileCoverageBadge(props: { selectedCommit?: CommitSelection; summary?: CoverageFileSummary; comparison?: CoverageFileComparison; + baselineStale: boolean; hasCoverageArtifact: boolean; }) { const isCandidate = () => !isCommitHashSelection(props.selectedCommit) && isCoverageCandidate(props.file); const summary = () => (isCandidate() ? props.summary : undefined); const badge = () => - isCandidate() && props.comparison ? comparisonBadge(props.comparison) : null; + isCandidate() && props.comparison + ? comparisonBadge(props.comparison, props.baselineStale) + : null; return ( <> @@ -900,6 +904,7 @@ export function ChangedFilesList(props: ChangedFilesListProps) { selectedCommit={props.selectedCommit} summary={coverageFiles()[row().node.path]} comparison={coverageComparison()?.files[row().node.path]} + baselineStale={coverageComparison()?.baseline?.stale ?? false} hasCoverageArtifact={hasCoverageArtifact()} /> )} @@ -963,7 +968,8 @@ export function ChangedFilesList(props: ChangedFilesListProps) { title={aggregateCoverageTitle()} style={{ color: - coverageComparison()?.aggregate.delta === null + coverageComparison()?.aggregate.delta === null || + coverageComparison()?.baseline?.stale ? theme.fgMuted : deltaColor(coverageComparison()?.aggregate.delta ?? 0), 'font-weight': '600', From 6c8149c91d9e45fa9dbb0fb2f181340465208b96 Mon Sep 17 00:00:00 2001 From: Liang Hu Date: Wed, 29 Jul 2026 08:48:48 -0400 Subject: [PATCH 06/10] fix(coverage): neutralize unanchored comparisons --- src/components/ChangedFilesList.tsx | 38 +++++++++++++++------- src/components/MergeReadinessPanel.test.ts | 29 +++++++++++++++++ src/components/merge-readiness.ts | 8 +++++ src/lib/coverage-comparison.test.ts | 9 +++++ src/lib/coverage-comparison.ts | 13 +++++--- 5 files changed, 82 insertions(+), 15 deletions(-) diff --git a/src/components/ChangedFilesList.tsx b/src/components/ChangedFilesList.tsx index efa7fc2e..9fe6f139 100644 --- a/src/components/ChangedFilesList.tsx +++ b/src/components/ChangedFilesList.tsx @@ -151,10 +151,16 @@ function deltaColor(delta: number): string { function comparisonBadge( comparison: CoverageFileComparison, - baselineStale: boolean, + baseline: CoverageComparison['baseline'], ): { label: string; color: string; title: string } | null { const taskLabel = coverageValueLabel(comparison.task); const baseLabel = coverageValueLabel(comparison.base); + const baselineInformational = baseline?.stale || baseline?.unanchored; + const baselineDetail = baseline?.stale + ? ' The base report predates the task merge-base, so this delta is informational only.' + : baseline?.unanchored + ? ' The base report cannot be anchored to the task merge-base, so this delta is informational only.' + : ''; const renameDetail = comparison.kind === 'renamed' ? ` (${comparison.basePath} → ${comparison.path})` : ''; @@ -188,8 +194,8 @@ function comparisonBadge( if (comparison.delta !== null) { return { label: `${taskLabel} ${formatCoverageDelta(comparison.delta)}`, - color: baselineStale ? theme.fgMuted : deltaColor(comparison.delta), - title: `Task: ${taskLabel}; base: ${baseLabel}; delta: ${formatCoverageDelta(comparison.delta)}${renameDetail}.${baselineStale ? ' The base report predates the task merge-base, so this delta is informational only.' : ''}`, + color: baselineInformational ? theme.fgMuted : deltaColor(comparison.delta), + title: `Task: ${taskLabel}; base: ${baseLabel}; delta: ${formatCoverageDelta(comparison.delta)}${renameDetail}.${baselineDetail}`, }; } @@ -207,16 +213,14 @@ function FileCoverageBadge(props: { selectedCommit?: CommitSelection; summary?: CoverageFileSummary; comparison?: CoverageFileComparison; - baselineStale: boolean; + baseline?: CoverageComparison['baseline']; hasCoverageArtifact: boolean; }) { const isCandidate = () => !isCommitHashSelection(props.selectedCommit) && isCoverageCandidate(props.file); const summary = () => (isCandidate() ? props.summary : undefined); const badge = () => - isCandidate() && props.comparison - ? comparisonBadge(props.comparison, props.baselineStale) - : null; + isCandidate() && props.comparison ? comparisonBadge(props.comparison, props.baseline) : null; return ( <> @@ -396,7 +400,11 @@ export function ChangedFilesList(props: ChangedFilesListProps) { if (comparison.aggregate.delta !== null) { lines.push(`Delta: ${formatCoverageDelta(comparison.aggregate.delta)}.`); } - if (comparison.baseline) { + if (comparison.baseline?.unanchored) { + lines.push( + 'The base report cannot be anchored to the task merge-base, so merge readiness ignores the delta.', + ); + } else if (comparison.baseline?.mergeBaseAt) { lines.push(`Task merge-base: ${comparison.baseline.mergeBaseAt}.`); if (comparison.baseline.stale) { lines.push( @@ -904,7 +912,7 @@ export function ChangedFilesList(props: ChangedFilesListProps) { selectedCommit={props.selectedCommit} summary={coverageFiles()[row().node.path]} comparison={coverageComparison()?.files[row().node.path]} - baselineStale={coverageComparison()?.baseline?.stale ?? false} + baseline={coverageComparison()?.baseline} hasCoverageArtifact={hasCoverageArtifact()} /> )} @@ -969,7 +977,8 @@ export function ChangedFilesList(props: ChangedFilesListProps) { style={{ color: coverageComparison()?.aggregate.delta === null || - coverageComparison()?.baseline?.stale + coverageComparison()?.baseline?.stale || + coverageComparison()?.baseline?.unanchored ? theme.fgMuted : deltaColor(coverageComparison()?.aggregate.delta ?? 0), 'font-weight': '600', @@ -1032,7 +1041,14 @@ export function ChangedFilesList(props: ChangedFilesListProps) { 0}> ↕ {coverageComparison()?.impactedUnchangedFiles.length} other diff --git a/src/components/MergeReadinessPanel.test.ts b/src/components/MergeReadinessPanel.test.ts index d51d93ac..c03a1ca3 100644 --- a/src/components/MergeReadinessPanel.test.ts +++ b/src/components/MergeReadinessPanel.test.ts @@ -307,6 +307,35 @@ describe('buildMergeReadiness', () => { }), ); }); + + it('keeps an unanchored base report neutral even when its delta is positive', () => { + const readiness = buildMergeReadiness( + input({ + coverage: { + aggregate: { + task: { state: 'available', pct: 84 }, + base: { state: 'available', pct: 82 }, + delta: 2, + }, + files: {}, + impactedUnchangedFiles: [], + baseline: { + stale: false, + unanchored: true, + }, + }, + }), + ); + + expect(readiness.overall).toBe('ready'); + expect(readiness.checks[2]).toEqual( + expect.objectContaining({ + status: 'neutral', + detail: + 'Base coverage report cannot be anchored to the task merge-base; comparison is informational only.', + }), + ); + }); }); describe('MergeReadinessPanel', () => { diff --git a/src/components/merge-readiness.ts b/src/components/merge-readiness.ts index 8a398268..62b0707c 100644 --- a/src/components/merge-readiness.ts +++ b/src/components/merge-readiness.ts @@ -189,6 +189,14 @@ function coverageCheck(coverage?: CoverageComparison | null): MergeReadinessChec detail: 'Base coverage report predates the task merge-base; regenerate it before comparing.', }; } + if (coverage.baseline?.unanchored) { + return { + label: 'Coverage', + status: 'neutral', + detail: + 'Base coverage report cannot be anchored to the task merge-base; comparison is informational only.', + }; + } const regressedUnchanged = coverage.impactedUnchangedFiles.filter( (file) => diff --git a/src/lib/coverage-comparison.test.ts b/src/lib/coverage-comparison.test.ts index b020ebff..ffaec023 100644 --- a/src/lib/coverage-comparison.test.ts +++ b/src/lib/coverage-comparison.test.ts @@ -220,6 +220,15 @@ describe('buildCoverageComparison', () => { }); }); + it('marks a present base report with an unknown merge-base as unanchored', () => { + const result = buildCoverageComparison(report(82, []), report(80, []), []); + + expect(result.baseline).toEqual({ + stale: false, + unanchored: true, + }); + }); + it.each(['toString', 'constructor', '__proto__'])( 'treats a missing report entry named %s as absent instead of reading Object.prototype', (path) => { diff --git a/src/lib/coverage-comparison.ts b/src/lib/coverage-comparison.ts index f5a706de..974fb38d 100644 --- a/src/lib/coverage-comparison.ts +++ b/src/lib/coverage-comparison.ts @@ -38,8 +38,9 @@ export interface CoverageComparison { files: Record; impactedUnchangedFiles: ImpactedCoverageFile[]; baseline?: { - mergeBaseAt: string; + mergeBaseAt?: string; stale: boolean; + unanchored?: boolean; }; } @@ -148,13 +149,17 @@ export function buildCoverageComparison( const mergeBaseTime = mergeBaseAt ? Date.parse(mergeBaseAt) : Number.NaN; const baseGeneratedTime = baseSummary ? Date.parse(baseSummary.generatedAt) : Number.NaN; - const baseline = - mergeBaseAt && Number.isFinite(mergeBaseTime) && Number.isFinite(baseGeneratedTime) + const baseline = !baseSummary + ? undefined + : mergeBaseAt && Number.isFinite(mergeBaseTime) && Number.isFinite(baseGeneratedTime) ? { mergeBaseAt, stale: baseGeneratedTime < mergeBaseTime, } - : undefined; + : { + stale: false, + unanchored: true, + }; return { aggregate: { From 84170fb8e2afcf4f027d81a88ea783510e6c934e Mon Sep 17 00:00:00 2001 From: Liang Hu Date: Wed, 29 Jul 2026 16:33:34 -0400 Subject: [PATCH 07/10] fix(coverage): anchor base reports to base HEAD --- electron/ipc/channel-manifest.json | 1 - electron/ipc/git.test.ts | 48 +++--------- electron/ipc/git.ts | 34 ++++---- electron/ipc/register.ts | 4 - electron/preload.cjs | 1 - src/components/ChangedFilesList.tsx | 91 +++++++++++----------- src/components/MergeReadinessPanel.test.ts | 8 +- src/components/merge-readiness.ts | 16 ++-- src/lib/coverage-comparison.test.ts | 14 ++-- src/lib/coverage-comparison.ts | 19 +++-- 10 files changed, 103 insertions(+), 133 deletions(-) diff --git a/electron/ipc/channel-manifest.json b/electron/ipc/channel-manifest.json index cadacaa3..1d9316aa 100644 --- a/electron/ipc/channel-manifest.json +++ b/electron/ipc/channel-manifest.json @@ -19,7 +19,6 @@ "GetGitignoredDirs": "get_gitignored_dirs", "ListImportableWorktrees": "list_importable_worktrees", "GetBranchWorktreePath": "get_branch_worktree_path", - "GetMergeBaseTimestamp": "get_merge_base_timestamp", "GetWorktreeStatus": "get_worktree_status", "CheckMergeStatus": "check_merge_status", "MergeTask": "merge_task", diff --git a/electron/ipc/git.test.ts b/electron/ipc/git.test.ts index 0748d987..666d0f6b 100644 --- a/electron/ipc/git.test.ts +++ b/electron/ipc/git.test.ts @@ -60,7 +60,6 @@ import { getUncommittedChangedFiles, checkMergeStatus, getBranchWorktreePath, - getMergeBaseTimestamp, listImportableWorktrees, mergeTask, } from './git.js'; @@ -1495,55 +1494,26 @@ describe('getBranchWorktreePath', () => { '', ); } + if (args[0] === 'show') { + return cb(null, '2026-07-25T12:00:00-04:00\n', ''); + } return cb(new Error(`unexpected git call: ${args.join(' ')}`), '', ''); }); - await expect(getBranchWorktreePath('/repo', 'main')).resolves.toBe('/repo'); + await expect(getBranchWorktreePath('/repo', 'main')).resolves.toEqual({ + path: '/repo', + head: 'aaa111', + headCommittedAt: '2026-07-25T16:00:00.000Z', + }); await expect(getBranchWorktreePath('/repo', 'missing')).resolves.toBeNull(); expect(calls).toEqual([ ['worktree', 'list', '--porcelain'], + ['show', '-s', '--format=%cI', 'aaa111'], ['worktree', 'list', '--porcelain'], ]); }); }); -describe('getMergeBaseTimestamp', () => { - beforeEach(() => { - vi.clearAllMocks(); - }); - - it('returns the picked merge-base commit time as ISO UTC', async () => { - const calls: string[][] = []; - setupMock( - calls, - buildWorktreeMockHandler({ - mergeBaseTimestamp: '2026-07-25T12:00:00-04:00', - }), - ); - - await expect(getMergeBaseTimestamp(uniqueWorktreePath(), 'main')).resolves.toBe( - '2026-07-25T16:00:00.000Z', - ); - expect(calls).toContainEqual(['show', '-s', '--format=%cI', MERGE_BASE]); - }); - - it('returns null when no merge-base can be determined', async () => { - const calls: string[][] = []; - setupMock(calls, (args, cb) => { - if (args[0] === 'rev-parse' && args[1] === 'HEAD') { - return cb(null, `${HEAD_HASH}\n`, ''); - } - if (args[0] === 'rev-parse' && args[1] === '--verify') { - return cb(new Error('missing ref'), '', ''); - } - return cb(new Error(`unexpected git call: ${args.join(' ')}`), '', ''); - }); - - await expect(getMergeBaseTimestamp(uniqueWorktreePath(), 'main')).resolves.toBeNull(); - expect(calls.some((args) => args[0] === 'show')).toBe(false); - }); -}); - // --------------------------------------------------------------------------- // checkMergeStatus — main-ahead count uses cherry-pick filtering so rebased // patch-equivalent commits in main don't trigger a needless rebase prompt. diff --git a/electron/ipc/git.ts b/electron/ipc/git.ts index 3fbcddde..13b6b215 100644 --- a/electron/ipc/git.ts +++ b/electron/ipc/git.ts @@ -690,6 +690,7 @@ function safeRealpath(p: string): string { interface ListedWorktree { path: string; + head: string | null; branchName: string | null; detached: boolean; } @@ -710,6 +711,7 @@ function parseWorktreeList(output: string): ListedWorktree[] { if (current?.path) entries.push(current); current = { path: line.slice('worktree '.length).trim(), + head: null, branchName: null, detached: false, }; @@ -717,6 +719,10 @@ function parseWorktreeList(output: string): ListedWorktree[] { } if (!current) continue; + if (line.startsWith('HEAD ')) { + current.head = line.slice('HEAD '.length).trim() || null; + continue; + } if (line.startsWith('branch ')) { const ref = line.slice('branch '.length).trim(); const prefix = 'refs/heads/'; @@ -1579,7 +1585,7 @@ export async function listImportableWorktrees(projectRoot: string): Promise< export async function getBranchWorktreePath( projectRoot: string, branchName: string, -): Promise { +): Promise<{ path: string; head: string; headCommittedAt: string | null } | null> { const { stdout } = await exec('git', ['worktree', 'list', '--porcelain'], { cwd: projectRoot, maxBuffer: MAX_BUFFER, @@ -1587,27 +1593,19 @@ export async function getBranchWorktreePath( const match = parseWorktreeList(stdout).find( (entry) => !entry.detached && entry.branchName === branchName, ); - return match?.path ?? null; -} - -/** Return the task branch's merge-base commit time without mutating a worktree. */ -export async function getMergeBaseTimestamp( - worktreePath: string, - baseBranch?: string, -): Promise { + if (!match?.path || !match.head) return null; try { - const branch = baseBranch ?? (await detectMainBranch(worktreePath)); - const head = await pinHead(worktreePath); - const picked = await pickMergeBase(worktreePath, branch, head); - if (!picked) return null; - const mergeBase = picked.sha; - const { stdout } = await exec('git', ['show', '-s', '--format=%cI', mergeBase], { - cwd: worktreePath, + const { stdout } = await exec('git', ['show', '-s', '--format=%cI', match.head], { + cwd: match.path, }); const timestamp = new Date(stdout.trim()); - return Number.isNaN(timestamp.getTime()) ? null : timestamp.toISOString(); + return { + path: match.path, + head: match.head, + headCommittedAt: Number.isNaN(timestamp.getTime()) ? null : timestamp.toISOString(), + }; } catch { - return null; + return { path: match.path, head: match.head, headCommittedAt: null }; } } diff --git a/electron/ipc/register.ts b/electron/ipc/register.ts index 0f581fae..c85d92de 100644 --- a/electron/ipc/register.ts +++ b/electron/ipc/register.ts @@ -55,7 +55,6 @@ import { getWorktreeStatus, listImportableWorktrees, getBranchWorktreePath, - getMergeBaseTimestamp, commitAll, discardUncommitted, checkMergeStatus, @@ -608,9 +607,6 @@ export function registerAllHandlers(win: BrowserWindow): void { ipcMain.handle(IPC.GetBranchWorktreePath, (_e, args) => { return getBranchWorktreePath(projectRootArg(args), branchNameArg(args)); }); - ipcMain.handle(IPC.GetMergeBaseTimestamp, (_e, args) => { - return getMergeBaseTimestamp(worktreePathArg(args), optionalBaseBranch(args)); - }); ipcMain.handle(IPC.GetWorktreeStatus, (_e, args) => { const worktreePath = worktreePathArg(args); return getWorktreeStatus(worktreePath, optionalBaseBranch(args)); diff --git a/electron/preload.cjs b/electron/preload.cjs index 48044ade..1ce97c81 100644 --- a/electron/preload.cjs +++ b/electron/preload.cjs @@ -23,7 +23,6 @@ const ALLOWED_CHANNELS = new Set([ 'get_gitignored_dirs', 'list_importable_worktrees', 'get_branch_worktree_path', - 'get_merge_base_timestamp', 'get_worktree_status', 'check_merge_status', 'merge_task', diff --git a/src/components/ChangedFilesList.tsx b/src/components/ChangedFilesList.tsx index 9fe6f139..f7866833 100644 --- a/src/components/ChangedFilesList.tsx +++ b/src/components/ChangedFilesList.tsx @@ -9,6 +9,7 @@ import { buildFileTree, flattenVisibleTree } from '../lib/file-tree'; import { buildCoverageComparison, formatCoverageDelta, + isBaselineInformational, type CoverageComparison, type CoverageFileComparison, type CoverageValue, @@ -155,11 +156,12 @@ function comparisonBadge( ): { label: string; color: string; title: string } | null { const taskLabel = coverageValueLabel(comparison.task); const baseLabel = coverageValueLabel(comparison.base); - const baselineInformational = baseline?.stale || baseline?.unanchored; + const baselineInformational = isBaselineInformational(baseline); + const baselineBranch = baseline?.baseBranch ?? 'base branch'; const baselineDetail = baseline?.stale - ? ' The base report predates the task merge-base, so this delta is informational only.' + ? ` The base report predates ${baselineBranch} as currently checked out, so this delta is informational only.` : baseline?.unanchored - ? ' The base report cannot be anchored to the task merge-base, so this delta is informational only.' + ? ` The base report cannot be anchored to ${baselineBranch} as currently checked out, so this delta is informational only.` : ''; const renameDetail = comparison.kind === 'renamed' ? ` (${comparison.basePath} → ${comparison.path})` : ''; @@ -278,14 +280,17 @@ async function resolveBaseCoverageRoot( projectRoot: string | undefined, baseBranch: string, taskRoot: string, -): Promise { +): Promise<{ path: string; headCommittedAt: string | null } | null> { if (!projectRoot) return null; - const baseRoot = await invoke(IPC.GetBranchWorktreePath, { + const baseWorktree = await invoke<{ + path: string; + headCommittedAt: string | null; + } | null>(IPC.GetBranchWorktreePath, { projectRoot, branchName: baseBranch, }).catch(() => null); - if (!baseRoot || baseRoot === taskRoot) return null; - return baseRoot; + if (!baseWorktree || baseWorktree.path === taskRoot) return null; + return baseWorktree; } function OpenInEditorButton(props: { @@ -330,7 +335,8 @@ export function ChangedFilesList(props: ChangedFilesListProps) { const [comparisonFiles, setComparisonFiles] = createSignal(null); const [coverage, setCoverage] = createSignal(null); const [baseCoverage, setBaseCoverage] = createSignal(null); - const [mergeBaseAt, setMergeBaseAt] = createSignal(null); + const [baseHeadAt, setBaseHeadAt] = createSignal(null); + const [baseBranchName, setBaseBranchName] = createSignal(); const [canOpenFilesInEditor, setCanOpenFilesInEditor] = createSignal(false); const [selectedIndex, setSelectedIndex] = createSignal(-1); const [collapsed, setCollapsed] = createSignal>(new Set()); @@ -352,7 +358,13 @@ export function ChangedFilesList(props: ChangedFilesListProps) { const coverageComparison = createMemo(() => { const inventory = comparisonFiles(); if (!inventory || isCommitHashSelection(props.selectedCommit)) return null; - return buildCoverageComparison(coverage(), baseCoverage(), inventory, mergeBaseAt()); + return buildCoverageComparison( + coverage(), + baseCoverage(), + inventory, + baseHeadAt(), + baseBranchName(), + ); }); const coveredEligibleFiles = createMemo(() => eligibleFiles().filter((file) => Boolean(coverageFiles()[file.path])), @@ -402,13 +414,18 @@ export function ChangedFilesList(props: ChangedFilesListProps) { } if (comparison.baseline?.unanchored) { lines.push( - 'The base report cannot be anchored to the task merge-base, so merge readiness ignores the delta.', + `The base report cannot be anchored to ${comparison.baseline.baseBranch ?? 'the base branch'} as currently checked out, so merge readiness ignores the delta.`, + ); + } else if (comparison.baseline?.baseHeadAt) { + lines.push( + `Baseline: ${comparison.baseline.baseBranch ?? 'base branch'} as currently checked out (${comparison.baseline.baseHeadAt}).`, + ); + lines.push( + `This comparison may include changes merged into ${comparison.baseline.baseBranch ?? 'the base branch'} after the task branched.`, ); - } else if (comparison.baseline?.mergeBaseAt) { - lines.push(`Task merge-base: ${comparison.baseline.mergeBaseAt}.`); if (comparison.baseline.stale) { lines.push( - 'The base report predates the merge-base, so merge readiness ignores the delta.', + `The base report predates ${comparison.baseline.baseBranch ?? 'the base branch'} as currently checked out, so merge readiness ignores the delta.`, ); } } @@ -701,20 +718,14 @@ export function ChangedFilesList(props: ChangedFilesListProps) { batch(() => { setCoverage(null); setBaseCoverage(null); - setMergeBaseAt(null); + setBaseHeadAt(null); + setBaseBranchName(undefined); }); return; } if (!props.isActive) return; let cancelled = false; let inFlight = false; - let cachedMergeBase: - | { - taskGeneratedAt: string; - value: string | null; - } - | undefined; - async function refresh() { if (inFlight) return; inFlight = true; @@ -724,31 +735,21 @@ export function ChangedFilesList(props: ChangedFilesListProps) { reportPath: props.coverageReportPath, }).catch(() => null); let baseResult: CoverageSummary | null = null; - let mergeBaseResult: string | null = null; + let baseHeadResult: string | null = null; + let resolvedBaseBranch: string | null = null; if (taskResult) { - const resolvedBaseBranch = baseBranch + resolvedBaseBranch = baseBranch ? baseBranch : projectRoot ? await invoke(IPC.GetMainBranch, { projectRoot }).catch(() => null) : null; - const baseRoot = resolvedBaseBranch + const baseWorktree = resolvedBaseBranch ? await resolveBaseCoverageRoot(projectRoot, resolvedBaseBranch, repoRoot) : null; - if (resolvedBaseBranch) { - if (!cachedMergeBase || cachedMergeBase.taskGeneratedAt !== taskResult.generatedAt) { - cachedMergeBase = { - taskGeneratedAt: taskResult.generatedAt, - value: await invoke(IPC.GetMergeBaseTimestamp, { - worktreePath: repoRoot, - baseBranch: resolvedBaseBranch, - }).catch(() => null), - }; - } - mergeBaseResult = cachedMergeBase.value; - } - if (baseRoot) { + if (baseWorktree) { + baseHeadResult = baseWorktree.headCommittedAt; baseResult = await invoke(IPC.GetCoverageSummary, { - repoRoot: baseRoot, + repoRoot: baseWorktree.path, reportPath: props.coverageReportPath, }).catch(() => null); } @@ -761,7 +762,8 @@ export function ChangedFilesList(props: ChangedFilesListProps) { setBaseCoverage((current) => sameCoverageSummary(current, baseResult) ? current : baseResult, ); - setMergeBaseAt(mergeBaseResult); + setBaseHeadAt(baseHeadResult); + setBaseBranchName(resolvedBaseBranch ?? undefined); }); } } finally { @@ -977,8 +979,7 @@ export function ChangedFilesList(props: ChangedFilesListProps) { style={{ color: coverageComparison()?.aggregate.delta === null || - coverageComparison()?.baseline?.stale || - coverageComparison()?.baseline?.unanchored + isBaselineInformational(coverageComparison()?.baseline) ? theme.fgMuted : deltaColor(coverageComparison()?.aggregate.delta ?? 0), 'font-weight': '600', @@ -1042,11 +1043,9 @@ export function ChangedFilesList(props: ChangedFilesListProps) { diff --git a/src/components/MergeReadinessPanel.test.ts b/src/components/MergeReadinessPanel.test.ts index c03a1ca3..9bdc17bd 100644 --- a/src/components/MergeReadinessPanel.test.ts +++ b/src/components/MergeReadinessPanel.test.ts @@ -291,7 +291,8 @@ describe('buildMergeReadiness', () => { files: {}, impactedUnchangedFiles: [], baseline: { - mergeBaseAt: '2026-07-26T00:00:00.000Z', + baseBranch: 'main', + baseHeadAt: '2026-07-26T00:00:00.000Z', stale: true, }, }, @@ -303,7 +304,7 @@ describe('buildMergeReadiness', () => { expect.objectContaining({ status: 'neutral', detail: - 'Base coverage report predates the task merge-base; regenerate it before comparing.', + 'Base coverage report predates main as currently checked out; regenerate it before comparing.', }), ); }); @@ -320,6 +321,7 @@ describe('buildMergeReadiness', () => { files: {}, impactedUnchangedFiles: [], baseline: { + baseBranch: 'main', stale: false, unanchored: true, }, @@ -332,7 +334,7 @@ describe('buildMergeReadiness', () => { expect.objectContaining({ status: 'neutral', detail: - 'Base coverage report cannot be anchored to the task merge-base; comparison is informational only.', + 'Base coverage report cannot be anchored to main as currently checked out; comparison is informational only.', }), ); }); diff --git a/src/components/merge-readiness.ts b/src/components/merge-readiness.ts index 62b0707c..8918dbfe 100644 --- a/src/components/merge-readiness.ts +++ b/src/components/merge-readiness.ts @@ -1,6 +1,7 @@ import type { MergeStatus, PrChecksOverall, WorktreeStatus } from '../ipc/types'; import { formatCoverageDelta, + isBaselineInformational, MATERIAL_COVERAGE_DELTA, type CoverageComparison, } from '../lib/coverage-comparison'; @@ -182,19 +183,14 @@ function coverageCheck(coverage?: CoverageComparison | null): MergeReadinessChec detail: `Task ${taskPct}%; the base report has no executable lines.`, }; } - if (coverage.baseline?.stale) { + if (isBaselineInformational(coverage.baseline)) { + const baseBranch = coverage.baseline?.baseBranch ?? 'base branch'; return { label: 'Coverage', status: 'neutral', - detail: 'Base coverage report predates the task merge-base; regenerate it before comparing.', - }; - } - if (coverage.baseline?.unanchored) { - return { - label: 'Coverage', - status: 'neutral', - detail: - 'Base coverage report cannot be anchored to the task merge-base; comparison is informational only.', + detail: coverage.baseline?.stale + ? `Base coverage report predates ${baseBranch} as currently checked out; regenerate it before comparing.` + : `Base coverage report cannot be anchored to ${baseBranch} as currently checked out; comparison is informational only.`, }; } diff --git a/src/lib/coverage-comparison.test.ts b/src/lib/coverage-comparison.test.ts index ffaec023..3a6f34ec 100644 --- a/src/lib/coverage-comparison.test.ts +++ b/src/lib/coverage-comparison.test.ts @@ -192,35 +192,39 @@ describe('buildCoverageComparison', () => { expect(result.impactedUnchangedFiles).toEqual([]); }); - it('marks a base report older than the task merge-base as stale', () => { + it('marks a base report older than the base branch HEAD as stale', () => { const result = buildCoverageComparison( report(82, []), report(80, []), [], '2026-07-26T00:00:00.000Z', + 'main', ); expect(result.baseline).toEqual({ - mergeBaseAt: '2026-07-26T00:00:00.000Z', + baseBranch: 'main', + baseHeadAt: '2026-07-26T00:00:00.000Z', stale: true, }); }); - it('accepts a base report generated after the task merge-base', () => { + it('accepts a base report generated after the base branch HEAD', () => { const result = buildCoverageComparison( report(82, []), report(80, []), [], '2026-07-24T00:00:00.000Z', + 'main', ); expect(result.baseline).toEqual({ - mergeBaseAt: '2026-07-24T00:00:00.000Z', + baseBranch: 'main', + baseHeadAt: '2026-07-24T00:00:00.000Z', stale: false, }); }); - it('marks a present base report with an unknown merge-base as unanchored', () => { + it('marks a present base report with an unknown base branch HEAD as unanchored', () => { const result = buildCoverageComparison(report(82, []), report(80, []), []); expect(result.baseline).toEqual({ diff --git a/src/lib/coverage-comparison.ts b/src/lib/coverage-comparison.ts index 974fb38d..bcfb7a65 100644 --- a/src/lib/coverage-comparison.ts +++ b/src/lib/coverage-comparison.ts @@ -38,7 +38,8 @@ export interface CoverageComparison { files: Record; impactedUnchangedFiles: ImpactedCoverageFile[]; baseline?: { - mergeBaseAt?: string; + baseBranch?: string; + baseHeadAt?: string; stale: boolean; unanchored?: boolean; }; @@ -46,6 +47,10 @@ export interface CoverageComparison { export const MATERIAL_COVERAGE_DELTA = 1; +export function isBaselineInformational(baseline: CoverageComparison['baseline']): boolean { + return Boolean(baseline?.stale || baseline?.unanchored); +} + function roundPercentage(value: number): number { return Math.round(value * 100) / 100; } @@ -93,7 +98,8 @@ export function buildCoverageComparison( taskSummary: CoverageSummary | null, baseSummary: CoverageSummary | null, changedFiles: ChangedFile[], - mergeBaseAt?: string | null, + baseHeadAt?: string | null, + baseBranch?: string, ): CoverageComparison { const taskAggregate = aggregateValue(taskSummary); const baseAggregate = aggregateValue(baseSummary); @@ -147,14 +153,15 @@ export function buildCoverageComparison( a.path.localeCompare(b.path), ); - const mergeBaseTime = mergeBaseAt ? Date.parse(mergeBaseAt) : Number.NaN; + const baseHeadTime = baseHeadAt ? Date.parse(baseHeadAt) : Number.NaN; const baseGeneratedTime = baseSummary ? Date.parse(baseSummary.generatedAt) : Number.NaN; const baseline = !baseSummary ? undefined - : mergeBaseAt && Number.isFinite(mergeBaseTime) && Number.isFinite(baseGeneratedTime) + : baseHeadAt && Number.isFinite(baseHeadTime) && Number.isFinite(baseGeneratedTime) ? { - mergeBaseAt, - stale: baseGeneratedTime < mergeBaseTime, + ...(baseBranch ? { baseBranch } : {}), + baseHeadAt, + stale: baseGeneratedTime < baseHeadTime, } : { stale: false, From 9ae1f69c19c442846dc00ff401442f1ecc82ebc6 Mon Sep 17 00:00:00 2001 From: Liang Hu Date: Thu, 30 Jul 2026 14:11:26 -0400 Subject: [PATCH 08/10] fix(coverage): avoid attributing base drift to tasks --- src/components/MergeReadinessPanel.test.ts | 31 ++++++++++++++++++++++ src/components/merge-readiness.ts | 4 ++- src/lib/coverage-comparison.test.ts | 3 ++- src/lib/coverage-comparison.ts | 1 + 4 files changed, 37 insertions(+), 2 deletions(-) diff --git a/src/components/MergeReadinessPanel.test.ts b/src/components/MergeReadinessPanel.test.ts index 9bdc17bd..4078fd9f 100644 --- a/src/components/MergeReadinessPanel.test.ts +++ b/src/components/MergeReadinessPanel.test.ts @@ -237,6 +237,37 @@ describe('buildMergeReadiness', () => { ); }); + it('does not attribute base-only covered files to the task', () => { + const readiness = buildMergeReadiness( + input({ + coverage: { + aggregate: { + task: { state: 'available', pct: 82 }, + base: { state: 'available', pct: 82 }, + delta: 0, + }, + files: {}, + impactedUnchangedFiles: [ + { + path: 'src/added-on-base.ts', + task: { state: 'file-not-present', pct: null }, + base: { state: 'available', pct: 80 }, + delta: null, + }, + ], + }, + }), + ); + + expect(readiness.overall).toBe('ready'); + expect(readiness.checks[2]).toEqual( + expect.objectContaining({ + status: 'pass', + detail: 'Base 82% → task 82% (0pp).', + }), + ); + }); + it('does not warn for aggregate drift below the materiality threshold', () => { const readiness = buildMergeReadiness( input({ diff --git a/src/components/merge-readiness.ts b/src/components/merge-readiness.ts index 8918dbfe..b560eb29 100644 --- a/src/components/merge-readiness.ts +++ b/src/components/merge-readiness.ts @@ -197,7 +197,9 @@ function coverageCheck(coverage?: CoverageComparison | null): MergeReadinessChec const regressedUnchanged = coverage.impactedUnchangedFiles.filter( (file) => file.base.state === 'available' && - (file.task.state !== 'available' || (file.delta !== null && file.delta < 0)), + file.task.state === 'available' && + file.delta !== null && + file.delta <= -MATERIAL_COVERAGE_DELTA, ); const impactedDetail = regressedUnchanged.length > 0 diff --git a/src/lib/coverage-comparison.test.ts b/src/lib/coverage-comparison.test.ts index 3a6f34ec..f1d36489 100644 --- a/src/lib/coverage-comparison.test.ts +++ b/src/lib/coverage-comparison.test.ts @@ -225,9 +225,10 @@ describe('buildCoverageComparison', () => { }); it('marks a present base report with an unknown base branch HEAD as unanchored', () => { - const result = buildCoverageComparison(report(82, []), report(80, []), []); + const result = buildCoverageComparison(report(82, []), report(80, []), [], null, 'main'); expect(result.baseline).toEqual({ + baseBranch: 'main', stale: false, unanchored: true, }); diff --git a/src/lib/coverage-comparison.ts b/src/lib/coverage-comparison.ts index bcfb7a65..61b0136d 100644 --- a/src/lib/coverage-comparison.ts +++ b/src/lib/coverage-comparison.ts @@ -164,6 +164,7 @@ export function buildCoverageComparison( stale: baseGeneratedTime < baseHeadTime, } : { + ...(baseBranch ? { baseBranch } : {}), stale: false, unanchored: true, }; From 347108ed4d9daa68eb026ed77cbbb22913803329 Mon Sep 17 00:00:00 2001 From: Liang Hu Date: Fri, 31 Jul 2026 09:48:19 -0400 Subject: [PATCH 09/10] fix(review): harden coverage comparison freshness Signed-off-by: Liang Hu --- electron/ipc/git.test.ts | 53 ++++-- electron/ipc/git.ts | 84 +++++----- src/components/ChangedFilesList.test.ts | 29 +++- src/components/ChangedFilesList.tsx | 180 ++++++++++++++++----- src/components/MergeReadinessPanel.test.ts | 81 ++++++++++ src/components/merge-readiness.ts | 34 +++- src/lib/coverage-comparison.test.ts | 40 ++++- src/lib/coverage-comparison.ts | 25 ++- 8 files changed, 419 insertions(+), 107 deletions(-) diff --git a/electron/ipc/git.test.ts b/electron/ipc/git.test.ts index 666d0f6b..50f87860 100644 --- a/electron/ipc/git.test.ts +++ b/electron/ipc/git.test.ts @@ -542,7 +542,7 @@ function buildWorktreeMockHandler(opts: { ) { const fallback = [opts.committedRawNumstat, opts.uncommittedRawNumstat] .filter((part) => part && part.length > 0) - .join('\n'); + .join(''); cb(null, opts.finalRawNumstat ?? fallback, ''); return; } @@ -611,13 +611,19 @@ function buildWorktreeMockHandler(opts: { /** * Build a raw+numstat combined output string for a single modified file. - * Format matches `git diff --raw --numstat` output. + * Format matches `git diff --raw --numstat -z` output. */ function rawNumstatEntry(filePath: string, added: number, removed: number, status = 'M'): string { - return [ - `:100644 100644 aaa111 bbb222 ${status}\t${filePath}`, - `${added}\t${removed}\t${filePath}`, - ].join('\n'); + return `:100644 100644 aaa111 bbb222 ${status}\0${filePath}\0${added}\t${removed}\t${filePath}\0`; +} + +function rawNumstatRenameEntry( + previousPath: string, + path: string, + added: number, + removed: number, +): string { + return `:100644 100644 aaa111 bbb222 R100\0${previousPath}\0${path}\0${added}\t${removed}\t\0${previousPath}\0${path}\0`; } // --------------------------------------------------------------------------- @@ -679,10 +685,7 @@ describe('getChangedFiles (worktree-based, merge-base diff)', () => { setupMock( calls, buildWorktreeMockHandler({ - committedRawNumstat: [ - ':100644 100644 aaa111 bbb222 R100\tsrc/old-name.ts\tsrc/new-name.ts', - '0\t0\tsrc/{old-name.ts => new-name.ts}', - ].join('\n'), + committedRawNumstat: rawNumstatRenameEntry('src/old-name.ts', 'src/new-name.ts', 0, 0), }), ); @@ -697,6 +700,34 @@ describe('getChangedFiles (worktree-based, merge-base diff)', () => { ]); }); + it.each([ + ['dir/old => literal.ts', 'dir/new.ts'], + ['dir/{old}.ts', 'dir/{new}.ts'], + ])('should preserve exact Git paths when renaming %s', async (previousPath, path) => { + const calls: string[][] = []; + setupMock( + calls, + buildWorktreeMockHandler({ + committedRawNumstat: rawNumstatRenameEntry(previousPath, path, 4, 2), + }), + ); + + const files = await getChangedFiles(uniqueWorktreePath(), 'main'); + + expect(files).toEqual([ + expect.objectContaining({ + path, + previous_path: previousPath, + lines_added: 4, + lines_removed: 2, + status: 'R', + }), + ]); + expect( + calls.some((args) => args[0] === 'diff' && args.includes('--raw') && args.includes('-z')), + ).toBe(true); + }); + it('should preserve a literal arrow in an ordinary modified filename', async () => { const calls: string[][] = []; setupMock( @@ -726,7 +757,7 @@ describe('getChangedFiles (worktree-based, merge-base diff)', () => { committedRawNumstat: [ rawNumstatEntry('file-a.ts', 5, 2), rawNumstatEntry('file-b.ts', 3, 1), - ].join('\n'), + ].join(''), }), ); diff --git a/electron/ipc/git.ts b/electron/ipc/git.ts index 13b6b215..b920a8a5 100644 --- a/electron/ipc/git.ts +++ b/electron/ipc/git.ts @@ -536,24 +536,7 @@ function normalizeStatusPath(raw: string): string { return trimmed.replace(/^"|"$/g, '').replace(/\\(.)/g, '$1'); } -function parseNumstatPath(raw: string): string { - const trimmed = raw.trim(); - const arrowIndex = trimmed.indexOf(' => '); - if (arrowIndex < 0) return normalizeStatusPath(trimmed); - - const openBrace = trimmed.lastIndexOf('{', arrowIndex); - const closeBrace = trimmed.indexOf('}', arrowIndex); - if (openBrace >= 0 && closeBrace > arrowIndex) { - const prefix = trimmed.slice(0, openBrace); - const suffix = trimmed.slice(closeBrace + 1); - const destinationPath = `${prefix}${trimmed.slice(arrowIndex + 4, closeBrace)}${suffix}`; - return normalizeStatusPath(destinationPath); - } - - return normalizeStatusPath(trimmed.slice(arrowIndex + 4)); -} - -/** Parse combined `git diff --raw --numstat` output into status and numstat maps. */ +/** Parse combined `git diff --raw --numstat -z` output into status and numstat maps. */ function parseDiffRawNumstat(output: string): { statusMap: Map; numstatMap: Map; @@ -563,34 +546,43 @@ function parseDiffRawNumstat(output: string): { const numstatMap = new Map(); const previousPathMap = new Map(); - for (const line of output.split('\n')) { - if (line.startsWith(':')) { - // --raw format: ":old_mode new_mode old_hash new_hash status\tpath" - const parts = line.split('\t'); - if (parts.length >= 2) { - const statusLetter = parts[0].split(/\s+/).pop()?.charAt(0) ?? 'M'; - const rawPath = parts[parts.length - 1]; - const p = normalizeStatusPath(rawPath); - if (p) statusMap.set(p, statusLetter); - if ((statusLetter === 'R' || statusLetter === 'C') && parts.length >= 3) { - const previousPath = normalizeStatusPath(parts[parts.length - 2]); - if (p && previousPath) previousPathMap.set(p, previousPath); + const fields = output.split('\0'); + for (let index = 0; index < fields.length; index++) { + const field = fields[index]; + if (!field) continue; + + if (field.startsWith(':')) { + const statusLetter = field.split(/\s+/).pop()?.charAt(0) ?? 'M'; + const firstPath = fields[++index] ?? ''; + if (statusLetter === 'R' || statusLetter === 'C') { + const destinationPath = fields[++index] ?? ''; + if (destinationPath) { + statusMap.set(destinationPath, statusLetter); + if (firstPath) previousPathMap.set(destinationPath, firstPath); } + } else if (firstPath) { + statusMap.set(firstPath, statusLetter); } continue; } - // --numstat format: "added\tremoved\tpath" - const parts = line.split('\t'); - if (parts.length >= 3) { - const added = parseInt(parts[0], 10); - const removed = parseInt(parts[1], 10); - if (!isNaN(added) && !isNaN(removed)) { - const rawPath = parts[parts.length - 1]; - const normalizedPath = normalizeStatusPath(rawPath); - const p = statusMap.has(normalizedPath) ? normalizedPath : parseNumstatPath(rawPath); - if (p) numstatMap.set(p, [added, removed]); + + const firstTab = field.indexOf('\t'); + const secondTab = firstTab < 0 ? -1 : field.indexOf('\t', firstTab + 1); + if (secondTab < 0) continue; + + const added = Number.parseInt(field.slice(0, firstTab), 10); + const removed = Number.parseInt(field.slice(firstTab + 1, secondTab), 10); + if (!Number.isFinite(added) || !Number.isFinite(removed)) continue; + + let destinationPath = field.slice(secondTab + 1); + if (!destinationPath) { + const previousPath = fields[++index] ?? ''; + destinationPath = fields[++index] ?? ''; + if (destinationPath && previousPath && !previousPathMap.has(destinationPath)) { + previousPathMap.set(destinationPath, previousPath); } } + if (destinationPath) numstatMap.set(destinationPath, [added, removed]); } return { statusMap, numstatMap, previousPathMap }; @@ -1119,7 +1111,7 @@ export async function getChangedFiles( let finalDiffStr = ''; try { - const { stdout } = await exec('git', ['diff', '--raw', '--numstat', diffBase.sha], { + const { stdout } = await exec('git', ['diff', '--raw', '--numstat', '-z', diffBase.sha], { cwd: worktreePath, maxBuffer: MAX_BUFFER, }); @@ -1140,7 +1132,7 @@ export async function getChangedFiles( // git ls-files --others --exclude-standard — untracked files (no index lock needed). // Both commands run in parallel since they are independent. const [uncommittedResult, untrackedResult] = await Promise.all([ - exec('git', ['diff', '--raw', '--numstat', headHash], { + exec('git', ['diff', '--raw', '--numstat', '-z', headHash], { cwd: worktreePath, maxBuffer: MAX_BUFFER, }).catch(() => ({ stdout: '' })), @@ -1319,7 +1311,7 @@ export async function getUncommittedChangedFiles(worktreePath: string): Promise< const headHash = await pinHead(worktreePath); let diffStr = ''; try { - const { stdout } = await exec('git', ['diff', '--raw', '--numstat', headHash], { + const { stdout } = await exec('git', ['diff', '--raw', '--numstat', '-z', headHash], { cwd: worktreePath, maxBuffer: MAX_BUFFER, }); @@ -1816,7 +1808,7 @@ export async function getChangedFilesFromBranch( let diffStr = ''; try { - const { stdout } = await exec('git', ['diff', '--raw', '--numstat', diffRange], { + const { stdout } = await exec('git', ['diff', '--raw', '--numstat', '-z', diffRange], { cwd: projectRoot, maxBuffer: MAX_BUFFER, }); @@ -2034,7 +2026,7 @@ export async function getCommitChangedFiles( try { const { stdout } = await exec( 'git', - ['diff', '--raw', '--numstat', `${commitHash}^..${commitHash}`], + ['diff', '--raw', '--numstat', '-z', `${commitHash}^..${commitHash}`], { cwd: worktreePath, maxBuffer: MAX_BUFFER }, ); diffStr = stdout; @@ -2043,7 +2035,7 @@ export async function getCommitChangedFiles( try { const { stdout } = await exec( 'git', - ['diff', '--raw', '--numstat', `${EMPTY_TREE}..${commitHash}`], + ['diff', '--raw', '--numstat', '-z', `${EMPTY_TREE}..${commitHash}`], { cwd: worktreePath, maxBuffer: MAX_BUFFER }, ); diffStr = stdout; diff --git a/src/components/ChangedFilesList.test.ts b/src/components/ChangedFilesList.test.ts index fef489b8..ec783dcc 100644 --- a/src/components/ChangedFilesList.test.ts +++ b/src/components/ChangedFilesList.test.ts @@ -1,8 +1,10 @@ +import { renderToString } from 'solid-js/web'; import { describe, expect, it } from 'vitest'; -import type { ChangedFile } from '../ipc/types'; +import type { ChangedFile, CoverageFileSummary } from '../ipc/types'; import { coverageFooterLabel, coverageFooterTitle, + FileCoverageBadge, filesFooterLabel, filesFooterTitle, isCoverageEligible, @@ -80,3 +82,28 @@ describe('filesFooterLabel', () => { expect(filesFooterTitle(7, 2)).toBe('7 changed files, 2 uncommitted.'); }); }); + +describe('FileCoverageBadge', () => { + it('keeps task-only coverage visible while comparison inventory is unavailable', () => { + const summary: CoverageFileSummary = { + path: 'src/example.ts', + lines: { total: 100, covered: 82, skipped: 0, pct: 82 }, + statements: { total: 100, covered: 81, skipped: 0, pct: 81 }, + functions: { total: 10, covered: 8, skipped: 0, pct: 80 }, + branches: { total: 20, covered: 15, skipped: 0, pct: 75 }, + }; + + const html = renderToString(() => + FileCoverageBadge({ + file: changedFile({ path: summary.path }), + summary, + comparison: undefined, + hasCoverageArtifact: true, + }), + ); + + expect(html).toContain('82%'); + expect(html).toContain('Lines 82%'); + expect(html).not.toContain('No recent coverage data'); + }); +}); diff --git a/src/components/ChangedFilesList.tsx b/src/components/ChangedFilesList.tsx index f7866833..5467c8f5 100644 --- a/src/components/ChangedFilesList.tsx +++ b/src/components/ChangedFilesList.tsx @@ -158,11 +158,15 @@ function comparisonBadge( const baseLabel = coverageValueLabel(comparison.base); const baselineInformational = isBaselineInformational(baseline); const baselineBranch = baseline?.baseBranch ?? 'base branch'; - const baselineDetail = baseline?.stale - ? ` The base report predates ${baselineBranch} as currently checked out, so this delta is informational only.` - : baseline?.unanchored - ? ` The base report cannot be anchored to ${baselineBranch} as currently checked out, so this delta is informational only.` - : ''; + const baselineDetail = baseline?.taskStale + ? ' The task report predates task HEAD, so this delta is informational only.' + : baseline?.taskUnanchored + ? ' The task report cannot be anchored to task HEAD, so this delta is informational only.' + : baseline?.stale + ? ` The base report predates ${baselineBranch} as currently checked out, so this delta is informational only.` + : baseline?.unanchored + ? ` The base report cannot be anchored to ${baselineBranch} as currently checked out, so this delta is informational only.` + : ''; const renameDetail = comparison.kind === 'renamed' ? ` (${comparison.basePath} → ${comparison.path})` : ''; @@ -210,7 +214,7 @@ function comparisonBadge( }; } -function FileCoverageBadge(props: { +export function FileCoverageBadge(props: { file: ChangedFile; selectedCommit?: CommitSelection; summary?: CoverageFileSummary; @@ -248,6 +252,24 @@ function FileCoverageBadge(props: { )} + + {(coverageSummary) => ( + + {coverageSummary.lines.pct}% + + )} + { if (!projectRoot) return null; - const baseWorktree = await invoke<{ + return invoke<{ path: string; headCommittedAt: string | null; } | null>(IPC.GetBranchWorktreePath, { projectRoot, - branchName: baseBranch, + branchName, }).catch(() => null); +} + +async function resolveBaseCoverageRoot( + projectRoot: string | undefined, + baseBranch: string, + taskRoot: string, +): Promise<{ path: string; headCommittedAt: string | null } | null> { + const baseWorktree = await resolveCoverageWorktree(projectRoot, baseBranch); if (!baseWorktree || baseWorktree.path === taskRoot) return null; return baseWorktree; } @@ -336,7 +365,10 @@ export function ChangedFilesList(props: ChangedFilesListProps) { const [coverage, setCoverage] = createSignal(null); const [baseCoverage, setBaseCoverage] = createSignal(null); const [baseHeadAt, setBaseHeadAt] = createSignal(null); + const [taskHeadAt, setTaskHeadAt] = createSignal(null); const [baseBranchName, setBaseBranchName] = createSignal(); + const [comparisonInventoryState, setComparisonInventoryState] = + createSignal>('loading'); const [canOpenFilesInEditor, setCanOpenFilesInEditor] = createSignal(false); const [selectedIndex, setSelectedIndex] = createSignal(-1); const [collapsed, setCollapsed] = createSignal>(new Set()); @@ -357,14 +389,21 @@ export function ChangedFilesList(props: ChangedFilesListProps) { ); const coverageComparison = createMemo(() => { const inventory = comparisonFiles(); - if (!inventory || isCommitHashSelection(props.selectedCommit)) return null; - return buildCoverageComparison( - coverage(), - baseCoverage(), - inventory, - baseHeadAt(), - baseBranchName(), - ); + const taskReport = coverage(); + const baseReport = baseCoverage(); + if (isCommitHashSelection(props.selectedCommit)) return null; + if (!inventory && !taskReport && !baseReport) return null; + return { + ...buildCoverageComparison( + taskReport, + baseReport, + inventory ?? [], + baseHeadAt(), + baseBranchName(), + taskHeadAt(), + ), + inventoryState: inventory ? 'available' : comparisonInventoryState(), + }; }); const coveredEligibleFiles = createMemo(() => eligibleFiles().filter((file) => Boolean(coverageFiles()[file.path])), @@ -412,6 +451,22 @@ export function ChangedFilesList(props: ChangedFilesListProps) { if (comparison.aggregate.delta !== null) { lines.push(`Delta: ${formatCoverageDelta(comparison.aggregate.delta)}.`); } + if (comparison.inventoryState === 'loading') { + lines.push( + 'The changed-file inventory is still loading, so merge readiness ignores the comparison.', + ); + } else if (comparison.inventoryState === 'failed') { + lines.push( + 'The changed-file inventory is unavailable, so merge readiness ignores the comparison.', + ); + } + if (comparison.baseline?.taskUnanchored) { + lines.push( + 'The task report cannot be anchored to task HEAD, so merge readiness ignores the delta.', + ); + } else if (comparison.baseline?.taskStale) { + lines.push('The task report predates task HEAD, so merge readiness ignores the delta.'); + } if (comparison.baseline?.unanchored) { lines.push( `The base report cannot be anchored to ${comparison.baseline.baseBranch ?? 'the base branch'} as currently checked out, so merge readiness ignores the delta.`, @@ -557,9 +612,14 @@ export function ChangedFilesList(props: ChangedFilesListProps) { // pipelines for every off-screen task. createEffect(() => { void props.worktreePath; + void props.projectRoot; + void props.branchName; void props.baseBranch; void props.selectedCommit; - setComparisonFiles(null); + batch(() => { + setComparisonFiles(null); + setComparisonInventoryState('loading'); + }); }); createEffect(() => { @@ -601,7 +661,12 @@ export function ChangedFilesList(props: ChangedFilesListProps) { } if (uncommittedOnly && path) { - if (!comparisonEnabled && !cancelled) setComparisonFiles(null); + if (!comparisonEnabled && !cancelled) { + batch(() => { + setComparisonFiles(null); + setComparisonInventoryState('loading'); + }); + } const comparisonRequest = comparisonEnabled ? invoke(IPC.GetChangedFiles, { worktreePath: path, @@ -609,13 +674,21 @@ export function ChangedFilesList(props: ChangedFilesListProps) { }) .then((result) => { if (!cancelled) { - setComparisonFiles((current) => - sameChangedFiles(current, result) ? current : result, - ); + batch(() => { + setComparisonFiles((current) => + sameChangedFiles(current, result) ? current : result, + ); + setComparisonInventoryState('available'); + }); } }) .catch(() => { - if (!cancelled) setComparisonFiles(null); + if (!cancelled) { + batch(() => { + setComparisonFiles(null); + setComparisonInventoryState('failed'); + }); + } }) : Promise.resolve(); try { @@ -644,17 +717,23 @@ export function ChangedFilesList(props: ChangedFilesListProps) { baseBranch, }); if (!cancelled) { - setFiles((current) => (sameChangedFiles(current, result) ? current : result)); - setComparisonFiles((current) => - sameChangedFiles(current, result) ? current : result, - ); - setCanOpenFilesInEditor(true); + batch(() => { + setFiles((current) => (sameChangedFiles(current, result) ? current : result)); + setComparisonFiles((current) => + sameChangedFiles(current, result) ? current : result, + ); + setComparisonInventoryState('available'); + setCanOpenFilesInEditor(true); + }); } return; } catch { if (!cancelled) { - setComparisonFiles(null); - setCanOpenFilesInEditor(false); + batch(() => { + setComparisonFiles(null); + setComparisonInventoryState('failed'); + setCanOpenFilesInEditor(false); + }); } // Worktree may not exist — try branch fallback below } @@ -673,22 +752,33 @@ export function ChangedFilesList(props: ChangedFilesListProps) { const displayedFiles = uncommittedOnly ? result.filter((file) => !file.committed) : result; - setFiles((current) => - sameChangedFiles(current, displayedFiles) ? current : displayedFiles, - ); - setComparisonFiles((current) => - sameChangedFiles(current, result) ? current : result, - ); - setCanOpenFilesInEditor(false); + batch(() => { + setFiles((current) => + sameChangedFiles(current, displayedFiles) ? current : displayedFiles, + ); + setComparisonFiles((current) => + sameChangedFiles(current, result) ? current : result, + ); + setComparisonInventoryState('available'); + setCanOpenFilesInEditor(false); + }); + return; } } catch { if (!cancelled) { - setComparisonFiles(null); - setCanOpenFilesInEditor(false); + batch(() => { + setComparisonFiles(null); + setComparisonInventoryState('failed'); + setCanOpenFilesInEditor(false); + }); } // Branch may no longer exist } } + + if (!cancelled && comparisonEnabled) { + setComparisonInventoryState('failed'); + } } finally { inFlight = false; } @@ -712,6 +802,7 @@ export function ChangedFilesList(props: ChangedFilesListProps) { createEffect(() => { const repoRoot = props.worktreePath; const projectRoot = props.projectRoot; + const taskBranch = props.branchName; const baseBranch = props.baseBranch; const selection = props.selectedCommit; if (!repoRoot || isCommitHashSelection(selection)) { @@ -719,6 +810,7 @@ export function ChangedFilesList(props: ChangedFilesListProps) { setCoverage(null); setBaseCoverage(null); setBaseHeadAt(null); + setTaskHeadAt(null); setBaseBranchName(undefined); }); return; @@ -736,8 +828,13 @@ export function ChangedFilesList(props: ChangedFilesListProps) { }).catch(() => null); let baseResult: CoverageSummary | null = null; let baseHeadResult: string | null = null; + let taskHeadResult: string | null = null; let resolvedBaseBranch: string | null = null; if (taskResult) { + const taskWorktree = taskBranch + ? await resolveCoverageWorktree(projectRoot, taskBranch) + : null; + taskHeadResult = taskWorktree?.headCommittedAt ?? null; resolvedBaseBranch = baseBranch ? baseBranch : projectRoot @@ -763,6 +860,7 @@ export function ChangedFilesList(props: ChangedFilesListProps) { sameCoverageSummary(current, baseResult) ? current : baseResult, ); setBaseHeadAt(baseHeadResult); + setTaskHeadAt(taskHeadResult); setBaseBranchName(resolvedBaseBranch ?? undefined); }); } diff --git a/src/components/MergeReadinessPanel.test.ts b/src/components/MergeReadinessPanel.test.ts index 4078fd9f..85d15e19 100644 --- a/src/components/MergeReadinessPanel.test.ts +++ b/src/components/MergeReadinessPanel.test.ts @@ -310,6 +310,87 @@ describe('buildMergeReadiness', () => { expect(readiness.checks[2]).toEqual(expect.objectContaining({ status: 'warning' })); }); + it('keeps coverage informational until an ahead base branch is rebased', () => { + const readiness = buildMergeReadiness( + input({ + mergeStatus: { ...cleanMergeStatus, main_ahead_count: 2 }, + coverage: { + aggregate: { + task: { state: 'available', pct: 78 }, + base: { state: 'available', pct: 82 }, + delta: -4, + }, + files: {}, + impactedUnchangedFiles: [], + }, + }), + ); + + expect(readiness.checks[2]).toEqual( + expect.objectContaining({ + status: 'neutral', + detail: 'main is 2 commits ahead; rebase and regenerate task coverage before comparing.', + }), + ); + }); + + it('keeps a stale task report neutral even when its delta is negative', () => { + const readiness = buildMergeReadiness( + input({ + coverage: { + aggregate: { + task: { state: 'available', pct: 78 }, + base: { state: 'available', pct: 82 }, + delta: -4, + }, + files: {}, + impactedUnchangedFiles: [], + baseline: { + baseBranch: 'main', + stale: false, + taskHeadAt: '2026-07-26T00:00:00.000Z', + taskStale: true, + }, + }, + }), + ); + + expect(readiness.checks[2]).toEqual( + expect.objectContaining({ + status: 'neutral', + detail: 'Task coverage report predates task HEAD; regenerate it before comparing.', + }), + ); + }); + + it.each([ + ['loading', 'still loading'], + ['failed', 'unavailable'], + ] as const)( + 'keeps task coverage visible but neutral when changed-file inventory is %s', + (inventoryState, detailFragment) => { + const readiness = buildMergeReadiness( + input({ + coverage: { + aggregate: { + task: { state: 'available', pct: 78 }, + base: { state: 'available', pct: 82 }, + delta: -4, + }, + files: {}, + impactedUnchangedFiles: [], + inventoryState, + }, + }), + ); + + expect(readiness.checks[2]).toEqual(expect.objectContaining({ status: 'neutral' })); + expect(readiness.checks[2].detail).toContain('Task 78%'); + expect(readiness.checks[2].detail).toContain(detailFragment); + expect(readiness.checks[2].detail).not.toContain('No task coverage report'); + }, + ); + it('keeps a stale base report neutral even when its delta is negative', () => { const readiness = buildMergeReadiness( input({ diff --git a/src/components/merge-readiness.ts b/src/components/merge-readiness.ts index b560eb29..e98c5a55 100644 --- a/src/components/merge-readiness.ts +++ b/src/components/merge-readiness.ts @@ -155,7 +155,10 @@ function prCheck(prChecks?: PrReadinessState): MergeReadinessCheck { }; } -function coverageCheck(coverage?: CoverageComparison | null): MergeReadinessCheck { +function coverageCheck( + coverage?: CoverageComparison | null, + mergeStatus?: MergeStatus, +): MergeReadinessCheck { const aggregate = coverage?.aggregate; if (!aggregate || aggregate.task.state === 'no-report') { return { label: 'Coverage', status: 'neutral', detail: 'No task coverage report.' }; @@ -183,14 +186,35 @@ function coverageCheck(coverage?: CoverageComparison | null): MergeReadinessChec detail: `Task ${taskPct}%; the base report has no executable lines.`, }; } + if (coverage.inventoryState && coverage.inventoryState !== 'available') { + return { + label: 'Coverage', + status: 'neutral', + detail: + coverage.inventoryState === 'loading' + ? `Task ${taskPct}%; changed-file inventory is still loading, so comparison is informational only.` + : `Task ${taskPct}%; changed-file inventory is unavailable, so comparison is informational only.`, + }; + } + if (mergeStatus && mergeStatus.main_ahead_count > 0) { + return { + label: 'Coverage', + status: 'neutral', + detail: `${mergeStatus.base_branch} is ${countLabel(mergeStatus.main_ahead_count, 'commit')} ahead; rebase and regenerate task coverage before comparing.`, + }; + } if (isBaselineInformational(coverage.baseline)) { const baseBranch = coverage.baseline?.baseBranch ?? 'base branch'; return { label: 'Coverage', status: 'neutral', - detail: coverage.baseline?.stale - ? `Base coverage report predates ${baseBranch} as currently checked out; regenerate it before comparing.` - : `Base coverage report cannot be anchored to ${baseBranch} as currently checked out; comparison is informational only.`, + detail: coverage.baseline?.taskStale + ? 'Task coverage report predates task HEAD; regenerate it before comparing.' + : coverage.baseline?.taskUnanchored + ? 'Task coverage report cannot be anchored to task HEAD; comparison is informational only.' + : coverage.baseline?.stale + ? `Base coverage report predates ${baseBranch} as currently checked out; regenerate it before comparing.` + : `Base coverage report cannot be anchored to ${baseBranch} as currently checked out; comparison is informational only.`, }; } @@ -219,7 +243,7 @@ export function buildMergeReadiness(input: MergeReadinessInput): MergeReadiness const checks = [ mergeSafetyCheck(input), verificationCheck(input.verification), - coverageCheck(input.coverage), + coverageCheck(input.coverage, input.mergeStatus), prCheck(input.prChecks), ]; const overall = checks.some((check) => check.status === 'blocked') diff --git a/src/lib/coverage-comparison.test.ts b/src/lib/coverage-comparison.test.ts index f1d36489..88ee312b 100644 --- a/src/lib/coverage-comparison.test.ts +++ b/src/lib/coverage-comparison.test.ts @@ -31,11 +31,15 @@ function file(path: string, pct: number, total = 100): CoverageFileSummary { }; } -function report(totalPct: number, files: CoverageFileSummary[]): CoverageSummary { +function report( + totalPct: number, + files: CoverageFileSummary[], + generatedAt = '2026-07-25T00:00:00.000Z', +): CoverageSummary { const total = metric(totalPct); return { format: 'istanbul-summary', - generatedAt: '2026-07-25T00:00:00.000Z', + generatedAt, reportPath: '/repo/coverage/coverage-summary.json', totals: { lines: total, @@ -234,6 +238,38 @@ describe('buildCoverageComparison', () => { }); }); + it('marks a task report older than task HEAD as stale', () => { + const result = buildCoverageComparison( + report(82, [], '2026-07-25T00:00:00.000Z'), + report(80, []), + [], + '2026-07-24T00:00:00.000Z', + 'main', + '2026-07-26T00:00:00.000Z', + ); + + expect(result.baseline).toMatchObject({ + taskHeadAt: '2026-07-26T00:00:00.000Z', + taskStale: true, + }); + }); + + it('marks a task report with an unknown task HEAD as unanchored', () => { + const result = buildCoverageComparison( + report(82, []), + report(80, []), + [], + '2026-07-24T00:00:00.000Z', + 'main', + null, + ); + + expect(result.baseline).toMatchObject({ + taskStale: false, + taskUnanchored: true, + }); + }); + it.each(['toString', 'constructor', '__proto__'])( 'treats a missing report entry named %s as absent instead of reading Object.prototype', (path) => { diff --git a/src/lib/coverage-comparison.ts b/src/lib/coverage-comparison.ts index 61b0136d..1011e3d2 100644 --- a/src/lib/coverage-comparison.ts +++ b/src/lib/coverage-comparison.ts @@ -37,18 +37,24 @@ export interface CoverageComparison { }; files: Record; impactedUnchangedFiles: ImpactedCoverageFile[]; + inventoryState?: 'available' | 'loading' | 'failed'; baseline?: { baseBranch?: string; baseHeadAt?: string; + taskHeadAt?: string; stale: boolean; unanchored?: boolean; + taskStale?: boolean; + taskUnanchored?: boolean; }; } export const MATERIAL_COVERAGE_DELTA = 1; export function isBaselineInformational(baseline: CoverageComparison['baseline']): boolean { - return Boolean(baseline?.stale || baseline?.unanchored); + return Boolean( + baseline?.stale || baseline?.unanchored || baseline?.taskStale || baseline?.taskUnanchored, + ); } function roundPercentage(value: number): number { @@ -100,6 +106,7 @@ export function buildCoverageComparison( changedFiles: ChangedFile[], baseHeadAt?: string | null, baseBranch?: string, + taskHeadAt?: string | null, ): CoverageComparison { const taskAggregate = aggregateValue(taskSummary); const baseAggregate = aggregateValue(baseSummary); @@ -155,6 +162,20 @@ export function buildCoverageComparison( const baseHeadTime = baseHeadAt ? Date.parse(baseHeadAt) : Number.NaN; const baseGeneratedTime = baseSummary ? Date.parse(baseSummary.generatedAt) : Number.NaN; + const taskHeadTime = taskHeadAt ? Date.parse(taskHeadAt) : Number.NaN; + const taskGeneratedTime = taskSummary ? Date.parse(taskSummary.generatedAt) : Number.NaN; + const taskAnchor = + taskHeadAt === undefined + ? {} + : taskHeadAt && Number.isFinite(taskHeadTime) && Number.isFinite(taskGeneratedTime) + ? { + taskHeadAt, + taskStale: taskGeneratedTime < taskHeadTime, + } + : { + taskStale: false, + taskUnanchored: true, + }; const baseline = !baseSummary ? undefined : baseHeadAt && Number.isFinite(baseHeadTime) && Number.isFinite(baseGeneratedTime) @@ -162,11 +183,13 @@ export function buildCoverageComparison( ...(baseBranch ? { baseBranch } : {}), baseHeadAt, stale: baseGeneratedTime < baseHeadTime, + ...taskAnchor, } : { ...(baseBranch ? { baseBranch } : {}), stale: false, unanchored: true, + ...taskAnchor, }; return { From 087bd8ff65c8348902cc660a3b5a4504edaa9e81 Mon Sep 17 00:00:00 2001 From: Liang Hu Date: Sat, 1 Aug 2026 10:55:46 -0400 Subject: [PATCH 10/10] fix(coverage): hide file deltas without inventory Signed-off-by: Liang Hu --- package-lock.json | 63 +++++++++ package.json | 5 +- .../ChangedFilesList.client.test.tsx | 126 ++++++++++++++++++ src/components/ChangedFilesList.tsx | 30 +++-- vitest.client.config.ts | 10 ++ 5 files changed, 224 insertions(+), 10 deletions(-) create mode 100644 src/components/ChangedFilesList.client.test.tsx create mode 100644 vitest.client.config.ts diff --git a/package-lock.json b/package-lock.json index 4ee8b55b..865eaf20 100644 --- a/package-lock.json +++ b/package-lock.json @@ -47,6 +47,7 @@ "eslint": "^9.39.3", "eslint-config-prettier": "^10.1.8", "eslint-plugin-solid": "^0.14.5", + "happy-dom": "^20.11.1", "husky": "^9.1.7", "knip": "^6.12.2", "lint-staged": "^16.2.7", @@ -3592,6 +3593,13 @@ "license": "MIT", "optional": true }, + "node_modules/@types/whatwg-mimetype": { + "version": "3.0.2", + "resolved": "https://registry.npmjs.org/@types/whatwg-mimetype/-/whatwg-mimetype-3.0.2.tgz", + "integrity": "sha512-c2AKvDT8ToxLIOUlN51gTiHXflsfIFisS4pO7pDPoKouJCESkhZnEy623gwP9laCy5lnLDAw1vAzu2vM2YLOrA==", + "dev": true, + "license": "MIT" + }, "node_modules/@types/ws": { "version": "8.18.1", "resolved": "https://registry.npmjs.org/@types/ws/-/ws-8.18.1.tgz", @@ -4862,6 +4870,19 @@ "dev": true, "license": "MIT" }, + "node_modules/buffer-image-size": { + "version": "0.6.4", + "resolved": "https://registry.npmjs.org/buffer-image-size/-/buffer-image-size-0.6.4.tgz", + "integrity": "sha512-nEh+kZOPY1w+gcCMobZ6ETUp9WfibndnosbpwB1iJk/8Gt5ZF2bhS6+B6bPYz424KtwsR6Rflc3tCz1/ghX2dQ==", + "dev": true, + "license": "MIT", + "dependencies": { + "@types/node": "*" + }, + "engines": { + "node": ">=4.0" + } + }, "node_modules/builder-util": { "version": "26.8.1", "resolved": "https://registry.npmjs.org/builder-util/-/builder-util-26.8.1.tgz", @@ -8294,6 +8315,38 @@ "integrity": "sha512-3GKBOn+m2LX9iq+JC1064cSFprJY4jL1jCXTcpnfER5HYE2l/4EfWSGzkPa/ZDBmYI0ZOEj5VHV/eKnPGkHuOg==", "license": "MIT" }, + "node_modules/happy-dom": { + "version": "20.11.1", + "resolved": "https://registry.npmjs.org/happy-dom/-/happy-dom-20.11.1.tgz", + "integrity": "sha512-XSt8tMzbW9ymE7687xztkO1ckR7qJNQ3LywY9vlYGhGi3zXrGBHuUo2Cl1ztZaICW+1eAGdkLbj6iwVqDT33kg==", + "dev": true, + "license": "MIT", + "dependencies": { + "@types/node": ">=20.0.0", + "@types/whatwg-mimetype": "^3.0.2", + "@types/ws": "^8.18.1", + "buffer-image-size": "^0.6.4", + "entities": "^7.0.1", + "whatwg-mimetype": "^3.0.0", + "ws": "^8.21.0" + }, + "engines": { + "node": ">=20.0.0" + } + }, + "node_modules/happy-dom/node_modules/entities": { + "version": "7.0.1", + "resolved": "https://registry.npmjs.org/entities/-/entities-7.0.1.tgz", + "integrity": "sha512-TWrgLOFUQTH994YUyl1yT4uyavY5nNB5muff+RtWaqNVCAK408b5ZnnbNAUEWLTCpum9w6arT70i1XdQ4UeOPA==", + "dev": true, + "license": "BSD-2-Clause", + "engines": { + "node": ">=0.12" + }, + "funding": { + "url": "https://github.com/fb55/entities?sponsor=1" + } + }, "node_modules/has-flag": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/has-flag/-/has-flag-4.0.0.tgz", @@ -13739,6 +13792,16 @@ "defaults": "^1.0.3" } }, + "node_modules/whatwg-mimetype": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/whatwg-mimetype/-/whatwg-mimetype-3.0.0.tgz", + "integrity": "sha512-nt+N2dzIutVRxARx1nghPKGv1xHikU7HKdfafKkLNLindmPU/ch3U31NOCGGA/dmPcmb1VlofO0vnKAcsm0o/Q==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=12" + } + }, "node_modules/which": { "version": "5.0.0", "resolved": "https://registry.npmjs.org/which/-/which-5.0.0.tgz", diff --git a/package.json b/package.json index 314e7819..37c8362c 100644 --- a/package.json +++ b/package.json @@ -26,7 +26,9 @@ "lint:secrets": "command -v gitleaks >/dev/null 2>&1 && gitleaks detect --config .gitleaks.toml || (echo 'gitleaks not installed (brew install gitleaks)' >&2; exit 1)", "format": "prettier --write .", "format:check": "prettier --check .", - "test": "vitest run", + "test": "npm run test:unit && npm run test:client", + "test:unit": "vitest run", + "test:client": "vitest run --config vitest.client.config.ts", "test:coordinator-pty": "RUN_COORDINATOR_PTY_TEST=1 vitest run electron/mcp/coordinator-real-pty.integration.test.ts", "test:coverage": "vitest run --coverage", "check:coordinator-log": "node scripts/check-coordinator-run.mjs", @@ -75,6 +77,7 @@ "eslint": "^9.39.3", "eslint-config-prettier": "^10.1.8", "eslint-plugin-solid": "^0.14.5", + "happy-dom": "^20.11.1", "husky": "^9.1.7", "knip": "^6.12.2", "lint-staged": "^16.2.7", diff --git a/src/components/ChangedFilesList.client.test.tsx b/src/components/ChangedFilesList.client.test.tsx new file mode 100644 index 00000000..834d4ad3 --- /dev/null +++ b/src/components/ChangedFilesList.client.test.tsx @@ -0,0 +1,126 @@ +import { render } from 'solid-js/web'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { IPC } from '../../electron/ipc/channels'; +import type { ChangedFile, CoverageSummary } from '../ipc/types'; +import type { CoverageComparison } from '../lib/coverage-comparison'; +import { invoke } from '../lib/ipc'; +import { UNCOMMITTED_SELECTION } from './CommitNavBar'; +import { ChangedFilesList } from './ChangedFilesList'; + +vi.mock('../lib/ipc', () => ({ + invoke: vi.fn(), +})); + +const disposers: Array<() => void> = []; + +afterEach(() => { + while (disposers.length > 0) disposers.pop()?.(); + document.body.replaceChildren(); + vi.mocked(invoke).mockReset(); +}); + +function coverageSummary(repoRoot: string, pct: number): CoverageSummary { + return { + format: 'istanbul-summary', + generatedAt: '2026-08-01T12:00:00.000Z', + reportPath: `${repoRoot}/coverage/coverage-summary.json`, + totals: { + lines: { total: 100, covered: pct, skipped: 0, pct }, + statements: { total: 100, covered: pct, skipped: 0, pct }, + functions: { total: 10, covered: Math.round(pct / 10), skipped: 0, pct }, + branches: { total: 20, covered: Math.round(pct / 5), skipped: 0, pct }, + }, + files: { + 'src/example.ts': { + path: 'src/example.ts', + lines: { total: 100, covered: pct, skipped: 0, pct }, + statements: { total: 100, covered: pct, skipped: 0, pct }, + functions: { total: 10, covered: Math.round(pct / 10), skipped: 0, pct }, + branches: { total: 20, covered: Math.round(pct / 5), skipped: 0, pct }, + }, + }, + }; +} + +async function waitFor(predicate: () => boolean): Promise { + for (let attempt = 0; attempt < 50; attempt += 1) { + if (predicate()) return; + await new Promise((resolve) => setTimeout(resolve, 0)); + } + throw new Error('Timed out waiting for mounted ChangedFilesList state'); +} + +describe('ChangedFilesList coverage inventory fallbacks', () => { + it.each(['loading', 'failed'] as const)( + 'suppresses per-file comparison output while inventory is %s', + async (inventoryState) => { + const changedFile: ChangedFile = { + path: 'src/example.ts', + lines_added: 1, + lines_removed: 0, + status: 'M', + committed: false, + }; + const taskCoverage = coverageSummary('/task', 60); + const baseCoverage = coverageSummary('/base', 80); + + vi.mocked(invoke).mockImplementation(((channel: string, args?: Record) => { + if (channel === IPC.GetUncommittedChangedFiles) { + return Promise.resolve([changedFile]); + } + if (channel === IPC.GetChangedFiles) { + return inventoryState === 'loading' + ? new Promise(() => undefined) + : Promise.reject(new Error('inventory unavailable')); + } + if (channel === IPC.GetBranchWorktreePath) { + const branchName = args?.branchName; + return Promise.resolve( + branchName === 'main' + ? { path: '/base', headCommittedAt: '2026-08-01T11:00:00.000Z' } + : { path: '/task', headCommittedAt: '2026-08-01T11:00:00.000Z' }, + ); + } + if (channel === IPC.GetCoverageSummary) { + return Promise.resolve(args?.repoRoot === '/base' ? baseCoverage : taskCoverage); + } + return Promise.reject(new Error(`Unexpected IPC call: ${channel}`)); + }) as typeof invoke); + + const container = document.createElement('div'); + document.body.append(container); + const state: { latestComparison: CoverageComparison | null } = { latestComparison: null }; + disposers.push( + render( + () => ( + { + state.latestComparison = comparison; + }} + /> + ), + container, + ), + ); + + await waitFor( + () => + state.latestComparison?.inventoryState === inventoryState && + container.textContent.includes('base 80%'), + ); + + expect(state.latestComparison?.aggregate.delta).toBe(-20); + expect(Object.keys(state.latestComparison?.files ?? {})).toEqual([]); + expect(state.latestComparison?.impactedUnchangedFiles).toEqual([]); + expect(container.textContent).toContain('base 80% → task 60% (-20pp)'); + expect(container.querySelector('[title^="Lines 60%"]')).not.toBeNull(); + expect(container.textContent).not.toContain('↕'); + }, + ); +}); diff --git a/src/components/ChangedFilesList.tsx b/src/components/ChangedFilesList.tsx index 5467c8f5..8a32845b 100644 --- a/src/components/ChangedFilesList.tsx +++ b/src/components/ChangedFilesList.tsx @@ -393,16 +393,28 @@ export function ChangedFilesList(props: ChangedFilesListProps) { const baseReport = baseCoverage(); if (isCommitHashSelection(props.selectedCommit)) return null; if (!inventory && !taskReport && !baseReport) return null; + const comparison = buildCoverageComparison( + taskReport, + baseReport, + inventory ?? [], + baseHeadAt(), + baseBranchName(), + taskHeadAt(), + ); + const inventoryState = inventory ? 'available' : comparisonInventoryState(); + + if (!inventory) { + return { + ...comparison, + files: Object.create(null) as Record, + impactedUnchangedFiles: [], + inventoryState, + }; + } + return { - ...buildCoverageComparison( - taskReport, - baseReport, - inventory ?? [], - baseHeadAt(), - baseBranchName(), - taskHeadAt(), - ), - inventoryState: inventory ? 'available' : comparisonInventoryState(), + ...comparison, + inventoryState, }; }); const coveredEligibleFiles = createMemo(() => diff --git a/vitest.client.config.ts b/vitest.client.config.ts new file mode 100644 index 00000000..aba8afc5 --- /dev/null +++ b/vitest.client.config.ts @@ -0,0 +1,10 @@ +import { defineConfig } from 'vitest/config'; +import solidPlugin from 'vite-plugin-solid'; + +export default defineConfig({ + plugins: [solidPlugin({ ssr: false })], + test: { + environment: 'happy-dom', + include: ['src/**/*.client.test.tsx'], + }, +});