ccantynz/Gluecron.com
CodeIssuesPull RequestsActionsSecurityInsights
✨ AI
Understand
Explain codebaseAsk AIChat with repoClaude sessionsNL searchSemantic searchArchaeologyDebt mapDocs tracking
Build
Spec to PRWorkspaceGenerate testsPropose migrationDev environmentCodebase migrator
Ship
Preview URLsAI release notesAI changelog
More
Repository
CommitsBranchesTagsReleasesPackagesSymbolsStale branches
Planning
MilestonesProjectsDiscussionsWikiContributors
Delivery
GatesMerge queueMerge checksDeploymentsPipelineAgents
Activity
Health reportPulsePush log
Settings

feat(review): lazy per-file diffs for huge PRs + review-thread resolve state #5515

Merged⚡ AI-generatedXSccantynz wants to mergefeat/review-ergonomics→mainopened 10d ago
6 changed files+2004−220
Addeddrizzle/0126_pr_comment_thread_resolution.sql+21−0View fileUnifiedSplit
1-- pr_comments.resolved_at / resolved_by — review-thread resolve state
2-- (2026-08-22, review-ergonomics move #6B).
3--
4-- pr_comments is a flat table: inline review "threads" are the comments
5-- sharing a (file_path, line_number) anchor on one PR, ordered by
6-- created_at. Resolution is thread-root scoped — the two columns are set
7-- on the EARLIEST comment of an anchor group and every later comment in
8-- the group inherits the state by grouping. Non-inline (conversation)
9-- comments never carry these columns.
10--
11-- Display-only: resolution never gates merging, so there is no index —
12-- the columns are only ever read on rows already loaded for the PR page.
13--
14-- Nullable with no default: an existing thread is unresolved. resolved_by
15-- deliberately has no ON DELETE action beyond the FK default; deleting a
16-- user is already blocked while rows reference them elsewhere.
17
18--> statement-breakpoint
19ALTER TABLE "pr_comments" ADD COLUMN IF NOT EXISTS "resolved_at" timestamp with time zone;
20--> statement-breakpoint
21ALTER TABLE "pr_comments" ADD COLUMN IF NOT EXISTS "resolved_by" uuid REFERENCES "users"("id");
Addedsrc/__tests__/pr-review-ergonomics.test.ts+701−0View fileUnifiedSplit
1/**
2 * Review ergonomics — move #6.
3 *
4 * A) Big-PR diff budget (src/lib/pr-diff-budget.ts + pulls.tsx): above the
5 * budget (files > 100 OR patch bytes > 1.5MB) the "Files changed" tab
6 * renders a per-file list whose patches lazy-load from
7 * GET /:owner/:repo/pulls/:number/files/:index/patch; below it the
8 * original monolithic DiffView render is unchanged.
9 *
10 * B) Review-thread resolve state (migration 0126): pr_comments is FLAT, so
11 * a "thread" is the group of approved inline comments sharing a
12 * (filePath, lineNumber) anchor; the earliest comment is the root and
13 * carries resolved_at/resolved_by. Rule: PR author, thread-root author,
14 * or write access. Display-only — never gates merging.
15 *
16 * Pure halves (threshold decision, numstat/name-status parsing, the
17 * permission rule) run everywhere; DB/route tests sit behind
18 * `describe.skipIf(!HAS_DB)` and the git-backed ones seed a throwaway
19 * GIT_REPOS_PATH — both mirror the established patterns in this suite
20 * (pulls-all-filter.test.ts, api-v2-git-plumbing.test.ts).
21 */
22
23import { describe, it, expect, beforeAll, afterAll } from "bun:test";
24import { randomBytes } from "crypto";
25import { join } from "path";
26import { mkdir, rm, writeFile } from "fs/promises";
27
28import {
29 BIG_PR_FILE_COUNT,
30 BIG_PR_PATCH_BYTES,
31 isBigPrDiff,
32 parseNumstat,
33 parseNameStatus,
34 buildPrDiffSummary,
35} from "../lib/pr-diff-budget";
36import { canResolvePrThread } from "../routes/pulls";
37
38const HAS_DB = Boolean(process.env.DATABASE_URL);
39
40// ---------------------------------------------------------------------------
41// A1 — threshold decision (pure)
42// ---------------------------------------------------------------------------
43
44describe("pr-diff-budget — isBigPrDiff", () => {
45 it("boundary values are NOT big (exactly 100 files / exactly 1.5MB)", () => {
46 expect(
47 isBigPrDiff({ fileCount: BIG_PR_FILE_COUNT, patchBytes: 0 })
48 ).toBe(false);
49 expect(
50 isBigPrDiff({ fileCount: 1, patchBytes: BIG_PR_PATCH_BYTES })
51 ).toBe(false);
52 });
53
54 it("either bound alone trips the budget", () => {
55 expect(
56 isBigPrDiff({ fileCount: BIG_PR_FILE_COUNT + 1, patchBytes: 0 })
57 ).toBe(true);
58 expect(
59 isBigPrDiff({ fileCount: 2, patchBytes: BIG_PR_PATCH_BYTES + 1 })
60 ).toBe(true);
61 });
62
63 it("an ordinary PR is not big", () => {
64 expect(isBigPrDiff({ fileCount: 12, patchBytes: 40_000 })).toBe(false);
65 });
66});
67
68// ---------------------------------------------------------------------------
69// A2 — per-file summary parsing (pure)
70// ---------------------------------------------------------------------------
71
72describe("pr-diff-budget — numstat / name-status parsing", () => {
73 const NUMSTAT =
74 "10\t2\tsrc/a.ts\n" +
75 "-\t-\tassets/logo.png\n" +
76 "3\t1\tsrc/{old.ts => new.ts}\n" +
77 "5\t0\tplain-old.ts => plain-new.ts\n";
78
79 it("parseNumstat: counts, binary '-' markers, brace and plain renames", () => {
80 const files = parseNumstat(NUMSTAT);
81 expect(files).toHaveLength(4);
82
83 expect(files[0]).toMatchObject({
84 path: "src/a.ts",
85 oldPath: null,
86 additions: 10,
87 deletions: 2,
88 binary: false,
89 status: "changed",
90 });
91 expect(files[1]).toMatchObject({
92 path: "assets/logo.png",
93 binary: true,
94 additions: 0,
95 deletions: 0,
96 status: "binary",
97 });
98 expect(files[2]).toMatchObject({
99 path: "src/new.ts",
100 oldPath: "src/old.ts",
101 status: "renamed",
102 });
103 expect(files[3]).toMatchObject({
104 path: "plain-new.ts",
105 oldPath: "plain-old.ts",
106 status: "renamed",
107 });
108 });
109
110 it("parseNumstat: empty input yields an empty list", () => {
111 expect(parseNumstat("")).toHaveLength(0);
112 expect(parseNumstat("\n")).toHaveLength(0);
113 });
114
115 it("parseNameStatus: A/M/D and rename lines key by the right path", () => {
116 const map = parseNameStatus(
117 "A\tsrc/added.ts\nM\tsrc/a.ts\nD\tsrc/gone.ts\nR100\tsrc/old.ts\tsrc/new.ts\n"
118 );
119 expect(map.get("src/added.ts")).toBe("added");
120 expect(map.get("src/a.ts")).toBe("modified");
121 expect(map.get("src/gone.ts")).toBe("deleted");
122 expect(map.get("src/new.ts")).toBe("renamed");
123 expect(map.has("src/old.ts")).toBe(false);
124 });
125
126 it("buildPrDiffSummary: name-status statuses win; binary stays binary; no data degrades to 'changed'", () => {
127 const files = buildPrDiffSummary(
128 "1\t0\tsrc/added.ts\n4\t2\tsrc/a.ts\n-\t-\tassets/logo.png\n2\t2\tmystery.ts\n",
129 "A\tsrc/added.ts\nM\tsrc/a.ts\nM\tassets/logo.png\n"
130 );
131 const byPath = new Map(files.map((f) => [f.path, f]));
132 expect(byPath.get("src/added.ts")?.status).toBe("added");
133 expect(byPath.get("src/a.ts")?.status).toBe("modified");
134 // Binary + "M" keeps the honest binary pill.
135 expect(byPath.get("assets/logo.png")?.status).toBe("binary");
136 // No name-status row (e.g. merged-recovery range): no invented status.
137 expect(byPath.get("mystery.ts")?.status).toBe("changed");
138 });
139});
140
141// ---------------------------------------------------------------------------
142// B1 — resolve permission rule (pure)
143// ---------------------------------------------------------------------------
144
145describe("canResolvePrThread", () => {
146 const prAuthorId = "00000000-0000-4000-8000-00000000000a";
147 const threadAuthorId = "00000000-0000-4000-8000-00000000000b";
148 const strangerId = "00000000-0000-4000-8000-00000000000c";
149
150 it("anonymous viewers never resolve", () => {
151 expect(
152 canResolvePrThread({
153 viewerId: null,
154 prAuthorId,
155 threadAuthorId,
156 viewerAccess: "owner",
157 })
158 ).toBe(false);
159 });
160
161 it("the PR author may resolve, even with only read access", () => {
162 expect(
163 canResolvePrThread({
164 viewerId: prAuthorId,
165 prAuthorId,
166 threadAuthorId,
167 viewerAccess: "read",
168 })
169 ).toBe(true);
170 });
171
172 it("the thread root's author may resolve, even with only read access", () => {
173 expect(
174 canResolvePrThread({
175 viewerId: threadAuthorId,
176 prAuthorId,
177 threadAuthorId,
178 viewerAccess: "read",
179 })
180 ).toBe(true);
181 });
182
183 it("write access (and above) may resolve", () => {
184 for (const access of ["write", "admin", "owner"]) {
185 expect(
186 canResolvePrThread({
187 viewerId: strangerId,
188 prAuthorId,
189 threadAuthorId,
190 viewerAccess: access,
191 })
192 ).toBe(true);
193 }
194 });
195
196 it("a read-only stranger may not resolve", () => {
197 for (const access of ["none", "read"]) {
198 expect(
199 canResolvePrThread({
200 viewerId: strangerId,
201 prAuthorId,
202 threadAuthorId,
203 viewerAccess: access,
204 })
205 ).toBe(false);
206 }
207 });
208});
209
210// ---------------------------------------------------------------------------
211// A2b / B1b — server-side renders of the new view components (no DB, no git)
212// ---------------------------------------------------------------------------
213
214describe("diff-view — big-PR components render", () => {
215 it("DiffFileListView emits lazy cards with indexed patch URLs", async () => {
216 const { DiffFileListView } = await import("../views/diff-view");
217 const html = String(
218 DiffFileListView({
219 files: [
220 { path: "src/a.ts", oldPath: null, status: "modified", additions: 3, deletions: 1, binary: false },
221 { path: "src/new.ts", oldPath: "src/old.ts", status: "renamed", additions: 0, deletions: 0, binary: false },
222 ],
223 patchUrlBase: "/o/r/pulls/7/files",
224 viewFileBase: "/o/r/blob/feature",
225 })
226 );
227 expect(html).toContain("diff-view-lazy");
228 expect(html).toContain("diff-file-lazy");
229 expect(html).toContain("/o/r/pulls/7/files/0/patch?path=src%2Fa.ts");
230 expect(html).toContain("/o/r/pulls/7/files/1/patch?path=src%2Fnew.ts");
231 expect(html).toContain("src/old.ts"); // rename shows old → new
232 expect(html).toContain("each file loads when expanded");
233 });
234
235 it("SingleFileDiffFragment renders one file's hunks, and thread state collapses resolved threads", async () => {
236 const { SingleFileDiffFragment } = await import("../views/diff-view");
237 const raw = [
238 "diff --git a/src/a.ts b/src/a.ts",
239 "index 0000000..1111111 100644",
240 "--- a/src/a.ts",
241 "+++ b/src/a.ts",
242 "@@ -1,2 +1,3 @@",
243 " line one",
244 "+line two",
245 " line three",
246 "",
247 ].join("\n");
248 const mkComment = (thread?: object) => ({
249 id: "c1",
250 filePath: "src/a.ts",
251 lineNumber: 2,
252 authorUsername: "alice",
253 body: "<p>looks good</p>",
254 createdAt: new Date().toISOString(),
255 ...(thread ? { thread } : {}),
256 });
257
258 const resolvedHtml = String(
259 SingleFileDiffFragment({
260 raw,
261 inlineComments: [
262 mkComment({
263 resolved: true,
264 resolvedByUsername: "bob",
265 canResolve: true,
266 resolveUrl: "/o/r/pulls/7/comments/c1/resolve",
267 unresolveUrl: "/o/r/pulls/7/comments/c1/unresolve",
268 }) as never,
269 ],
270 })
271 );
272 expect(resolvedHtml).toContain("diff-body");
273 expect(resolvedHtml).toContain("diff-row-add");
274 expect(resolvedHtml).toContain("diff-thread-resolved");
275 expect(resolvedHtml).toContain("Resolved");
276 expect(resolvedHtml).toContain("by @bob");
277 expect(resolvedHtml).toContain("/o/r/pulls/7/comments/c1/unresolve");
278
279 const openHtml = String(
280 SingleFileDiffFragment({
281 raw,
282 inlineComments: [
283 mkComment({
284 resolved: false,
285 resolvedByUsername: null,
286 canResolve: true,
287 resolveUrl: "/o/r/pulls/7/comments/c1/resolve",
288 unresolveUrl: "/o/r/pulls/7/comments/c1/unresolve",
289 }) as never,
290 ],
291 })
292 );
293 expect(openHtml).not.toContain("diff-thread-resolved");
294 expect(openHtml).toContain("Resolve thread");
295 expect(openHtml).toContain("/o/r/pulls/7/comments/c1/resolve");
296
297 // A viewer without resolve permission sees no buttons at all.
298 const noPermHtml = String(
299 SingleFileDiffFragment({
300 raw,
301 inlineComments: [
302 mkComment({
303 resolved: false,
304 resolvedByUsername: null,
305 canResolve: false,
306 resolveUrl: "/o/r/pulls/7/comments/c1/resolve",
307 unresolveUrl: "/o/r/pulls/7/comments/c1/unresolve",
308 }) as never,
309 ],
310 })
311 );
312 expect(noPermHtml).not.toContain("Resolve thread");
313 });
314});
315
316// ---------------------------------------------------------------------------
317// Shared DB fixtures
318// ---------------------------------------------------------------------------
319
320/** Session-cookie POST headers that satisfy the csrf same-origin check. */
321function postHeaders(token: string): Record<string, string> {
322 return {
323 cookie: `session=${token}`,
324 host: "localhost",
325 origin: "http://localhost",
326 "content-type": "application/x-www-form-urlencoded",
327 };
328}
329
330async function mkUser(prefix: string) {
331 const { db } = await import("../db");
332 const { users } = await import("../db/schema");
333 const stamp = randomBytes(4).toString("hex");
334 const [u] = await db
335 .insert(users)
336 .values({
337 username: `${prefix}-${stamp}`,
338 email: `${prefix}-${stamp}@test.local`,
339 passwordHash: "x",
340 })
341 .returning();
342 return u!;
343}
344
345async function mkSession(userId: string) {
346 const { db } = await import("../db");
347 const { sessions } = await import("../db/schema");
348 const token = `revergo-${randomBytes(12).toString("hex")}`;
349 await db.insert(sessions).values({
350 userId,
351 token,
352 expiresAt: new Date(Date.now() + 10 * 60_000),
353 });
354 return token;
355}
356
357// ---------------------------------------------------------------------------
358// B2 — resolve / unresolve routes (DB)
359// ---------------------------------------------------------------------------
360
361describe.skipIf(!HAS_DB)("POST …/comments/:commentId/(un)resolve", () => {
362 async function seed() {
363 const { db } = await import("../db");
364 const { repositories, pullRequests, prComments } = await import(
365 "../db/schema"
366 );
367 const owner = await mkUser("rvo");
368 const stranger = await mkUser("rvs");
369 const stamp = randomBytes(4).toString("hex");
370 const [repo] = await db
371 .insert(repositories)
372 .values({
373 name: `rv-${stamp}`,
374 ownerId: owner.id,
375 diskPath: `/tmp/rv-${stamp}.git`,
376 defaultBranch: "main",
377 })
378 .returning();
379 const [pr] = await db
380 .insert(pullRequests)
381 .values({
382 repositoryId: repo!.id,
383 authorId: owner.id,
384 title: "Resolve-state fixture PR",
385 body: "",
386 baseBranch: "main",
387 headBranch: "feature",
388 state: "open",
389 })
390 .returning();
391 // Thread root (owner) + a later reply on the same anchor (stranger).
392 const [root] = await db
393 .insert(prComments)
394 .values({
395 pullRequestId: pr!.id,
396 authorId: owner.id,
397 body: "root of the thread",
398 filePath: "src/a.ts",
399 lineNumber: 3,
400 })
401 .returning();
402 const [reply] = await db
403 .insert(prComments)
404 .values({
405 pullRequestId: pr!.id,
406 authorId: stranger.id,
407 body: "a reply on the same anchor",
408 filePath: "src/a.ts",
409 lineNumber: 3,
410 })
411 .returning();
412 return { db, prComments, owner, stranger, repo: repo!, pr: pr!, root: root!, reply: reply! };
413 }
414
415 it("PR author resolves the thread; unresolve clears it", async () => {
416 const app = (await import("../app")).default;
417 const { eq } = await import("drizzle-orm");
418 const fx = await seed();
419 const token = await mkSession(fx.owner.id);
420 const base = `/${fx.owner.username}/${fx.repo.name}/pulls/${fx.pr.number}`;
421
422 const res = await app.request(`${base}/comments/${fx.root.id}/resolve`, {
423 method: "POST",
424 headers: postHeaders(token),
425 body: new URLSearchParams({}),
426 });
427 expect(res.status).toBe(302);
428 expect(res.headers.get("location") || "").toContain("tab=files");
429 expect(res.headers.get("location") || "").not.toContain("error=");
430
431 let [row] = await fx.db
432 .select()
433 .from(fx.prComments)
434 .where(eq(fx.prComments.id, fx.root.id))
435 .limit(1);
436 expect(row?.resolvedAt).not.toBeNull();
437 expect(row?.resolvedBy).toBe(fx.owner.id);
438
439 const res2 = await app.request(
440 `${base}/comments/${fx.root.id}/unresolve`,
441 {
442 method: "POST",
443 headers: postHeaders(token),
444 body: new URLSearchParams({}),
445 }
446 );
447 expect(res2.status).toBe(302);
448 [row] = await fx.db
449 .select()
450 .from(fx.prComments)
451 .where(eq(fx.prComments.id, fx.root.id))
452 .limit(1);
453 expect(row?.resolvedAt).toBeNull();
454 expect(row?.resolvedBy).toBeNull();
455 });
456
457 it("a read-only stranger (not PR/thread author) is refused", async () => {
458 const app = (await import("../app")).default;
459 const { eq } = await import("drizzle-orm");
460 const fx = await seed();
461 const token = await mkSession(fx.stranger.id);
462 const base = `/${fx.owner.username}/${fx.repo.name}/pulls/${fx.pr.number}`;
463
464 const res = await app.request(`${base}/comments/${fx.root.id}/resolve`, {
465 method: "POST",
466 headers: postHeaders(token),
467 body: new URLSearchParams({}),
468 });
469 expect(res.status).toBe(302);
470 expect(res.headers.get("location") || "").toContain("error=");
471
472 const [row] = await fx.db
473 .select()
474 .from(fx.prComments)
475 .where(eq(fx.prComments.id, fx.root.id))
476 .limit(1);
477 expect(row?.resolvedAt).toBeNull();
478 });
479
480 it("resolving a REPLY canonicalises to the thread root", async () => {
481 const app = (await import("../app")).default;
482 const { eq } = await import("drizzle-orm");
483 const fx = await seed();
484 const token = await mkSession(fx.owner.id);
485 const base = `/${fx.owner.username}/${fx.repo.name}/pulls/${fx.pr.number}`;
486
487 const res = await app.request(`${base}/comments/${fx.reply.id}/resolve`, {
488 method: "POST",
489 headers: postHeaders(token),
490 body: new URLSearchParams({}),
491 });
492 expect(res.status).toBe(302);
493 expect(res.headers.get("location") || "").not.toContain("error=");
494
495 const [rootRow] = await fx.db
496 .select()
497 .from(fx.prComments)
498 .where(eq(fx.prComments.id, fx.root.id))
499 .limit(1);
500 const [replyRow] = await fx.db
501 .select()
502 .from(fx.prComments)
503 .where(eq(fx.prComments.id, fx.reply.id))
504 .limit(1);
505 expect(rootRow?.resolvedAt).not.toBeNull();
506 expect(replyRow?.resolvedAt).toBeNull();
507 });
508
509 it("the conversation tab shows the thread-resolution summary", async () => {
510 const app = (await import("../app")).default;
511 const fx = await seed();
512 const token = await mkSession(fx.owner.id);
513 const base = `/${fx.owner.username}/${fx.repo.name}/pulls/${fx.pr.number}`;
514
515 // Resolve first, then render.
516 await app.request(`${base}/comments/${fx.root.id}/resolve`, {
517 method: "POST",
518 headers: postHeaders(token),
519 body: new URLSearchParams({}),
520 });
521 const page = await app.request(base, {
522 headers: { cookie: `session=${token}` },
523 });
524 expect(page.status).toBe(200);
525 const html = await page.text();
526 expect(html).toContain("1 of 1 review thread resolved");
527 });
528});
529
530// ---------------------------------------------------------------------------
531// A3 — big-PR files tab + per-file patch fragment (DB + throwaway git repos)
532// ---------------------------------------------------------------------------
533
534const TEST_REPOS = join(
535 import.meta.dir,
536 "../../.test-repos-review-ergonomics-" + Date.now()
537);
538let prevReposPath: string | undefined;
539
540async function gitRun(cmd: string[], cwd: string) {
541 const proc = Bun.spawn(cmd, {
542 cwd,
543 stdout: "pipe",
544 stderr: "pipe",
545 env: {
546 ...process.env,
547 GIT_AUTHOR_NAME: "t",
548 GIT_AUTHOR_EMAIL: "t@t",
549 GIT_COMMITTER_NAME: "t",
550 GIT_COMMITTER_EMAIL: "t@t",
551 },
552 });
553 const code = await proc.exited;
554 if (code !== 0) {
555 const err = await new Response(proc.stderr).text();
556 throw new Error(`git fixture command failed (exit ${code}): ${err}`);
557 }
558}
559
560describe.skipIf(!HAS_DB)("big-PR lazy diff + patch fragment", () => {
561 beforeAll(async () => {
562 prevReposPath = process.env.GIT_REPOS_PATH;
563 process.env.GIT_REPOS_PATH = TEST_REPOS;
564 await rm(TEST_REPOS, { recursive: true, force: true });
565 await mkdir(TEST_REPOS, { recursive: true });
566 });
567
568 afterAll(async () => {
569 if (prevReposPath === undefined) delete process.env.GIT_REPOS_PATH;
570 else process.env.GIT_REPOS_PATH = prevReposPath;
571 await rm(TEST_REPOS, { recursive: true, force: true });
572 });
573
574 /**
575 * Seed one bare repo with a main branch (1 file) and two feature
576 * branches: `feature-small` (3 changed files) and `feature-big`
577 * (BIG_PR_FILE_COUNT + 5 changed files), plus matching DB rows and two
578 * open PRs. The repo is public so the anonymous read path — the same
579 * gating the PR detail page uses — is what the requests exercise.
580 */
581 async function seedGitPrs() {
582 const { db } = await import("../db");
583 const { repositories, pullRequests } = await import("../db/schema");
584 const { initBareRepo, getRepoPath } = await import("../git/repository");
585
586 const owner = await mkUser("bigd");
587 const repoName = `bigd-${randomBytes(4).toString("hex")}`;
588 await initBareRepo(owner.username, repoName);
589 const bare = getRepoPath(owner.username, repoName);
590
591 const work = join(TEST_REPOS, "_work_" + randomBytes(4).toString("hex"));
592 await mkdir(work, { recursive: true });
593 await gitRun(["git", "clone", bare, work], TEST_REPOS);
594 await gitRun(["git", "checkout", "-B", "main"], work);
595 await writeFile(join(work, "readme.md"), "hello\n");
596 await gitRun(["git", "add", "."], work);
597 await gitRun(["git", "commit", "-q", "-m", "base"], work);
598 await gitRun(["git", "push", "-q", "origin", "main"], work);
599
600 // Small branch — 3 files.
601 await gitRun(["git", "checkout", "-q", "-b", "feature-small"], work);
602 for (let i = 0; i < 3; i++) {
603 await writeFile(join(work, `small-${i}.txt`), `small ${i}\n`);
604 }
605 await gitRun(["git", "add", "."], work);
606 await gitRun(["git", "commit", "-q", "-m", "small change"], work);
607 await gitRun(["git", "push", "-q", "origin", "feature-small"], work);
608
609 // Big branch — over the file-count budget.
610 await gitRun(["git", "checkout", "-q", "main"], work);
611 await gitRun(["git", "checkout", "-q", "-b", "feature-big"], work);
612 for (let i = 0; i < BIG_PR_FILE_COUNT + 5; i++) {
613 await writeFile(join(work, `big-${i}.txt`), `big file ${i}\n`);
614 }
615 await gitRun(["git", "add", "."], work);
616 await gitRun(["git", "commit", "-q", "-m", "big change"], work);
617 await gitRun(["git", "push", "-q", "origin", "feature-big"], work);
618
619 const [repo] = await db
620 .insert(repositories)
621 .values({
622 name: repoName,
623 ownerId: owner.id,
624 diskPath: bare,
625 defaultBranch: "main",
626 })
627 .returning();
628 const [smallPr] = await db
629 .insert(pullRequests)
630 .values({
631 repositoryId: repo!.id,
632 authorId: owner.id,
633 title: "Small PR",
634 body: "",
635 baseBranch: "main",
636 headBranch: "feature-small",
637 state: "open",
638 })
639 .returning();
640 const [bigPr] = await db
641 .insert(pullRequests)
642 .values({
643 repositoryId: repo!.id,
644 authorId: owner.id,
645 title: "Big PR",
646 body: "",
647 baseBranch: "main",
648 headBranch: "feature-big",
649 state: "open",
650 })
651 .returning();
652 return { owner, repoName, smallPr: smallPr!, bigPr: bigPr! };
653 }
654
655 it("above the budget the tab renders the lazy list; below it the full DiffView", async () => {
656 const app = (await import("../app")).default;
657 const fx = await seedGitPrs();
658 const base = `/${fx.owner.username}/${fx.repoName}/pulls`;
659
660 const big = await app.request(`${base}/${fx.bigPr.number}?tab=files`);
661 expect(big.status).toBe(200);
662 const bigHtml = await big.text();
663 expect(bigHtml).toContain("diff-view-lazy");
664 expect(bigHtml).toContain(`/pulls/${fx.bigPr.number}/files/0/patch`);
665 expect(bigHtml).toContain("each file loads when expanded");
666
667 const small = await app.request(`${base}/${fx.smallPr.number}?tab=files`);
668 expect(small.status).toBe(200);
669 const smallHtml = await small.text();
670 expect(smallHtml).not.toContain("diff-view-lazy");
671 expect(smallHtml).toContain("diff-view");
672 // The full render still ships the actual patch inline.
673 expect(smallHtml).toContain("small-0.txt");
674 }, 120_000);
675
676 it("the patch fragment returns one file's rendered diff", async () => {
677 const app = (await import("../app")).default;
678 const fx = await seedGitPrs();
679 const base = `/${fx.owner.username}/${fx.repoName}/pulls/${fx.bigPr.number}`;
680
681 const frag = await app.request(`${base}/files/0/patch`);
682 expect(frag.status).toBe(200);
683 const html = await frag.text();
684 expect(html).toContain("diff-body");
685 expect(html).toContain("diff-row");
686 // A fragment, not a page: no document chrome.
687 expect(html).not.toContain("<html");
688
689 // Out-of-range index without a path hint → 404.
690 const miss = await app.request(`${base}/files/99999/patch`);
691 expect(miss.status).toBe(404);
692
693 // Wrong index but a correct ?path= falls back to the path.
694 const byPath = await app.request(
695 `${base}/files/99999/patch?path=${encodeURIComponent("big-1.txt")}`
696 );
697 expect(byPath.status).toBe(200);
698 const byPathHtml = await byPath.text();
699 expect(byPathHtml).toContain("big file 1");
700 }, 120_000);
701});
Modifiedsrc/db/schema.ts+6−0View fileUnifiedSplit
845845 }),
846846 createdAt: timestamp("created_at").defaultNow().notNull(),
847847 updatedAt: timestamp("updated_at").defaultNow().notNull(),
848 // Migration 0126 (2026-08-22) — review-thread resolve state. Threads are
849 // the comments sharing a (filePath, lineNumber) anchor; both columns are
850 // set on the thread ROOT (earliest comment of the anchor group) and
851 // replies inherit by grouping. Display-only — never gates merging.
852 resolvedAt: timestamp("resolved_at", { withTimezone: true }),
853 resolvedBy: uuid("resolved_by").references(() => users.id),
848854 },
849855 (table) => [index("pr_comments_pr").on(table.pullRequestId)]
850856);
Addedsrc/lib/pr-diff-budget.ts+142−0View fileUnifiedSplit
1/**
2 * PR diff render budget — move #6A (review ergonomics).
3 *
4 * The PR "Files changed" tab used to render the entire diff server-side in
5 * one page, which on monster PRs produced tens of megabytes of HTML and a
6 * dead browser tab. Above the budget defined here, pulls.tsx switches to a
7 * per-file summary list whose patches lazy-load one file at a time from
8 * `GET /:owner/:repo/pulls/:number/files/:index/patch`.
9 *
10 * Everything in this module is pure (no git, no DB, no JSX) so the
11 * threshold decision and the numstat/name-status parsing are directly
12 * unit-testable. The single source of truth for the budget lives here —
13 * do not re-declare these constants at a call site.
14 */
15
16/** More changed files than this ⇒ big-PR mode. 100 files keeps the
17 * full render for every ordinary PR while catching generated-code and
18 * vendored-dependency monsters. */
19export const BIG_PR_FILE_COUNT = 100;
20
21/** A raw patch larger than this (bytes) ⇒ big-PR mode, regardless of file
22 * count — a 2-file PR that vendors a 5MB bundle is just as unrenderable
23 * as a 500-file one. */
24export const BIG_PR_PATCH_BYTES = 1_500_000;
25
26/**
27 * The threshold decision: files > BIG_PR_FILE_COUNT OR patch bytes >
28 * BIG_PR_PATCH_BYTES. Boundary values themselves are NOT big — a PR with
29 * exactly 100 files and exactly 1.5MB of patch still gets the full render.
30 */
31export function isBigPrDiff(args: {
32 fileCount: number;
33 patchBytes: number;
34}): boolean {
35 return args.fileCount > BIG_PR_FILE_COUNT || args.patchBytes > BIG_PR_PATCH_BYTES;
36}
37
38/** One row of the big-PR per-file list. */
39export interface PrDiffFileSummary {
40 /** Path shown to the user (the new path for renames). */
41 path: string;
42 /** Original path on a rename, else null. */
43 oldPath: string | null;
44 /** added | modified | renamed | deleted | binary | changed.
45 * "changed" is the honest fallback when no name-status data exists
46 * (e.g. the merged-PR recovery range) — we don't guess. */
47 status: "added" | "modified" | "renamed" | "deleted" | "binary" | "changed";
48 additions: number;
49 deletions: number;
50 binary: boolean;
51}
52
53/**
54 * Expand git's brace rename shorthand: `dir/{old.ts => new.ts}` (and the
55 * braceless `old => new` form) into [oldPath, newPath]. Returns null when
56 * the path carries no rename marker.
57 */
58function splitRenamePath(raw: string): [string, string] | null {
59 const brace = raw.match(/^(.*)\{(.*) => (.*)\}(.*)$/);
60 if (brace) {
61 const [, pre, oldMid, newMid, post] = brace;
62 // `{old => }` / `{ => new}` collapse to nothing on the empty side;
63 // git also emits `pre/{ => mid}/post` — join and squeeze the `//`.
64 const oldPath = `${pre}${oldMid}${post}`.replace(/\/\//g, "/");
65 const newPath = `${pre}${newMid}${post}`.replace(/\/\//g, "/");
66 return [oldPath, newPath];
67 }
68 const plain = raw.match(/^(.+) => (.+)$/);
69 if (plain) return [plain[1], plain[2]];
70 return null;
71}
72
73/**
74 * Parse `git diff --numstat` output. Binary files report "-" for both
75 * counts; renames use the `=>` shorthand in the path column.
76 */
77export function parseNumstat(raw: string): PrDiffFileSummary[] {
78 const out: PrDiffFileSummary[] = [];
79 for (const line of (raw || "").trim().split("\n")) {
80 if (!line) continue;
81 const [add, del, ...pathParts] = line.split("\t");
82 if (add === undefined || del === undefined || pathParts.length === 0) continue;
83 const pathRaw = pathParts.join("\t");
84 const binary = add === "-" || del === "-";
85 const rename = splitRenamePath(pathRaw);
86 out.push({
87 path: rename ? rename[1] : pathRaw,
88 oldPath: rename ? rename[0] : null,
89 status: rename ? "renamed" : binary ? "binary" : "changed",
90 additions: binary ? 0 : parseInt(add, 10) || 0,
91 deletions: binary ? 0 : parseInt(del, 10) || 0,
92 binary,
93 });
94 }
95 return out;
96}
97
98/**
99 * Parse `git diff --name-status` output into path → status. Rename lines
100 * (`R100\told\tnew`) key by the NEW path.
101 */
102export function parseNameStatus(
103 raw: string
104): Map<string, PrDiffFileSummary["status"]> {
105 const map = new Map<string, PrDiffFileSummary["status"]>();
106 for (const line of (raw || "").trim().split("\n")) {
107 if (!line) continue;
108 const cols = line.split("\t");
109 const code = cols[0] || "";
110 if (code.startsWith("R") || code.startsWith("C")) {
111 const newPath = cols[2];
112 if (newPath) map.set(newPath, "renamed");
113 continue;
114 }
115 const p = cols[1];
116 if (!p) continue;
117 if (code.startsWith("A")) map.set(p, "added");
118 else if (code.startsWith("D")) map.set(p, "deleted");
119 else if (code.startsWith("M")) map.set(p, "modified");
120 }
121 return map;
122}
123
124/**
125 * The per-file list for big-PR mode: numstat for the counts, name-status
126 * for honest status labels. `nameStatusRaw` may be empty (merged-PR
127 * recovery ranges skip it) — statuses then degrade to numstat's best
128 * guess ("renamed"/"binary"/"changed") rather than inventing anything.
129 */
130export function buildPrDiffSummary(
131 numstatRaw: string,
132 nameStatusRaw: string
133): PrDiffFileSummary[] {
134 const files = parseNumstat(numstatRaw);
135 const statuses = parseNameStatus(nameStatusRaw);
136 for (const f of files) {
137 const s = statuses.get(f.path);
138 if (s && !(f.binary && s === "modified")) f.status = s;
139 else if (f.binary) f.status = f.status === "renamed" ? "renamed" : "binary";
140 }
141 return files;
142}
Modifiedsrc/routes/pulls.tsx+564−34View fileUnifiedSplit
1414 */
1515
1616import { Hono } from "hono";
17import { parseIdNumber } from "../lib/route-params";
17import { parseIdNumber, parseIdUuid } from "../lib/route-params";
1818import { eq, and, desc, asc, sql, inArray, ilike, ne, isNotNull } from "drizzle-orm";
1919import { db } from "../db";
2020import {
3333import { Layout } from "../views/layout";
3434import { RepoHeader, RepoNav, PageHeader, Button as GxButton, Badge as GxBadge, sharedComponentStyles } from "../views/components";
3535import { PendingCommentsBanner } from "../views/pending-comments-banner";
36import { DiffView, type InlineDiffComment } from "../views/diff-view";
36import {
37 DiffView,
38 DiffFileListView,
39 SingleFileDiffFragment,
40 type InlineDiffComment,
41} from "../views/diff-view";
42import {
43 isBigPrDiff,
44 buildPrDiffSummary,
45 type PrDiffFileSummary,
46} from "../lib/pr-diff-budget";
3747import { ReactionsBar } from "../views/reactions";
3848import { summariseReactions } from "../lib/reactions";
3949import { loadPrTemplate } from "../lib/templates";
5060import { ctrlEnterSubmitScript, codeBlockCopyScript } from "../lib/keyboard-ux";
5161import { softAuth, requireAuth } from "../middleware/auth";
5262import type { AuthEnv } from "../middleware/auth";
53import { requireRepoAccess } from "../middleware/repo-access";
63import {
64 requireRepoAccess,
65 satisfiesAccess,
66 type RepoAccessLevel,
67} from "../middleware/repo-access";
5468import {
5569 decideInitialStatus,
5670 notifyOwnerOfPendingComment,
15691583 .split-lines { display: inline-block; margin-left: 10px; font-size: 11.5px; color: var(--text-muted); background: var(--bg-tertiary); padding: 1px 7px; border-radius: 9999px; }
15701584 .split-order { font-size: 13px; color: var(--text-muted); margin: 14px 0 0; }
15711585 .split-order strong { color: var(--text); }
1586
1587 /* Move #6B — review-thread resolution progress pill (display only). */
1588 .prs-thread-progress { margin: 0 0 12px; }
15721589`;
15731590
15741591/* ──────────────────────────────────────────────────────────────────────
21452162 return { owner, repo };
21462163}
21472164
2165/* ──────────────────────────────────────────────────────────────────────
2166 * Move #6 — review ergonomics helpers.
2167 *
2168 * pr_comments is a FLAT table (no parent id): an inline review "thread"
2169 * is the group of approved comments sharing a (filePath, lineNumber)
2170 * anchor on one PR, ordered by createdAt; the earliest comment is the
2171 * thread ROOT and carries the resolution columns (migration 0126).
2172 * ────────────────────────────────────────────────────────────────────── */
2173
2174/**
2175 * Who may resolve/unresolve a review thread. Pure so tests can pin the
2176 * rule: the PR author, the thread root's author, or anyone with write
2177 * access. There is no pre-existing PR-comment edit/delete permission rule
2178 * in this file to mirror, so this is the documented rule set. Display-only
2179 * state — resolution never gates merging.
2180 */
2181export function canResolvePrThread(args: {
2182 viewerId: string | null;
2183 prAuthorId: string;
2184 threadAuthorId: string;
2185 /** RepoAccessLevel string as stashed by requireRepoAccess. */
2186 viewerAccess: string;
2187}): boolean {
2188 if (!args.viewerId) return false;
2189 if (args.viewerId === args.prAuthorId) return true;
2190 if (args.viewerId === args.threadAuthorId) return true;
2191 return satisfiesAccess(args.viewerAccess as RepoAccessLevel, "write");
2192}
2193
2194/**
2195 * Inline (file+line anchored) comments for the diff surfaces, with thread
2196 * resolve state attached to each thread root. Shared by the PR detail
2197 * "Files changed" tab and the per-file patch fragment endpoint so the two
2198 * renders cannot drift.
2199 */
2200async function loadDiffInlineComments(opts: {
2201 prId: string;
2202 prNumber: number;
2203 ownerName: string;
2204 repoName: string;
2205 prAuthorId: string;
2206 viewer: { id: string } | null;
2207 viewerAccess: string;
2208 /** When set, only this file's comments are loaded (fragment endpoint). */
2209 filePath?: string;
2210}): Promise<InlineDiffComment[]> {
2211 const conditions = [
2212 eq(prComments.pullRequestId, opts.prId),
2213 eq(prComments.moderationStatus, "approved"),
2214 ];
2215 if (opts.filePath) conditions.push(eq(prComments.filePath, opts.filePath));
2216
2217 const inlineRows = await db
2218 .select({
2219 id: prComments.id,
2220 filePath: prComments.filePath,
2221 lineNumber: prComments.lineNumber,
2222 body: prComments.body,
2223 isAiReview: prComments.isAiReview,
2224 createdAt: prComments.createdAt,
2225 authorId: prComments.authorId,
2226 resolvedAt: prComments.resolvedAt,
2227 resolvedBy: prComments.resolvedBy,
2228 authorUsername: users.username,
2229 })
2230 .from(prComments)
2231 .innerJoin(users, eq(prComments.authorId, users.id))
2232 .where(and(...conditions))
2233 .orderBy(asc(prComments.createdAt));
2234
2235 const anchored = inlineRows.filter(
2236 (r) => r.filePath != null && r.lineNumber != null
2237 );
2238
2239 // Resolver usernames — one batched lookup for the "Resolved by @user" bar.
2240 const resolverIds = [
2241 ...new Set(
2242 anchored.map((r) => r.resolvedBy).filter((v): v is string => !!v)
2243 ),
2244 ];
2245 const resolverNames = new Map<string, string>();
2246 if (resolverIds.length > 0) {
2247 const rows = await db
2248 .select({ id: users.id, username: users.username })
2249 .from(users)
2250 .where(inArray(users.id, resolverIds));
2251 for (const r of rows) resolverNames.set(r.id, r.username);
2252 }
2253
2254 const urlBase = `/${opts.ownerName}/${opts.repoName}/pulls/${opts.prNumber}`;
2255 const seenAnchors = new Set<string>();
2256 return anchored.map((r) => {
2257 const anchor = `${r.filePath}:${r.lineNumber}`;
2258 const isRoot = !seenAnchors.has(anchor);
2259 if (isRoot) seenAnchors.add(anchor);
2260 const out: InlineDiffComment = {
2261 id: r.id,
2262 filePath: r.filePath!,
2263 lineNumber: r.lineNumber!,
2264 authorUsername: r.authorUsername,
2265 body: renderMarkdown(
2266 r.isAiReview ? presentAiCommentBody(r.body) : r.body
2267 ),
2268 isAiReview: r.isAiReview,
2269 createdAt: r.createdAt.toISOString(),
2270 };
2271 if (isRoot) {
2272 out.thread = {
2273 resolved: r.resolvedAt != null,
2274 resolvedByUsername: r.resolvedBy
2275 ? (resolverNames.get(r.resolvedBy) ?? null)
2276 : null,
2277 canResolve: canResolvePrThread({
2278 viewerId: opts.viewer?.id ?? null,
2279 prAuthorId: opts.prAuthorId,
2280 threadAuthorId: r.authorId,
2281 viewerAccess: opts.viewerAccess,
2282 }),
2283 resolveUrl: `${urlBase}/comments/${r.id}/resolve`,
2284 unresolveUrl: `${urlBase}/comments/${r.id}/unresolve`,
2285 };
2286 }
2287 return out;
2288 });
2289}
2290
2291/**
2292 * Move #6A — cheap per-range summary pair: `git diff --numstat` +
2293 * `--name-status`. Neither generates a patch, so this stays fast on
2294 * monster PRs.
2295 */
2296async function prDiffSummaryRaw(
2297 repoDir: string,
2298 range: string
2299): Promise<{ numstat: string; nameStatus: string }> {
2300 const numProc = Bun.spawn(["git", "diff", "--numstat", range], {
2301 timeout: gitExecTimeoutMs(), killSignal: "SIGKILL", cwd: repoDir, stdout: "pipe", stderr: "pipe",
2302 });
2303 const nsProc = Bun.spawn(["git", "diff", "--name-status", range], {
2304 timeout: gitExecTimeoutMs(), killSignal: "SIGKILL", cwd: repoDir, stdout: "pipe", stderr: "pipe",
2305 });
2306 const [numstat, nameStatus] = await Promise.all([
2307 new Response(numProc.stdout).text(),
2308 new Response(nsProc.stdout).text(),
2309 ]);
2310 await Promise.all([numProc.exited, nsProc.exited]);
2311 return { numstat, nameStatus };
2312}
2313
2314/**
2315 * Move #6A — merged-PR range recovery for the fragment endpoint. Compact
2316 * version of the detail handler's "MERGED PRs" recovery (see the comment
2317 * there): after merge, `base...head` is empty, so find the merge commit by
2318 * the "pull request #N" message convention; for fast-forward merges fall
2319 * back to the head ref's final commit. Never throws.
2320 */
2321async function findMergedPrDiffRange(
2322 repoDir: string,
2323 prNumber: number,
2324 headBranch: string
2325): Promise<string | null> {
2326 try {
2327 const findProc = Bun.spawn(
2328 [
2329 "git", "log", "--all", "-n", "1",
2330 `--grep=pull request #${prNumber}\\b`,
2331 "--format=%H %P",
2332 ],
2333 { timeout: gitExecTimeoutMs(), killSignal: "SIGKILL", cwd: repoDir, stdout: "pipe", stderr: "pipe" }
2334 );
2335 const found = (await new Response(findProc.stdout).text()).trim();
2336 await findProc.exited;
2337 if (found) {
2338 const [sha, ...parents] = found.split(/\s+/);
2339 if (parents.length >= 2) return `${sha}^1..${sha}`;
2340 if (parents.length === 1) return `${sha}^..${sha}`;
2341 return null;
2342 }
2343 const tipProc = Bun.spawn(
2344 ["git", "rev-parse", "--verify", "--quiet", headBranch],
2345 { timeout: gitExecTimeoutMs(), killSignal: "SIGKILL", cwd: repoDir, stdout: "pipe", stderr: "pipe" }
2346 );
2347 const tip = (await new Response(tipProc.stdout).text()).trim();
2348 await tipProc.exited;
2349 return tip ? `${tip}~1..${tip}` : null;
2350 } catch {
2351 return null;
2352 }
2353}
2354
21482355// PR Nav helper
21492356// Thin wrapper over the shared RepoNav so the PR pages render the ONE
21502357// canonical repo nav instead of a stripped 4-tab bar. Owner-directed nav
40974304 ? await countPendingForRepo(resolved.repo.id)
40984305 : 0;
40994306
4307 // Move #6B — review-thread resolution progress. A thread is the group of
4308 // approved inline comments sharing a (filePath, lineNumber) anchor; the
4309 // earliest comment is the root and carries the resolve columns
4310 // (migration 0126). Computed from rows already fetched — no extra query.
4311 // Display-only: never consulted by any merge gate.
4312 const threadRoots = new Map<string, (typeof comments)[number]["comment"]>();
4313 for (const { comment } of comments) {
4314 if (comment.moderationStatus !== "approved") continue;
4315 if (!comment.filePath || comment.lineNumber == null) continue;
4316 const anchor = `${comment.filePath}:${comment.lineNumber}`;
4317 if (!threadRoots.has(anchor)) threadRoots.set(anchor, comment);
4318 }
4319 const threadTotal = threadRoots.size;
4320 const threadResolvedCount = [...threadRoots.values()].filter(
4321 (cm) => cm.resolvedAt != null
4322 ).length;
4323 const resolvedAnchors = new Set(
4324 [...threadRoots.entries()]
4325 .filter(([, cm]) => cm.resolvedAt != null)
4326 .map(([anchor]) => anchor)
4327 );
4328
41004329 // Reactions for the PR body + each comment, in parallel.
41014330 const [prReactions, ...prCommentReactions] = await Promise.all([
41024331 summariseReactions("pr", pr.id, user?.id),
43144543 let diffRaw = "";
43154544 let diffFiles: GitDiffFile[] = [];
43164545 let diffInlineComments: InlineDiffComment[] = [];
4546 // Move #6A — big-PR mode. When the diff blows the render budget the tab
4547 // switches to a per-file list (diffSummary) with lazy-loaded patches.
4548 let diffIsBig = false;
4549 let diffSummary: PrDiffFileSummary[] = [];
43174550 if (tab === "files" && prRefsSafe) {
43184551 const repoDir = getRepoPath(ownerName, repoName);
43194552 // Run the two git diffs in parallel — they're independent reads of
44194652 };
44204653 });
44214654
4422 // Fetch inline comments (file+line anchored) for the files tab
4423 const inlineRows = await db
4424 .select({
4425 id: prComments.id,
4426 filePath: prComments.filePath,
4427 lineNumber: prComments.lineNumber,
4428 body: prComments.body,
4429 isAiReview: prComments.isAiReview,
4430 createdAt: prComments.createdAt,
4431 authorUsername: users.username,
4432 })
4433 .from(prComments)
4434 .innerJoin(users, eq(prComments.authorId, users.id))
4435 .where(
4436 and(
4437 eq(prComments.pullRequestId, pr.id),
4438 eq(prComments.moderationStatus, "approved"),
4439 )
4440 )
4441 .orderBy(asc(prComments.createdAt));
4655 // Fetch inline comments (file+line anchored) for the files tab —
4656 // shared with the per-file patch fragment endpoint (move #6B), with
4657 // thread resolve state attached to each thread root.
4658 diffInlineComments = await loadDiffInlineComments({
4659 prId: pr.id,
4660 prNumber: pr.number,
4661 ownerName,
4662 repoName,
4663 prAuthorId: pr.authorId,
4664 viewer: user,
4665 viewerAccess: (c.get("repoAccess") as string | undefined) ?? "read",
4666 });
44424667
4443 diffInlineComments = inlineRows
4444 .filter(r => r.filePath != null && r.lineNumber != null)
4445 .map(r => ({
4446 id: r.id,
4447 filePath: r.filePath!,
4448 lineNumber: r.lineNumber!,
4449 authorUsername: r.authorUsername,
4450 body: renderMarkdown(r.isAiReview ? presentAiCommentBody(r.body) : r.body),
4451 isAiReview: r.isAiReview,
4452 createdAt: r.createdAt.toISOString(),
4453 }));
4668 // Move #6A — render-budget decision. Below the budget nothing changes:
4669 // the exact DiffView render ships as before. Above it, the monolithic
4670 // patch is dropped and the tab renders the per-file summary list whose
4671 // patches lazy-load from /files/:index/patch. The full diff has already
4672 // been generated at this point (it doubles as the byte measurement and
4673 // the merged-PR recovery input); the render cost — parsing plus
4674 // per-line highlighting into tens of MB of HTML — is what the budget
4675 // actually avoids.
4676 diffIsBig = isBigPrDiff({
4677 fileCount: diffFiles.length,
4678 patchBytes: Buffer.byteLength(diffRaw, "utf8"),
4679 });
4680 if (diffIsBig) {
4681 // name-status pass for honest per-file status labels — numstat alone
4682 // cannot tell added from deleted. Cheap (no patch generation). For
4683 // merged-recovery ranges this range is empty and statuses degrade to
4684 // numstat's best guess rather than inventing anything.
4685 let nameStatusRaw = "";
4686 try {
4687 const nsProc = Bun.spawn(
4688 ["git", "diff", "--name-status", `${pr.baseBranch}...${pr.headBranch}`],
4689 { timeout: gitExecTimeoutMs(), killSignal: "SIGKILL", cwd: repoDir, stdout: "pipe", stderr: "pipe" }
4690 );
4691 nameStatusRaw = await new Response(nsProc.stdout).text();
4692 await nsProc.exited;
4693 } catch { /* status pills degrade to "changed" */ }
4694 diffSummary = buildPrDiffSummary(stat, nameStatusRaw);
4695 diffRaw = ""; // never ship the monolithic patch to the renderer
4696 }
44544697 }
44554698
44564699 // Proactive pattern warning — get changed file paths and check for recurring
49845227 );
49855228 })()}
49865229
5230 {diffIsBig ? (
5231 /* Move #6A — above the render budget: per-file list, patches
5232 lazy-load from /files/:index/patch. Below the budget the
5233 DiffView branch is byte-for-byte what it always was. */
5234 <DiffFileListView
5235 files={diffSummary}
5236 patchUrlBase={`/${ownerName}/${repoName}/pulls/${pr.number}/files`}
5237 viewFileBase={`/${ownerName}/${repoName}/blob/${pr.headBranch}`}
5238 commentActionUrl={user ? `/${ownerName}/${repoName}/pulls/${pr.number}/comment` : undefined}
5239 applySuggestionUrl={user ? `/${ownerName}/${repoName}/pulls/${pr.number}/apply-suggestion` : undefined}
5240 pendingReviewUrl={user ? `/${ownerName}/${repoName}/pulls/${pr.number}/review/pending/add` : undefined}
5241 />
5242 ) : (
49875243 <DiffView
49885244 raw={diffRaw}
49895245 files={diffFiles}
49995255 repo={repoName}
50005256 prNumber={pr.number}
50015257 />
5258 )}
50025259 </>
50035260 ) : (
50045261 <>
50105267 />
50115268 )}
50125269
5270 {/* Move #6B — review-thread resolution progress. Informational
5271 only; merging never waits on it. */}
5272 {threadTotal > 0 && (
5273 <div class="prs-thread-progress">
5274 <span
5275 class={`prs-tag${threadResolvedCount === threadTotal ? " is-approved" : ""}`}
5276 title="Review-thread resolution is informational — it never blocks merging"
5277 >
5278 {"✓ "}
5279 {threadResolvedCount} of {threadTotal} review thread
5280 {threadTotal === 1 ? "" : "s"} resolved
5281 </span>
5282 </div>
5283 )}
5284
50135285 {/* Block H — AI trio review (security/correctness/style). When
50145286 `AI_TRIO_REVIEW_ENABLED=1` the three persona comments are
50155287 hoisted into a 3-column card grid above the normal comment
50725344 {comment.lineNumber ? `:${comment.lineNumber}` : ""}
50735345 </span>
50745346 )}
5347 {comment.filePath &&
5348 comment.lineNumber != null &&
5349 resolvedAnchors.has(
5350 `${comment.filePath}:${comment.lineNumber}`
5351 ) && (
5352 <span
5353 class="prs-tag is-approved"
5354 title="This review thread is marked resolved (display only)"
5355 >
5356 {"✓ resolved"}
5357 </span>
5358 )}
50755359 </div>
50765360 <div class="prs-comment-body">
50775361 <MarkdownContent
59746258 }
59756259);
59766260
6261// ─── Move #6A — per-file patch fragment (big-PR lazy diff) ─────────────────
6262// Returns the rendered HTML fragment for ONE file's diff, fetched by the
6263// DiffFileListView cards on expand. Access gating matches the PR detail
6264// page exactly: softAuth + read-level repo access, same resolveRepo path.
6265pulls.get(
6266 "/:owner/:repo/pulls/:number/files/:index/patch",
6267 softAuth,
6268 requireRepoAccess("read"),
6269 async (c) => {
6270 const { owner: ownerName, repo: repoName } = c.req.param();
6271 const prNum = (parseIdNumber(c.req.param("number")) ?? -1);
6272 // 0-based index — parseIdNumber rejects 0, so parse by hand.
6273 const indexRaw = c.req.param("index");
6274 const fileIndex = /^\d+$/.test(indexRaw ?? "") ? Number(indexRaw) : -1;
6275 const user = c.get("user");
6276
6277 const resolved = await resolveRepo(ownerName, repoName);
6278 if (!resolved) return c.notFound();
6279
6280 const [pr] = await db
6281 .select()
6282 .from(pullRequests)
6283 .where(
6284 and(
6285 eq(pullRequests.repositoryId, resolved.repo.id),
6286 eq(pullRequests.number, prNum)
6287 )
6288 )
6289 .limit(1);
6290 if (!pr || fileIndex < 0) return c.notFound();
6291 // Same legacy-row guard as the detail handler — a branch name starting
6292 // with a dash would read as a git option below.
6293 if (!(isSafeRef(pr.baseBranch) && isSafeRef(pr.headBranch))) {
6294 return c.notFound();
6295 }
6296
6297 const repoDir = getRepoPath(ownerName, repoName);
6298 let range = `${pr.baseBranch}...${pr.headBranch}`;
6299 let summaryRaw = await prDiffSummaryRaw(repoDir, range).catch(() => ({
6300 numstat: "",
6301 nameStatus: "",
6302 }));
6303 if (!summaryRaw.numstat.trim() && pr.state === "merged") {
6304 const recovered = await findMergedPrDiffRange(
6305 repoDir,
6306 pr.number,
6307 pr.headBranch
6308 );
6309 if (recovered) {
6310 range = recovered;
6311 summaryRaw = await prDiffSummaryRaw(repoDir, range).catch(() => ({
6312 numstat: "",
6313 nameStatus: "",
6314 }));
6315 }
6316 }
6317 const summary = buildPrDiffSummary(summaryRaw.numstat, summaryRaw.nameStatus);
6318
6319 // The list cards encode both index and path; prefer the index but fall
6320 // back to the path when the branch moved between page render and click.
6321 const wantPath = c.req.query("path") || null;
6322 let entry = fileIndex < summary.length ? summary[fileIndex] : undefined;
6323 if (wantPath && (!entry || entry.path !== wantPath)) {
6324 entry = summary.find((f) => f.path === wantPath) ?? entry;
6325 }
6326 if (!entry) return c.notFound();
6327
6328 const diffArgs = [
6329 "git", "diff", range, "--",
6330 ...(entry.oldPath ? [entry.oldPath] : []),
6331 entry.path,
6332 ];
6333 const proc = Bun.spawn(diffArgs, {
6334 timeout: gitExecTimeoutMs(), killSignal: "SIGKILL", cwd: repoDir, stdout: "pipe", stderr: "pipe",
6335 });
6336 // Same 30s ceiling as the detail handler's diff spawns.
6337 const killer = setTimeout(() => proc.kill(), 30_000);
6338 let raw = "";
6339 try {
6340 raw = await new Response(proc.stdout).text();
6341 await proc.exited;
6342 } finally {
6343 clearTimeout(killer);
6344 }
6345
6346 const inlineComments = await loadDiffInlineComments({
6347 prId: pr.id,
6348 prNumber: pr.number,
6349 ownerName,
6350 repoName,
6351 prAuthorId: pr.authorId,
6352 viewer: user,
6353 viewerAccess: (c.get("repoAccess") as string | undefined) ?? "read",
6354 filePath: entry.path,
6355 });
6356
6357 return c.html(
6358 <SingleFileDiffFragment
6359 raw={raw}
6360 blobHref={`/${ownerName}/${repoName}/blob/${pr.headBranch}/${entry.path}`}
6361 inlineComments={inlineComments}
6362 commentActionUrl={user ? `/${ownerName}/${repoName}/pulls/${pr.number}/comment` : undefined}
6363 applySuggestionUrl={user ? `/${ownerName}/${repoName}/pulls/${pr.number}/apply-suggestion` : undefined}
6364 pendingReviewUrl={user ? `/${ownerName}/${repoName}/pulls/${pr.number}/review/pending/add` : undefined}
6365 />
6366 );
6367 }
6368);
6369
6370// ─── Move #6B — review-thread resolve / unresolve ──────────────────────────
6371// Thread-root scoped: resolving any comment of a (filePath, lineNumber)
6372// anchor group canonicalises to the group's earliest comment and stamps the
6373// resolution there; replies inherit by grouping. Permission rule is
6374// canResolvePrThread (PR author / thread author / write access). The
6375// middleware chain matches the other comment actions in this file.
6376// Display-only — merge gating never reads these columns.
6377
6378/** Shared worker for the two routes below. Returns an error string, or
6379 * null on success. */
6380async function setPrThreadResolution(args: {
6381 ownerName: string;
6382 repoName: string;
6383 prNum: number;
6384 commentId: string;
6385 userId: string;
6386 viewerAccess: string;
6387 markResolved: boolean;
6388}): Promise<string | null> {
6389 const repoCtx = await resolveRepo(args.ownerName, args.repoName);
6390 if (!repoCtx) return "Repository not found";
6391
6392 const [pr] = await db
6393 .select()
6394 .from(pullRequests)
6395 .where(
6396 and(
6397 eq(pullRequests.repositoryId, repoCtx.repo.id),
6398 eq(pullRequests.number, args.prNum)
6399 )
6400 )
6401 .limit(1);
6402 if (!pr) return "Pull request not found";
6403
6404 const [comment] = await db
6405 .select()
6406 .from(prComments)
6407 .where(
6408 and(
6409 eq(prComments.id, args.commentId),
6410 eq(prComments.pullRequestId, pr.id)
6411 )
6412 )
6413 .limit(1);
6414 if (!comment) return "Comment not found";
6415 if (!comment.filePath || comment.lineNumber == null) {
6416 return "Only inline review threads can be resolved";
6417 }
6418
6419 // Canonicalise to the thread root — the earliest approved comment on the
6420 // same anchor. Resolving a reply resolves the thread it belongs to.
6421 const [root] = await db
6422 .select()
6423 .from(prComments)
6424 .where(
6425 and(
6426 eq(prComments.pullRequestId, pr.id),
6427 eq(prComments.filePath, comment.filePath),
6428 eq(prComments.lineNumber, comment.lineNumber),
6429 eq(prComments.moderationStatus, "approved")
6430 )
6431 )
6432 .orderBy(asc(prComments.createdAt))
6433 .limit(1);
6434 const target = root ?? comment;
6435
6436 const allowed = canResolvePrThread({
6437 viewerId: args.userId,
6438 prAuthorId: pr.authorId,
6439 threadAuthorId: target.authorId,
6440 viewerAccess: args.viewerAccess,
6441 });
6442 if (!allowed) return "You don't have permission to resolve this thread";
6443
6444 await db
6445 .update(prComments)
6446 .set(
6447 args.markResolved
6448 ? { resolvedAt: new Date(), resolvedBy: args.userId }
6449 : { resolvedAt: null, resolvedBy: null }
6450 )
6451 .where(eq(prComments.id, target.id));
6452 return null;
6453}
6454
6455pulls.post(
6456 "/:owner/:repo/pulls/:number/comments/:commentId/resolve",
6457 softAuth,
6458 requireAuth,
6459 requireRepoAccess("read"),
6460 async (c) => {
6461 const { owner: ownerName, repo: repoName } = c.req.param();
6462 const prNum = (parseIdNumber(c.req.param("number")) ?? -1);
6463 const commentId = parseIdUuid(c.req.param("commentId"));
6464 const user = c.get("user")!;
6465 const backUrl = `/${ownerName}/${repoName}/pulls/${prNum}?tab=files`;
6466 if (!commentId) return c.redirect(backUrl);
6467 const err = await setPrThreadResolution({
6468 ownerName,
6469 repoName,
6470 prNum,
6471 commentId,
6472 userId: user.id,
6473 viewerAccess: (c.get("repoAccess") as string | undefined) ?? "read",
6474 markResolved: true,
6475 });
6476 if (err) return c.redirect(`${backUrl}&error=${encodeURIComponent(err)}`);
6477 return c.redirect(backUrl);
6478 }
6479);
6480
6481pulls.post(
6482 "/:owner/:repo/pulls/:number/comments/:commentId/unresolve",
6483 softAuth,
6484 requireAuth,
6485 requireRepoAccess("read"),
6486 async (c) => {
6487 const { owner: ownerName, repo: repoName } = c.req.param();
6488 const prNum = (parseIdNumber(c.req.param("number")) ?? -1);
6489 const commentId = parseIdUuid(c.req.param("commentId"));
6490 const user = c.get("user")!;
6491 const backUrl = `/${ownerName}/${repoName}/pulls/${prNum}?tab=files`;
6492 if (!commentId) return c.redirect(backUrl);
6493 const err = await setPrThreadResolution({
6494 ownerName,
6495 repoName,
6496 prNum,
6497 commentId,
6498 userId: user.id,
6499 viewerAccess: (c.get("repoAccess") as string | undefined) ?? "read",
6500 markResolved: false,
6501 });
6502 if (err) return c.redirect(`${backUrl}&error=${encodeURIComponent(err)}`);
6503 return c.redirect(backUrl);
6504 }
6505);
6506
59776507// ─── Batched PR review workflow ─────────────────────────────────────────────
59786508// Reviewers can stage multiple inline comments in a pending_reviews session,
59796509// then submit them all at once as an Approve / Request Changes / Comment review.
Modifiedsrc/views/diff-view.tsx+570−186View fileUnifiedSplit
350350 body: string;
351351 createdAt: string;
352352 isAiReview?: boolean;
353 /**
354 * Review-thread resolve state (move #6B) — set on the thread ROOT only
355 * (the earliest comment of a file+line anchor group). Replies inherit
356 * the state by grouping. Display-only; never gates merging.
357 */
358 thread?: InlineThreadState;
359}
360
361export interface InlineThreadState {
362 resolved: boolean;
363 resolvedByUsername: string | null;
364 /** Viewer may resolve/unresolve (PR author, thread author, or write access). */
365 canResolve: boolean;
366 /** POST targets for the resolve/unresolve forms. */
367 resolveUrl: string;
368 unresolveUrl: string;
353369}
354370
355371export interface DiffViewProps {
379395 prNumber?: number;
380396}
381397
398/**
399 * One inline comment card, anchored under its diff line. Extracted verbatim
400 * from the DiffView per-line map (move #6) so the thread wrapper and the
401 * single-file fragment can reuse it — rendering is unchanged.
402 */
403const InlineCommentCard: FC<{ c: InlineDiffComment; applySuggestionUrl?: string }> = ({ c, applySuggestionUrl }) => {
404 // Detect suggestion block: ```suggestion\n...\n```
405 const suggMatch = c.body.match(/^```suggestion\n([\s\S]*?)\n```/);
406 if (suggMatch) {
407 const suggCode = suggMatch[1];
408 // Any text after the closing ``` fence is treated as the comment prose
409 const afterFence = c.body.slice(suggMatch[0].length).trim();
410 return (
411 <div class={`diff-inline-comment${c.isAiReview ? " diff-inline-comment-ai" : ""}`} data-comment-id={c.id}>
412 <div class="diff-inline-comment-head">
413 <strong>{c.authorUsername}</strong>
414 <span class="diff-inline-comment-meta">
415 {c.isAiReview && <span class="diff-inline-ai-badge">AI</span>}
416 {new Date(c.createdAt).toLocaleDateString()}
417 </span>
418 </div>
419 <div class="diff-suggestion-block">
420 <div class="diff-suggestion-header">
421 <span>Suggested change</span>
422 {applySuggestionUrl && (
423 <form method="post" action={`${applySuggestionUrl}/${c.id}`} style="margin:0;display:inline;">
424 <button type="submit" class="diff-apply-btn">Apply suggestion</button>
425 </form>
426 )}
427 </div>
428 <pre class="diff-suggestion-code">{suggCode}</pre>
429 </div>
430 {afterFence && (
431 <div class="diff-inline-comment-body" style="margin-top:6px;" dangerouslySetInnerHTML={{ __html: afterFence }} />
432 )}
433 </div>
434 );
435 }
436 return (
437 <div class={`diff-inline-comment${c.isAiReview ? " diff-inline-comment-ai" : ""}`} data-comment-id={c.id}>
438 <div class="diff-inline-comment-head">
439 <strong>{c.authorUsername}</strong>
440 <span class="diff-inline-comment-meta">
441 {c.isAiReview && <span class="diff-inline-ai-badge">AI</span>}
442 {new Date(c.createdAt).toLocaleDateString()}
443 </span>
444 </div>
445 <div class="diff-inline-comment-body" dangerouslySetInnerHTML={{ __html: c.body }} />
446 </div>
447 );
448};
449
450/**
451 * A file+line anchored comment thread (move #6B). The thread root's
452 * `thread` state decides the chrome: resolved threads collapse behind a
453 * quiet "Resolved by @user · show" bar; unresolved threads render exactly
454 * as before, plus a quiet "Resolve thread" button when the viewer may
455 * resolve. Display-only — resolution never gates merging.
456 */
457const InlineCommentThread: FC<{
458 comments: InlineDiffComment[];
459 applySuggestionUrl?: string;
460}> = ({ comments, applySuggestionUrl }) => {
461 const thread = comments[0]?.thread;
462 const rendered = comments.map((c) => (
463 <InlineCommentCard c={c} applySuggestionUrl={applySuggestionUrl} />
464 ));
465 if (thread?.resolved) {
466 return (
467 <div class="diff-thread diff-thread-resolved">
468 <div class="diff-thread-bar">
469 <span class="diff-thread-badge">
470 {"✓ Resolved"}
471 {thread.resolvedByUsername ? ` by @${thread.resolvedByUsername}` : ""}
472 </span>
473 <button
474 type="button"
475 class="diff-thread-toggle"
476 onclick="var w=this.closest('.diff-thread');var b=w.querySelector('.diff-thread-body');var h=b.hasAttribute('hidden');b.toggleAttribute('hidden');this.textContent=h?'hide':'show';"
477 >
478 show
479 </button>
480 <span class="diff-thread-spacer" />
481 {thread.canResolve && (
482 <form method="post" action={thread.unresolveUrl} class="diff-thread-form">
483 <button type="submit" class="diff-thread-quiet-btn">Unresolve</button>
484 </form>
485 )}
486 </div>
487 <div class="diff-thread-body" hidden>
488 {rendered}
489 </div>
490 </div>
491 );
492 }
493 return (
494 <>
495 {rendered}
496 {thread && thread.canResolve && (
497 <form method="post" action={thread.resolveUrl} class="diff-thread-form diff-thread-resolve-row">
498 <button
499 type="submit"
500 class="diff-thread-quiet-btn"
501 title="Mark this review thread as resolved (display only — never blocks merging)"
502 >
503 Resolve thread
504 </button>
505 </form>
506 )}
507 </>
508 );
509};
510
511/**
512 * One file's diff body — the binary/too-big/hunks render, extracted
513 * verbatim from DiffView's per-file map (move #6) so the per-file patch
514 * fragment endpoint can reuse it. Rendering is unchanged for DiffView.
515 */
516const DiffFileBody: FC<{
517 file: ParsedFile;
518 isSplit?: boolean;
519 blobHref: string | null;
520 commentsByLine: Map<string, InlineDiffComment[]>;
521 commentActionUrl?: string;
522 applySuggestionUrl?: string;
523 pendingReviewUrl?: string;
524}> = ({ file, isSplit, blobHref, commentsByLine, commentActionUrl, applySuggestionUrl, pendingReviewUrl }) => {
525 const tooBig = file.lineCount > BIG_FILE_LINES;
526 const showDeletedOnly = file.status === "deleted";
527
528 // Highlight the post-change view + (separately) the pre-change view
529 // for deletions, so syntax colors survive both sides of a diff.
530 const { perLine: addedHighlights, language } = file.binary || tooBig
531 ? { perLine: new Map<string, string>(), language: null }
532 : highlightFile(file.path, file.hunks);
533 const deletedHighlights = file.binary || tooBig
534 ? new Map<string, string>()
535 : highlightDeletedLines(file.path, file.hunks);
536
537 return file.binary ? (
538 <div class="diff-empty">Binary file not shown.</div>
539 ) : tooBig ? (
540 <div class="diff-empty diff-empty-big">
541 Large file ({file.lineCount.toLocaleString()} lines).{" "}
542 {blobHref ? (
543 <a href={blobHref}>Load full file</a>
544 ) : (
545 "Skipped inline render."
546 )}
547 </div>
548 ) : file.hunks.length === 0 ? (
549 <div class="diff-empty">No textual changes.</div>
550 ) : isSplit ? (
551 <div class={`diff-body diff-body-split${language ? " has-hljs" : ""}`}>
552 <table class="diff-table-split" style="width:100%;border-collapse:collapse;">
553 <colgroup>
554 <col style="width:40px;" /><col style="width:50%;" />
555 <col style="width:40px;" /><col style="width:50%;" />
556 </colgroup>
557 <tbody>
558 {file.hunks.map((hunk) => {
559 const splitRows = toSplitRows(hunk.lines);
560 return (
561 <>
562 <tr class="diff-split-hunk-row">
563 <td colspan={4} class="diff-hunk-header" style="padding:4px 8px;font-size:11px;font-family:var(--font-mono);color:var(--text-muted);background:color-mix(in srgb, var(--accent) 6%, transparent);">
564 <span class="diff-hunk-header-text">{hunk.header}</span>
565 </td>
566 </tr>
567 {splitRows.map((row) => {
568 const leftBg = row.left?.kind === "del" ? "color-mix(in srgb, var(--red) 12%, var(--bg-elevated))" : "transparent";
569 const rightBg = row.right?.kind === "add" ? "color-mix(in srgb, var(--green) 14%, var(--bg-elevated))" : "transparent";
570 return (
571 <tr class="diff-split-row">
572 <td class="diff-ln" style={`background:${leftBg};color:var(--text-muted);padding:0 6px;font-size:11px;font-family:var(--font-mono);text-align:right;user-select:none;border-right:1px solid var(--border);`}>
573 {row.left?.oldNum ?? ""}
574 </td>
575 <td class="diff-split-cell" style={`background:${leftBg};padding:0 var(--space-2,8px);font-family:var(--font-mono);font-size:12.5px;white-space:pre;overflow:hidden;border-right:1px solid var(--border);`}>
576 {row.left ? (
577 <span>{(row.left.kind === "del" ? "- " : " ") + row.left.text}</span>
578 ) : null}
579 </td>
580 <td class="diff-ln" style={`background:${rightBg};color:var(--text-muted);padding:0 6px;font-size:11px;font-family:var(--font-mono);text-align:right;user-select:none;border-right:1px solid var(--border);`}>
581 {row.right?.newNum ?? ""}
582 </td>
583 <td class="diff-split-cell" style={`background:${rightBg};padding:0 var(--space-2,8px);font-family:var(--font-mono);font-size:12.5px;white-space:pre;overflow:hidden;`}>
584 {row.right ? (
585 <span>{(row.right.kind === "add" ? "+ " : " ") + row.right.text}</span>
586 ) : null}
587 </td>
588 </tr>
589 );
590 })}
591 </>
592 );
593 })}
594 </tbody>
595 </table>
596 </div>
597 ) : (
598 <div class={`diff-body${language ? " has-hljs" : ""}`}>
599 {file.hunks.map((hunk, hIdx) => (
600 <>
601 {hIdx > 0 && <div class="diff-hunk-gap" aria-hidden="true" />}
602 <div class="diff-hunk-header" role="separator">
603 <span class="diff-hunk-header-text">{hunk.header}</span>
604 </div>
605 {hunk.lines.map((ln, lIdx) => {
606 const key =
607 ln.kind === "del"
608 ? `${hunk.oldStart}:${ln.oldNum ?? "x"}`
609 : `${hunk.newStart}:${ln.newNum ?? "x"}`;
610 // Lookup the highlight match. Our maps key with an
611 // index suffix, so iterate to find it (cheap — small).
612 let highlighted: string | null = null;
613 if (showDeletedOnly || ln.kind === "del") {
614 for (const [k, v] of deletedHighlights) {
615 if (k.startsWith(`${hunk.oldStart}:${ln.oldNum ?? "x"}:`)) {
616 highlighted = v;
617 deletedHighlights.delete(k);
618 break;
619 }
620 }
621 } else {
622 for (const [k, v] of addedHighlights) {
623 if (k.startsWith(`${hunk.newStart}:${ln.newNum ?? "x"}:`)) {
624 highlighted = v;
625 addedHighlights.delete(k);
626 break;
627 }
628 }
629 }
630 const marker =
631 ln.kind === "add" ? "+" : ln.kind === "del" ? "−" : " ";
632 // Inline comments anchor to the new-file line number
633 const commentKey = ln.newNum != null ? `${file.path}:${ln.newNum}` : null;
634 const lineComments = commentKey ? (commentsByLine.get(commentKey) ?? []) : [];
635 const canComment = (commentActionUrl || pendingReviewUrl) && ln.kind !== "del" && ln.newNum != null;
636 return (
637 <>
638 <div
639 class={`diff-row diff-row-${ln.kind}`}
640 data-line={key}
641 data-file={canComment ? file.path : undefined}
642 data-newline={canComment ? ln.newNum : undefined}
643 data-linetext={canComment ? ln.text : undefined}
644 >
645 <span class="diff-gutter diff-gutter-old">
646 {ln.oldNum ?? ""}
647 </span>
648 <span class="diff-gutter diff-gutter-new">
649 {ln.newNum ?? ""}
650 {canComment && (
651 <button class="diff-comment-btn" title="Add comment" aria-label="Add inline comment">+</button>
652 )}
653 </span>
654 <span class="diff-marker" aria-hidden="true">
655 {marker}
656 </span>
657 <CodeSpan html={highlighted} text={ln.text} />
658 </div>
659 {lineComments.length > 0 && (
660 <InlineCommentThread
661 comments={lineComments}
662 applySuggestionUrl={applySuggestionUrl}
663 />
664 )}
665 </>
666 );
667 })}
668 </>
669 ))}
670 </div>
671 );
672};
673
382674export const DiffView: FC<DiffViewProps> = ({ raw, files, viewFileBase, inlineComments, commentActionUrl, applySuggestionUrl, pendingReviewUrl, submitReviewUrl, isSplit, pendingCount, owner, repo, prNumber }) => {
383675 const parsed = parseUnifiedDiff(raw);
384676
448740 statByPath.get(file.oldPath ?? "") ??
449741 countAddsDels(file);
450742 const id = `diff-file-${fIdx}`;
451 const tooBig = file.lineCount > BIG_FILE_LINES;
452 const showDeletedOnly = file.status === "deleted";
453743 const blobHref = viewFileBase
454744 ? `${viewFileBase}/${file.path}`
455745 : null;
456746
457 // Highlight the post-change view + (separately) the pre-change view
458 // for deletions, so syntax colors survive both sides of a diff.
459 const { perLine: addedHighlights, language } = file.binary || tooBig
460 ? { perLine: new Map<string, string>(), language: null }
461 : highlightFile(file.path, file.hunks);
462 const deletedHighlights = file.binary || tooBig
463 ? new Map<string, string>()
464 : highlightDeletedLines(file.path, file.hunks);
465
466747 return (
467748 <details class="diff-file" id={id} open>
468749 <summary class="diff-file-summary">
514795 </span>
515796 </summary>
516797
517 {file.binary ? (
518 <div class="diff-empty">Binary file not shown.</div>
519 ) : tooBig ? (
520 <div class="diff-empty diff-empty-big">
521 Large file ({file.lineCount.toLocaleString()} lines).{" "}
522 {blobHref ? (
523 <a href={blobHref}>Load full file</a>
524 ) : (
525 "Skipped inline render."
526 )}
527 </div>
528 ) : file.hunks.length === 0 ? (
529 <div class="diff-empty">No textual changes.</div>
530 ) : isSplit ? (
531 <div class={`diff-body diff-body-split${language ? " has-hljs" : ""}`}>
532 <table class="diff-table-split" style="width:100%;border-collapse:collapse;">
533 <colgroup>
534 <col style="width:40px;" /><col style="width:50%;" />
535 <col style="width:40px;" /><col style="width:50%;" />
536 </colgroup>
537 <tbody>
538 {file.hunks.map((hunk) => {
539 const splitRows = toSplitRows(hunk.lines);
540 return (
541 <>
542 <tr class="diff-split-hunk-row">
543 <td colspan={4} class="diff-hunk-header" style="padding:4px 8px;font-size:11px;font-family:var(--font-mono);color:var(--text-muted);background:color-mix(in srgb, var(--accent) 6%, transparent);">
544 <span class="diff-hunk-header-text">{hunk.header}</span>
545 </td>
546 </tr>
547 {splitRows.map((row) => {
548 const leftBg = row.left?.kind === "del" ? "color-mix(in srgb, var(--red) 12%, var(--bg-elevated))" : "transparent";
549 const rightBg = row.right?.kind === "add" ? "color-mix(in srgb, var(--green) 14%, var(--bg-elevated))" : "transparent";
550 return (
551 <tr class="diff-split-row">
552 <td class="diff-ln" style={`background:${leftBg};color:var(--text-muted);padding:0 6px;font-size:11px;font-family:var(--font-mono);text-align:right;user-select:none;border-right:1px solid var(--border);`}>
553 {row.left?.oldNum ?? ""}
554 </td>
555 <td class="diff-split-cell" style={`background:${leftBg};padding:0 var(--space-2,8px);font-family:var(--font-mono);font-size:12.5px;white-space:pre;overflow:hidden;border-right:1px solid var(--border);`}>
556 {row.left ? (
557 <span>{(row.left.kind === "del" ? "- " : " ") + row.left.text}</span>
558 ) : null}
559 </td>
560 <td class="diff-ln" style={`background:${rightBg};color:var(--text-muted);padding:0 6px;font-size:11px;font-family:var(--font-mono);text-align:right;user-select:none;border-right:1px solid var(--border);`}>
561 {row.right?.newNum ?? ""}
562 </td>
563 <td class="diff-split-cell" style={`background:${rightBg};padding:0 var(--space-2,8px);font-family:var(--font-mono);font-size:12.5px;white-space:pre;overflow:hidden;`}>
564 {row.right ? (
565 <span>{(row.right.kind === "add" ? "+ " : " ") + row.right.text}</span>
566 ) : null}
567 </td>
568 </tr>
569 );
570 })}
571 </>
572 );
573 })}
574 </tbody>
575 </table>
576 </div>
577 ) : (
578 <div class={`diff-body${language ? " has-hljs" : ""}`}>
579 {file.hunks.map((hunk, hIdx) => (
580 <>
581 {hIdx > 0 && <div class="diff-hunk-gap" aria-hidden="true" />}
582 <div class="diff-hunk-header" role="separator">
583 <span class="diff-hunk-header-text">{hunk.header}</span>
584 </div>
585 {hunk.lines.map((ln, lIdx) => {
586 const key =
587 ln.kind === "del"
588 ? `${hunk.oldStart}:${ln.oldNum ?? "x"}`
589 : `${hunk.newStart}:${ln.newNum ?? "x"}`;
590 // Lookup the highlight match. Our maps key with an
591 // index suffix, so iterate to find it (cheap — small).
592 let highlighted: string | null = null;
593 if (showDeletedOnly || ln.kind === "del") {
594 for (const [k, v] of deletedHighlights) {
595 if (k.startsWith(`${hunk.oldStart}:${ln.oldNum ?? "x"}:`)) {
596 highlighted = v;
597 deletedHighlights.delete(k);
598 break;
599 }
600 }
601 } else {
602 for (const [k, v] of addedHighlights) {
603 if (k.startsWith(`${hunk.newStart}:${ln.newNum ?? "x"}:`)) {
604 highlighted = v;
605 addedHighlights.delete(k);
606 break;
607 }
608 }
609 }
610 const marker =
611 ln.kind === "add" ? "+" : ln.kind === "del" ? "−" : " ";
612 // Inline comments anchor to the new-file line number
613 const commentKey = ln.newNum != null ? `${file.path}:${ln.newNum}` : null;
614 const lineComments = commentKey ? (commentsByLine.get(commentKey) ?? []) : [];
615 const canComment = (commentActionUrl || pendingReviewUrl) && ln.kind !== "del" && ln.newNum != null;
616 return (
617 <>
618 <div
619 class={`diff-row diff-row-${ln.kind}`}
620 data-line={key}
621 data-file={canComment ? file.path : undefined}
622 data-newline={canComment ? ln.newNum : undefined}
623 data-linetext={canComment ? ln.text : undefined}
624 >
625 <span class="diff-gutter diff-gutter-old">
626 {ln.oldNum ?? ""}
627 </span>
628 <span class="diff-gutter diff-gutter-new">
629 {ln.newNum ?? ""}
630 {canComment && (
631 <button class="diff-comment-btn" title="Add comment" aria-label="Add inline comment">+</button>
632 )}
633 </span>
634 <span class="diff-marker" aria-hidden="true">
635 {marker}
636 </span>
637 <CodeSpan html={highlighted} text={ln.text} />
638 </div>
639 {lineComments.map(c => {
640 // Detect suggestion block: ```suggestion\n...\n```
641 const suggMatch = c.body.match(/^```suggestion\n([\s\S]*?)\n```/);
642 if (suggMatch) {
643 const suggCode = suggMatch[1];
644 // Any text after the closing ``` fence is treated as the comment prose
645 const afterFence = c.body.slice(suggMatch[0].length).trim();
646 return (
647 <div class={`diff-inline-comment${c.isAiReview ? " diff-inline-comment-ai" : ""}`} data-comment-id={c.id}>
648 <div class="diff-inline-comment-head">
649 <strong>{c.authorUsername}</strong>
650 <span class="diff-inline-comment-meta">
651 {c.isAiReview && <span class="diff-inline-ai-badge">AI</span>}
652 {new Date(c.createdAt).toLocaleDateString()}
653 </span>
654 </div>
655 <div class="diff-suggestion-block">
656 <div class="diff-suggestion-header">
657 <span>Suggested change</span>
658 {applySuggestionUrl && (
659 <form method="post" action={`${applySuggestionUrl}/${c.id}`} style="margin:0;display:inline;">
660 <button type="submit" class="diff-apply-btn">Apply suggestion</button>
661 </form>
662 )}
663 </div>
664 <pre class="diff-suggestion-code">{suggCode}</pre>
665 </div>
666 {afterFence && (
667 <div class="diff-inline-comment-body" style="margin-top:6px;" dangerouslySetInnerHTML={{ __html: afterFence }} />
668 )}
669 </div>
670 );
671 }
672 return (
673 <div class={`diff-inline-comment${c.isAiReview ? " diff-inline-comment-ai" : ""}`} data-comment-id={c.id}>
674 <div class="diff-inline-comment-head">
675 <strong>{c.authorUsername}</strong>
676 <span class="diff-inline-comment-meta">
677 {c.isAiReview && <span class="diff-inline-ai-badge">AI</span>}
678 {new Date(c.createdAt).toLocaleDateString()}
679 </span>
680 </div>
681 <div class="diff-inline-comment-body" dangerouslySetInnerHTML={{ __html: c.body }} />
682 </div>
683 );
684 })}
685 </>
686 );
687 })}
688 </>
689 ))}
690 </div>
691 )}
798 <DiffFileBody
799 file={file}
800 isSplit={isSplit}
801 blobHref={blobHref}
802 commentsByLine={commentsByLine}
803 commentActionUrl={commentActionUrl}
804 applySuggestionUrl={applySuggestionUrl}
805 pendingReviewUrl={pendingReviewUrl}
806 />
692807 </details>
693808 );
694809 })}
767882 );
768883};
769884
885// ─── Big-PR mode (move #6A) ────────────────────────────────────────────
886
887/**
888 * HTML fragment for ONE file's diff, served by
889 * `GET /:owner/:repo/pulls/:number/files/:index/patch` and injected into a
890 * `DiffFileListView` card on expand. `raw` is expected to hold a single
891 * file's `git diff` output; extra files, if any, are ignored. No <style>
892 * or <script> here — the host page already carries both.
893 */
894export interface SingleFileDiffFragmentProps {
895 raw: string;
896 isSplit?: boolean;
897 blobHref?: string | null;
898 inlineComments?: InlineDiffComment[];
899 commentActionUrl?: string;
900 applySuggestionUrl?: string;
901 pendingReviewUrl?: string;
902}
903
904export const SingleFileDiffFragment: FC<SingleFileDiffFragmentProps> = ({
905 raw,
906 isSplit,
907 blobHref,
908 inlineComments,
909 commentActionUrl,
910 applySuggestionUrl,
911 pendingReviewUrl,
912}) => {
913 const parsed = parseUnifiedDiff(raw);
914 const file = parsed[0];
915 if (!file) return <div class="diff-empty">No textual changes.</div>;
916
917 const commentsByLine = new Map<string, InlineDiffComment[]>();
918 for (const c of inlineComments ?? []) {
919 const key = `${c.filePath}:${c.lineNumber}`;
920 const arr = commentsByLine.get(key) ?? [];
921 arr.push(c);
922 commentsByLine.set(key, arr);
923 }
924
925 return (
926 <DiffFileBody
927 file={file}
928 isSplit={isSplit}
929 blobHref={blobHref ?? null}
930 commentsByLine={commentsByLine}
931 commentActionUrl={commentActionUrl}
932 applySuggestionUrl={applySuggestionUrl}
933 pendingReviewUrl={pendingReviewUrl}
934 />
935 );
936};
937
938/** One row of the big-PR per-file list — shape-compatible with
939 * `PrDiffFileSummary` from src/lib/pr-diff-budget.ts (kept structural so
940 * this view module stays import-light). */
941export interface DiffFileListItem {
942 path: string;
943 oldPath: string | null;
944 status: "added" | "modified" | "renamed" | "deleted" | "binary" | "changed";
945 additions: number;
946 deletions: number;
947 binary: boolean;
948}
949
950export interface DiffFileListViewProps {
951 files: DiffFileListItem[];
952 /** Fragment endpoint base: `/${owner}/${repo}/pulls/${n}/files` —
953 * each card fetches `${patchUrlBase}/${index}/patch?path=…` on expand. */
954 patchUrlBase: string;
955 viewFileBase?: string;
956 commentActionUrl?: string;
957 applySuggestionUrl?: string;
958 pendingReviewUrl?: string;
959}
960
961/**
962 * Big-PR "Files changed" render: the per-file summary list with lazy-loaded
963 * patches. Visuals reuse the exact DiffView card classes (diff-file,
964 * diff-file-summary, StatusPill, StatPills) so the two modes read as one
965 * surface; below-threshold PRs never see this component.
966 */
967export const DiffFileListView: FC<DiffFileListViewProps> = ({
968 files,
969 patchUrlBase,
970 viewFileBase,
971 commentActionUrl,
972 applySuggestionUrl,
973 pendingReviewUrl,
974}) => {
975 const totalAdd = files.reduce((s, f) => s + f.additions, 0);
976 const totalDel = files.reduce((s, f) => s + f.deletions, 0);
977 return (
978 <div class="diff-view diff-view-lazy">
979 <style dangerouslySetInnerHTML={{ __html: DIFF_VIEW_CSS }} />
980
981 <div class="diff-summary">
982 <span class="diff-summary-count">
983 <strong>{files.length}</strong>{" "}
984 changed file{files.length !== 1 ? "s" : ""}
985 </span>
986 <StatPills add={totalAdd} del={totalDel} />
987 <span class="diff-lazy-note">
988 Large diff — each file loads when expanded.
989 </span>
990 </div>
991
992 {files.map((f, i) => (
993 <details
994 class="diff-file diff-file-lazy"
995 id={`diff-file-${i}`}
996 data-patch-url={`${patchUrlBase}/${i}/patch?path=${encodeURIComponent(f.path)}`}
997 >
998 <summary class="diff-file-summary">
999 <span class="diff-file-chevron" aria-hidden="true">{"▾"}</span>
1000 {f.status === "changed" ? (
1001 <span class="diff-status diff-status-binary">Changed</span>
1002 ) : (
1003 <StatusPill status={f.status} />
1004 )}
1005 <span class="diff-file-path" title={f.path}>
1006 {f.oldPath ? (
1007 <>
1008 <span class="diff-file-old">{f.oldPath}</span>
1009 <span class="diff-file-arrow" aria-hidden="true">{" → "}</span>
1010 <span class="diff-file-new">{f.path}</span>
1011 </>
1012 ) : (
1013 f.path
1014 )}
1015 </span>
1016 <span class="diff-file-spacer" />
1017 <StatPills add={f.additions} del={f.deletions} />
1018 {viewFileBase && (
1019 <a
1020 href={`${viewFileBase}/${f.path}`}
1021 class="diff-file-blob-link"
1022 title="View file at this revision"
1023 >
1024 View file
1025 </a>
1026 )}
1027 </summary>
1028 <div class="diff-lazy-slot">
1029 <div class="diff-empty diff-lazy-loading">Loading diff…</div>
1030 </div>
1031 </details>
1032 ))}
1033
1034 {commentActionUrl && (
1035 <meta name="diff-comment-url" content={commentActionUrl} />
1036 )}
1037 {applySuggestionUrl && (
1038 <meta name="diff-apply-suggestion-url" content={applySuggestionUrl} />
1039 )}
1040 {pendingReviewUrl && (
1041 <meta name="diff-pending-review-url" content={pendingReviewUrl} />
1042 )}
1043
1044 {/* DIFF_VIEW_JS is document-delegated, so copy-path and inline-comment
1045 "+" buttons keep working inside fragments injected later. */}
1046 <script dangerouslySetInnerHTML={{ __html: DIFF_VIEW_JS }} />
1047 <script dangerouslySetInnerHTML={{ __html: DIFF_LAZY_JS }} />
1048 </div>
1049 );
1050};
1051
1052// ─── Inline script: lazy per-file patch loading (big-PR mode) ──────────
1053
1054const DIFF_LAZY_JS = `
1055(function () {
1056 function load(d) {
1057 if (d.getAttribute('data-loaded')) return;
1058 d.setAttribute('data-loaded', '1');
1059 var slot = d.querySelector('.diff-lazy-slot');
1060 var url = d.getAttribute('data-patch-url');
1061 if (!slot || !url) return;
1062 slot.innerHTML = '<div class="diff-empty diff-lazy-loading">Loading diff\\u2026</div>';
1063 fetch(url, { credentials: 'same-origin' })
1064 .then(function (r) {
1065 if (!r.ok) throw new Error('HTTP ' + r.status);
1066 return r.text();
1067 })
1068 .then(function (html) { slot.innerHTML = html; })
1069 .catch(function () {
1070 d.removeAttribute('data-loaded');
1071 slot.innerHTML = '<div class="diff-empty">Could not load this file\\u2019s diff. <button type="button" class="diff-lazy-retry">Retry</button></div>';
1072 });
1073 }
1074 // 'toggle' does not bubble — listen in the capture phase so one document
1075 // listener covers every card, including any injected later.
1076 document.addEventListener('toggle', function (e) {
1077 var d = e.target;
1078 if (d && d.classList && d.classList.contains('diff-file-lazy') && d.open) load(d);
1079 }, true);
1080 document.addEventListener('click', function (e) {
1081 var t = e.target;
1082 var btn = t && t.closest && t.closest('.diff-lazy-retry');
1083 if (!btn) return;
1084 var d = btn.closest('.diff-file-lazy');
1085 if (d) load(d);
1086 });
1087})();
1088`;
1089
7701090// ─── Inline script: copy-path button ───────────────────────────────────
7711091
7721092const DIFF_VIEW_JS = `
15161836 .diff-table-split { table-layout: fixed; }
15171837 .diff-split-row:hover { filter: brightness(1.05); }
15181838 .diff-split-hunk-row td { border-top: 1px solid color-mix(in srgb, var(--accent) 18%, transparent); border-bottom: 1px solid color-mix(in srgb, var(--accent) 18%, transparent); }
1839
1840 /* ─── Big-PR lazy file list (move #6A) ─── */
1841 .diff-lazy-note {
1842 font-size: 12px;
1843 color: var(--text-muted);
1844 }
1845 .diff-lazy-loading { color: var(--text-muted); }
1846 .diff-lazy-retry {
1847 background: transparent;
1848 color: var(--accent);
1849 border: 1px solid var(--border);
1850 border-radius: 5px;
1851 padding: 3px 10px;
1852 font-size: 12px;
1853 cursor: pointer;
1854 font-family: var(--font-sans, inherit);
1855 }
1856
1857 /* ─── Review-thread resolve state (move #6B) ─── */
1858 .diff-thread-bar {
1859 display: flex;
1860 align-items: center;
1861 gap: 8px;
1862 padding: 6px 14px;
1863 background: color-mix(in srgb, var(--accent) 5%, var(--bg-elevated));
1864 border-top: 1px solid var(--border);
1865 border-bottom: 1px solid var(--border);
1866 font-family: var(--font-sans, inherit);
1867 font-size: 12px;
1868 }
1869 .diff-thread-badge { color: var(--accent); font-weight: 600; }
1870 .diff-thread-toggle {
1871 background: transparent;
1872 border: none;
1873 color: var(--text-muted);
1874 font-size: 12px;
1875 cursor: pointer;
1876 text-decoration: underline;
1877 padding: 0;
1878 font-family: var(--font-sans, inherit);
1879 }
1880 .diff-thread-toggle:hover { color: var(--text); }
1881 .diff-thread-spacer { flex: 1; }
1882 .diff-thread-form { margin: 0; display: inline; }
1883 .diff-thread-quiet-btn {
1884 background: transparent;
1885 color: var(--text-muted);
1886 border: 1px solid var(--border);
1887 border-radius: 5px;
1888 padding: 3px 10px;
1889 font-size: 12px;
1890 cursor: pointer;
1891 font-family: var(--font-sans, inherit);
1892 transition: all 120ms ease;
1893 }
1894 .diff-thread-quiet-btn:hover {
1895 color: var(--accent);
1896 border-color: color-mix(in srgb, var(--accent) 45%, transparent);
1897 }
1898 .diff-thread-resolve-row {
1899 display: block;
1900 padding: 4px 14px 8px;
1901 background: var(--bg-elevated);
1902 }
15191903`;
15201904
15211905
c comment · e edit title · m merge · a approve · r request changes · ? shortcuts