From 814736a665021f015d186fb2cb2d4b4e6d79baa1 Mon Sep 17 00:00:00 2001 From: "Fredrik Liljegren (Claude Code Opus 5.5)" Date: Fri, 25 Sep 2026 09:16:49 +0200 Subject: [PATCH 1/4] fix: a comparison at a pinned head sees that head's CI and size, and not the discussion since Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Bwp5QefjsjMFeq6CK6cT6w --- package-lock.json | 12 ++++---- packages/api/package.json | 2 +- packages/cli/package.json | 2 +- packages/git/package.json | 2 +- packages/github/package.json | 2 +- packages/parser/package.json | 2 +- packages/ui/package.json | 2 +- scripts/inbox-compare.test.ts | 17 ++++++++++ scripts/inbox-compare.ts | 58 ++++++++++++++++++++++++++++++----- 9 files changed, 80 insertions(+), 19 deletions(-) diff --git a/package-lock.json b/package-lock.json index d3cfa0bf..c4aebb0e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8457,7 +8457,7 @@ }, "packages/api": { "name": "@diffity/api", - "version": "0.10.37", + "version": "0.10.38", "dependencies": { "@diffity/parser": "*" }, @@ -8468,7 +8468,7 @@ }, "packages/cli": { "name": "@naturalcycles/diffity", - "version": "0.10.37", + "version": "0.10.38", "license": "MIT", "dependencies": { "commander": "^14.0.3", @@ -8492,7 +8492,7 @@ }, "packages/git": { "name": "@diffity/git", - "version": "0.10.37", + "version": "0.10.38", "devDependencies": { "@types/node": "^25.5.0", "typescript": "^5.9.3", @@ -8501,7 +8501,7 @@ }, "packages/github": { "name": "@diffity/github", - "version": "0.10.37", + "version": "0.10.38", "dependencies": { "@diffity/api": "*", "@diffity/parser": "*" @@ -8514,7 +8514,7 @@ }, "packages/parser": { "name": "@diffity/parser", - "version": "0.10.37", + "version": "0.10.38", "devDependencies": { "typescript": "^5.9.3", "vitest": "^4.1.0" @@ -8522,7 +8522,7 @@ }, "packages/ui": { "name": "@diffity/ui", - "version": "0.10.37", + "version": "0.10.38", "dependencies": { "@diffity/api": "*", "@diffity/parser": "*", diff --git a/packages/api/package.json b/packages/api/package.json index 3699cd30..4dde05e0 100644 --- a/packages/api/package.json +++ b/packages/api/package.json @@ -1,6 +1,6 @@ { "name": "@diffity/api", - "version": "0.10.37", + "version": "0.10.38", "private": true, "type": "module", "main": "./dist/index.js", diff --git a/packages/cli/package.json b/packages/cli/package.json index e351a659..f2b8e50a 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -1,6 +1,6 @@ { "name": "@naturalcycles/diffity", - "version": "0.10.37", + "version": "0.10.38", "description": "Agent-agnostic, GitHub-style diff viewer and code review tool with a live agent loop", "type": "module", "bin": { diff --git a/packages/git/package.json b/packages/git/package.json index 9fa81a2f..31abca77 100644 --- a/packages/git/package.json +++ b/packages/git/package.json @@ -1,6 +1,6 @@ { "name": "@diffity/git", - "version": "0.10.37", + "version": "0.10.38", "private": true, "type": "module", "main": "./dist/index.js", diff --git a/packages/github/package.json b/packages/github/package.json index 3bc64045..99c05b6b 100644 --- a/packages/github/package.json +++ b/packages/github/package.json @@ -1,6 +1,6 @@ { "name": "@diffity/github", - "version": "0.10.37", + "version": "0.10.38", "private": true, "type": "module", "main": "./dist/index.js", diff --git a/packages/parser/package.json b/packages/parser/package.json index af78d40c..01303d28 100644 --- a/packages/parser/package.json +++ b/packages/parser/package.json @@ -1,6 +1,6 @@ { "name": "@diffity/parser", - "version": "0.10.37", + "version": "0.10.38", "private": true, "type": "module", "main": "./dist/index.js", diff --git a/packages/ui/package.json b/packages/ui/package.json index 6fc1ec7b..a5904140 100644 --- a/packages/ui/package.json +++ b/packages/ui/package.json @@ -1,6 +1,6 @@ { "name": "@diffity/ui", - "version": "0.10.37", + "version": "0.10.38", "type": "module", "private": true, "scripts": { diff --git a/scripts/inbox-compare.test.ts b/scripts/inbox-compare.test.ts index c23a4baf..bb770851 100644 --- a/scripts/inbox-compare.test.ts +++ b/scripts/inbox-compare.test.ts @@ -1,7 +1,9 @@ import { describe, expect, it } from 'vitest'; import type { BundleThread } from '@diffity/api'; +import type { PrSnapshot } from '@diffity/github'; import { UsageError, + atHead, bundleNamesFor, candidateLabel, compareFindings, @@ -384,3 +386,18 @@ describe('the JSON shape', () => { }); }); }); + +describe('atHead', () => { + const now = { + owner: 'o', repo: 'r', number: 7, headSha: 'newest', body: 'rewritten since', + checks: [{ name: 'test', status: 'success' }], additions: 90, deletions: 9, changedFiles: 6, + } as PrSnapshot; + + it('describes the pinned head, not the newest, and drops the description', () => { + const facts = { checks: [{ name: 'test', status: 'failure' as const }], additions: 40, deletions: 2, changedFiles: 3 }; + + expect(atHead(now, 'pinned', facts)).toMatchObject({ + owner: 'o', repo: 'r', number: 7, headSha: 'pinned', body: '', ...facts, + }); + }); +}); diff --git a/scripts/inbox-compare.ts b/scripts/inbox-compare.ts index 1f9e52e0..9f35e703 100644 --- a/scripts/inbox-compare.ts +++ b/scripts/inbox-compare.ts @@ -6,12 +6,13 @@ * what it costs. One agent run per invocation; everything it writes stays in a scratch directory. */ +import { execFileSync } from 'node:child_process'; import { existsSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { basename, dirname, join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; import { GENERAL_THREAD_FILE_PATH, parseReviewBundle, type BundleThread, type ReviewBundle } from '@diffity/api'; -import { viewPr } from '@diffity/github'; +import { parseChecks, viewPr, type PrSnapshot } from '@diffity/github'; import type { RunStats } from '../packages/cli/src/inbox/agent-output.js'; import { DEFAULT_INBOX_CONFIG, expandHome, type InboxConfig } from '../packages/cli/src/inbox/config.js'; import { preparePr, type PrepareResult } from '../packages/cli/src/inbox/prepare.js'; @@ -384,6 +385,44 @@ function readBaseline(options: Options): { path: string; bundle: ReviewBundle } return newest; } +/** What GitHub says about one commit of a pull request, as opposed to the pull request as it is now. */ +export interface HeadFacts { + checks: PrSnapshot['checks']; + additions: number; + deletions: number; + changedFiles: number; +} + +/** + * The pull request as a reviewer saw it at `head`. `gh pr view` describes it as it is now: the + * newest head's CI and size, and a description that may have been rewritten since. The + * description is dropped rather than guessed at, which also keeps runs comparable with the ones + * made before the prompt carried it. + */ +export function atHead(snapshot: PrSnapshot, head: string, facts: HeadFacts): PrSnapshot { + return { ...snapshot, headSha: head, body: '', ...facts }; +} + +function ghJson(args: string[]): unknown { + return JSON.parse(execFileSync('gh', args, { encoding: 'utf8', maxBuffer: 50 * 1024 * 1024 })); +} + +function headFacts(snapshot: PrSnapshot, head: string): HeadFacts { + const base = `repos/${snapshot.owner}/${snapshot.repo}`; + const runs = ghJson(['api', `${base}/commits/${head}/check-runs?per_page=100`]) as { check_runs?: unknown[] }; + const statuses = ghJson(['api', `${base}/commits/${head}/status`]) as { statuses?: unknown[] }; + const compare = ghJson(['api', `${base}/compare/${snapshot.baseRef}...${head}`]) as { + files?: { additions?: number; deletions?: number }[]; + }; + const files = compare.files ?? []; + return { + checks: parseChecks([...(runs.check_runs ?? []), ...(statuses.statuses ?? [])]), + additions: files.reduce((sum, file) => sum + (file.additions ?? 0), 0), + deletions: files.reduce((sum, file) => sum + (file.deletions ?? 0), 0), + changedFiles: files.length, + }; +} + function prName(ref: PrRefSpec): string { return `${ref.owner}/${ref.repo}#${ref.number}`; } @@ -445,11 +484,16 @@ async function main(): Promise { validate: { ...DEFAULT_INBOX_CONFIG.validate, model: null }, }; const candidate = candidateLabel(options.model, options.effort); - const deps = realPrepareDeps( - process.execPath, entry, - worktree => join(scratch, 'data', basename(worktree)), - config, message => console.error(` ${message}`), - ); + const deps = { + ...realPrepareDeps( + process.execPath, entry, + worktree => join(scratch, 'data', basename(worktree)), + config, message => console.error(` ${message}`), + ), + // The discussion as it stands now holds the reviews of this very head, the baseline's findings + // among them, so a candidate given it would be told what it is being scored on finding. + prContext: async () => null, + }; console.error(`🤖 preparing ${prName(options.ref)} at ${short} with ${candidate} — agent running…`); const startedAt = Date.now(); @@ -457,7 +501,7 @@ async function main(): Promise { const ticker = setInterval(() => console.error(` … ${elapsed().toFixed(0)} min`), 60_000); let result: PrepareResult; try { - result = await preparePr({ ...snapshot, headSha: head }, config, deps, { bumped: true, pinHead: head }); + result = await preparePr(atHead(snapshot, head, headFacts(snapshot, head)), config, deps, { bumped: true, pinHead: head }); } finally { clearInterval(ticker); } From 2b3499817a73eb1a4ac3feec76ac9e6e8da941ec Mon Sep 17 00:00:00 2001 From: "Fredrik Liljegren (Claude Code Opus 5.5)" Date: Fri, 25 Sep 2026 09:42:27 +0200 Subject: [PATCH 2/4] fix: a comparison waits up to two minutes for its diffity server Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Bwp5QefjsjMFeq6CK6cT6w --- scripts/inbox-compare.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/scripts/inbox-compare.ts b/scripts/inbox-compare.ts index 9f35e703..f33de3f1 100644 --- a/scripts/inbox-compare.ts +++ b/scripts/inbox-compare.ts @@ -16,7 +16,7 @@ import { parseChecks, viewPr, type PrSnapshot } from '@diffity/github'; import type { RunStats } from '../packages/cli/src/inbox/agent-output.js'; import { DEFAULT_INBOX_CONFIG, expandHome, type InboxConfig } from '../packages/cli/src/inbox/config.js'; import { preparePr, type PrepareResult } from '../packages/cli/src/inbox/prepare.js'; -import { realPrepareDeps } from '../packages/cli/src/inbox/runtime.js'; +import { realPrepareDeps, startDiffityServer } from '../packages/cli/src/inbox/runtime.js'; import { severityOf } from '../packages/cli/src/inbox/summary.js'; import { cloneDir, removeWorktree } from '../packages/cli/src/inbox/worktree.js'; @@ -484,12 +484,13 @@ async function main(): Promise { validate: { ...DEFAULT_INBOX_CONFIG.validate, model: null }, }; const candidate = candidateLabel(options.model, options.effort); + const dataDirFor = (worktree: string) => join(scratch, 'data', basename(worktree)); const deps = { - ...realPrepareDeps( - process.execPath, entry, - worktree => join(scratch, 'data', basename(worktree)), - config, message => console.error(` ${message}`), - ), + ...realPrepareDeps(process.execPath, entry, dataDirFor, config, message => console.error(` ${message}`)), + // Comparisons run back to back on a busy machine, where a large clone can take longer than the + // daemon's 30s to serve; a missed start would be scored as the candidate failing. + startServer: (worktree: string, diffRef: string) => + startDiffityServer(process.execPath, entry, worktree, diffRef, dataDirFor(worktree), 120_000), // The discussion as it stands now holds the reviews of this very head, the baseline's findings // among them, so a candidate given it would be told what it is being scored on finding. prContext: async () => null, From 2db177f4509ede312aa9701f03df972d572d60d3 Mon Sep 17 00:00:00 2001 From: "Fredrik Liljegren (Claude Code Fable 5.1)" Date: Fri, 30 Oct 2026 08:51:35 +0100 Subject: [PATCH 3/4] fix: a pinned head's CI reads every page, and its size comes from the clone The compare endpoint lists at most 300 files, and check runs and statuses page. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Bwp5QefjsjMFeq6CK6cT6w --- scripts/inbox-compare.test.ts | 12 ++++++++++ scripts/inbox-compare.ts | 43 +++++++++++++++++++++++------------ 2 files changed, 40 insertions(+), 15 deletions(-) diff --git a/scripts/inbox-compare.test.ts b/scripts/inbox-compare.test.ts index bb770851..95bf37bf 100644 --- a/scripts/inbox-compare.test.ts +++ b/scripts/inbox-compare.test.ts @@ -7,6 +7,7 @@ import { bundleNamesFor, candidateLabel, compareFindings, + parseShortStat, findingsOf, firstSentence, formatTokens, @@ -401,3 +402,14 @@ describe('atHead', () => { }); }); }); + +describe('parseShortStat', () => { + it('reads files, insertions and deletions from git diff --shortstat', () => { + expect(parseShortStat(' 3 files changed, 40 insertions(+), 2 deletions(-)')).toEqual({ additions: 40, deletions: 2, changedFiles: 3 }); + }); + + it('counts a missing kind of change as zero', () => { + expect(parseShortStat(' 1 file changed, 1 insertion(+)')).toEqual({ additions: 1, deletions: 0, changedFiles: 1 }); + expect(parseShortStat('')).toEqual({ additions: 0, deletions: 0, changedFiles: 0 }); + }); +}); diff --git a/scripts/inbox-compare.ts b/scripts/inbox-compare.ts index f33de3f1..41f4a102 100644 --- a/scripts/inbox-compare.ts +++ b/scripts/inbox-compare.ts @@ -403,26 +403,39 @@ export function atHead(snapshot: PrSnapshot, head: string, facts: HeadFacts): Pr return { ...snapshot, headSha: head, body: '', ...facts }; } -function ghJson(args: string[]): unknown { - return JSON.parse(execFileSync('gh', args, { encoding: 'utf8', maxBuffer: 50 * 1024 * 1024 })); +/** Every page of a paginated endpoint, as gh returns them. */ +function ghPages(path: string): unknown[] { + return JSON.parse(execFileSync('gh', ['api', '--paginate', '--slurp', path], { encoding: 'utf8', maxBuffer: 50 * 1024 * 1024 })) as unknown[]; } -function headFacts(snapshot: PrSnapshot, head: string): HeadFacts { - const base = `repos/${snapshot.owner}/${snapshot.repo}`; - const runs = ghJson(['api', `${base}/commits/${head}/check-runs?per_page=100`]) as { check_runs?: unknown[] }; - const statuses = ghJson(['api', `${base}/commits/${head}/status`]) as { statuses?: unknown[] }; - const compare = ghJson(['api', `${base}/compare/${snapshot.baseRef}...${head}`]) as { - files?: { additions?: number; deletions?: number }[]; - }; - const files = compare.files ?? []; +export function parseShortStat(text: string): Omit { + const count = (pattern: RegExp): number => Number(pattern.exec(text)?.[1] ?? 0); return { - checks: parseChecks([...(runs.check_runs ?? []), ...(statuses.statuses ?? [])]), - additions: files.reduce((sum, file) => sum + (file.additions ?? 0), 0), - deletions: files.reduce((sum, file) => sum + (file.deletions ?? 0), 0), - changedFiles: files.length, + additions: count(/(\d+) insertions?\(\+\)/), + deletions: count(/(\d+) deletions?\(-\)/), + changedFiles: count(/(\d+) files? changed/), }; } +/** + * CI comes from GitHub, the size from the clone: the compare endpoint lists at most 300 files, so + * a larger change would be undercounted there. + */ +function headFacts(snapshot: PrSnapshot, head: string, clone: string): HeadFacts { + const base = `repos/${snapshot.owner}/${snapshot.repo}`; + const runs = ghPages(`${base}/commits/${head}/check-runs?per_page=100`).flatMap(page => (page as { check_runs?: unknown[] }).check_runs ?? []); + const statuses = ghPages(`${base}/commits/${head}/status?per_page=100`).flatMap(page => (page as { statuses?: unknown[] }).statuses ?? []); + const git = (args: string[]): string => execFileSync('git', ['-C', clone, ...args], { encoding: 'utf8' }).trim(); + const baseRef = `refs/diffity/base/${snapshot.baseRef}`; + git(['fetch', 'origin', `+refs/heads/${snapshot.baseRef}:${baseRef}`]); + try { + git(['cat-file', '-e', `${head}^{commit}`]); + } catch { + git(['fetch', 'origin', head]); + } + return { checks: parseChecks([...runs, ...statuses]), ...parseShortStat(git(['diff', '--shortstat', `${baseRef}...${head}`])) }; +} + function prName(ref: PrRefSpec): string { return `${ref.owner}/${ref.repo}#${ref.number}`; } @@ -502,7 +515,7 @@ async function main(): Promise { const ticker = setInterval(() => console.error(` … ${elapsed().toFixed(0)} min`), 60_000); let result: PrepareResult; try { - result = await preparePr(atHead(snapshot, head, headFacts(snapshot, head)), config, deps, { bumped: true, pinHead: head }); + result = await preparePr(atHead(snapshot, head, headFacts(snapshot, head, clone)), config, deps, { bumped: true, pinHead: head }); } finally { clearInterval(ticker); } From 1560c42ee53a64cbadf513809d8f15d0a1eb4734 Mon Sep 17 00:00:00 2001 From: "Fredrik Liljegren (Claude Code Fable 5.1)" Date: Fri, 30 Oct 2026 08:58:13 +0100 Subject: [PATCH 4/4] fix: the pinned head's size is read with --numstat, hooks off --shortstat is localized, and git in the clone runs with the reviewer's credentials. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Bwp5QefjsjMFeq6CK6cT6w --- scripts/inbox-compare.test.ts | 14 +++++++------- scripts/inbox-compare.ts | 28 +++++++++++++++++++--------- 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/scripts/inbox-compare.test.ts b/scripts/inbox-compare.test.ts index 95bf37bf..0b8d2d57 100644 --- a/scripts/inbox-compare.test.ts +++ b/scripts/inbox-compare.test.ts @@ -7,7 +7,7 @@ import { bundleNamesFor, candidateLabel, compareFindings, - parseShortStat, + parseNumStat, findingsOf, firstSentence, formatTokens, @@ -403,13 +403,13 @@ describe('atHead', () => { }); }); -describe('parseShortStat', () => { - it('reads files, insertions and deletions from git diff --shortstat', () => { - expect(parseShortStat(' 3 files changed, 40 insertions(+), 2 deletions(-)')).toEqual({ additions: 40, deletions: 2, changedFiles: 3 }); +describe('parseNumStat', () => { + it('sums the lines and counts the files of git diff --numstat', () => { + expect(parseNumStat('40\t2\tsrc/a.ts\n0\t0\tsrc/b.ts\n')).toEqual({ additions: 40, deletions: 2, changedFiles: 2 }); }); - it('counts a missing kind of change as zero', () => { - expect(parseShortStat(' 1 file changed, 1 insertion(+)')).toEqual({ additions: 1, deletions: 0, changedFiles: 1 }); - expect(parseShortStat('')).toEqual({ additions: 0, deletions: 0, changedFiles: 0 }); + it('counts a binary file as changed without lines, and nothing as nothing', () => { + expect(parseNumStat('-\t-\timage.png\n')).toEqual({ additions: 0, deletions: 0, changedFiles: 1 }); + expect(parseNumStat('')).toEqual({ additions: 0, deletions: 0, changedFiles: 0 }); }); }); diff --git a/scripts/inbox-compare.ts b/scripts/inbox-compare.ts index 41f4a102..ef1364a2 100644 --- a/scripts/inbox-compare.ts +++ b/scripts/inbox-compare.ts @@ -408,13 +408,19 @@ function ghPages(path: string): unknown[] { return JSON.parse(execFileSync('gh', ['api', '--paginate', '--slurp', path], { encoding: 'utf8', maxBuffer: 50 * 1024 * 1024 })) as unknown[]; } -export function parseShortStat(text: string): Omit { - const count = (pattern: RegExp): number => Number(pattern.exec(text)?.[1] ?? 0); - return { - additions: count(/(\d+) insertions?\(\+\)/), - deletions: count(/(\d+) deletions?\(-\)/), - changedFiles: count(/(\d+) files? changed/), - }; +/** `git diff --numstat` summed; a binary file counts as changed with no lines. */ +export function parseNumStat(text: string): Omit { + const size = { additions: 0, deletions: 0, changedFiles: 0 }; + for (const line of text.split('\n')) { + const [added, deleted] = line.split('\t'); + if (added === undefined || deleted === undefined) { + continue; + } + size.changedFiles += 1; + size.additions += Number(added) || 0; + size.deletions += Number(deleted) || 0; + } + return size; } /** @@ -425,7 +431,11 @@ function headFacts(snapshot: PrSnapshot, head: string, clone: string): HeadFacts const base = `repos/${snapshot.owner}/${snapshot.repo}`; const runs = ghPages(`${base}/commits/${head}/check-runs?per_page=100`).flatMap(page => (page as { check_runs?: unknown[] }).check_runs ?? []); const statuses = ghPages(`${base}/commits/${head}/status?per_page=100`).flatMap(page => (page as { statuses?: unknown[] }).statuses ?? []); - const git = (args: string[]): string => execFileSync('git', ['-C', clone, ...args], { encoding: 'utf8' }).trim(); + // Hooks off, as the inbox runs git in the clone: this process holds the reviewer's credentials. + const git = (args: string[]): string => + execFileSync('git', ['-C', clone, '-c', 'core.hooksPath=/dev/null', ...args], { + encoding: 'utf8', maxBuffer: 16 * 1024 * 1024, env: { ...process.env, GIT_TERMINAL_PROMPT: '0' }, + }).trim(); const baseRef = `refs/diffity/base/${snapshot.baseRef}`; git(['fetch', 'origin', `+refs/heads/${snapshot.baseRef}:${baseRef}`]); try { @@ -433,7 +443,7 @@ function headFacts(snapshot: PrSnapshot, head: string, clone: string): HeadFacts } catch { git(['fetch', 'origin', head]); } - return { checks: parseChecks([...runs, ...statuses]), ...parseShortStat(git(['diff', '--shortstat', `${baseRef}...${head}`])) }; + return { checks: parseChecks([...runs, ...statuses]), ...parseNumStat(git(['diff', '--numstat', `${baseRef}...${head}`])) }; } function prName(ref: PrRefSpec): string {