Skip to content

Commit 447afd0

Browse files
committed
feat(shared): bring the Code tab closer to the real PR diff viewer
Adds what was missing: independent old/new line-number gutters, a "Files changed" nav (linked anchors, shown once there's more than one file), word-level diff highlighting for edited lines (longest common prefix/suffix, only when a remove/add run pairs 1:1), a "no newline at end of file" marker, and a dimmed directory / bold filename split in each file's header — matching the real PullRequestCodeTab more closely than the previous flat, whole-line-only rendering.
1 parent df31d00 commit 447afd0

2 files changed

Lines changed: 371 additions & 19 deletions

File tree

packages/shared/src/review-report.test.ts

Lines changed: 124 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,9 @@ describe("renderReviewReportHtml", () => {
172172

173173
expect(html).toContain('id="tab-summary"');
174174
expect(html).toContain('id="panel-code"');
175-
expect(html).toContain("src/foo.ts");
175+
expect(html).toContain(
176+
'<span class="diff-file-dir">src/</span><span class="diff-file-base">foo.ts</span>',
177+
);
176178
expect(html).toContain('<span class="diff-stat-add">+1</span>');
177179
expect(html).toContain('<span class="diff-stat-del">-1</span>');
178180
expect(html).toContain('class="diff-line diff-add"');
@@ -199,7 +201,9 @@ describe("renderReviewReportHtml", () => {
199201
findings: [],
200202
diff,
201203
});
202-
expect(html).toContain("src/gone.ts");
204+
expect(html).toContain(
205+
'<span class="diff-file-dir">src/</span><span class="diff-file-base">gone.ts</span>',
206+
);
203207
expect(html).toContain('<span class="diff-stat-del">-2</span>');
204208
});
205209

@@ -260,8 +264,12 @@ describe("renderReviewReportHtml", () => {
260264
});
261265
expect(html).toContain('class="diff-line diff-remove"');
262266
expect(html).toContain('class="diff-line diff-add"');
263-
expect(html).toContain("-- old comment");
264-
expect(html).toContain("++ new comment");
267+
// Word-level diff pairs the two lines and highlights only the changed
268+
// span — "old"/"new" — leaving the shared " comment" suffix plain.
269+
expect(html).toContain(
270+
'<mark class="diff-word-remove">-- old</mark> comment',
271+
);
272+
expect(html).toContain('<mark class="diff-word-add">++ new</mark> comment');
265273
});
266274

267275
it("resolves a pure rename with no content change from rename to/from lines, even with no a/b prefix", () => {
@@ -370,6 +378,117 @@ describe("renderReviewReportHtml", () => {
370378
findings: [],
371379
diff,
372380
});
373-
expect(html).toContain("a b/c.ts");
381+
expect(html).toContain(
382+
'<span class="diff-file-dir">a b/</span><span class="diff-file-base">c.ts</span>',
383+
);
384+
});
385+
386+
it("numbers context/add/remove lines from the hunk header, old and new columns independently", () => {
387+
const diff = [
388+
"diff --git a/src/foo.ts b/src/foo.ts",
389+
"--- a/src/foo.ts",
390+
"+++ b/src/foo.ts",
391+
"@@ -10,3 +10,4 @@",
392+
" kept line",
393+
"-removed line",
394+
"+added line one",
395+
"+added line two",
396+
" trailing context",
397+
].join("\n");
398+
399+
const html = renderReviewReportHtml({
400+
title: "Fix bug",
401+
generatedAt: "2026-01-01T00:00:00.000Z",
402+
findings: [],
403+
diff,
404+
});
405+
406+
// Context line 10 keeps the same number on both sides.
407+
expect(html).toContain(
408+
'<span class="diff-ln">10</span>\n\t<span class="diff-ln">10</span>',
409+
);
410+
// The removed line only has an old-side number.
411+
expect(html).toContain(
412+
'<span class="diff-ln">11</span>\n\t<span class="diff-ln"></span>',
413+
);
414+
// Both added lines only have new-side numbers, continuing from 11.
415+
expect(html).toContain(
416+
'<span class="diff-ln"></span>\n\t<span class="diff-ln">11</span>',
417+
);
418+
expect(html).toContain(
419+
'<span class="diff-ln"></span>\n\t<span class="diff-ln">12</span>',
420+
);
421+
// Trailing context resumes in sync: old 12, new 13.
422+
expect(html).toContain(
423+
'<span class="diff-ln">12</span>\n\t<span class="diff-ln">13</span>',
424+
);
425+
});
426+
427+
it("marks a line with no trailing newline", () => {
428+
const diff = [
429+
"diff --git a/src/foo.ts b/src/foo.ts",
430+
"--- a/src/foo.ts",
431+
"+++ b/src/foo.ts",
432+
"@@ -1 +1 @@",
433+
"-old",
434+
"+new",
435+
"\\ No newline at end of file",
436+
].join("\n");
437+
438+
const html = renderReviewReportHtml({
439+
title: "Fix bug",
440+
generatedAt: "2026-01-01T00:00:00.000Z",
441+
findings: [],
442+
diff,
443+
});
444+
expect(html).toContain(
445+
'<span class="diff-no-newline">No newline at end of file</span>',
446+
);
447+
});
448+
449+
it("shows a Files changed nav with a link per file, only when there's more than one file", () => {
450+
const singleFileDiff = [
451+
"diff --git a/a.ts b/a.ts",
452+
"--- a/a.ts",
453+
"+++ b/a.ts",
454+
"@@ -1 +1 @@",
455+
"-x",
456+
"+y",
457+
].join("\n");
458+
const single = renderReviewReportHtml({
459+
title: "Fix bug",
460+
generatedAt: "2026-01-01T00:00:00.000Z",
461+
findings: [],
462+
diff: singleFileDiff,
463+
});
464+
expect(single).not.toContain('<nav class="diff-files-nav">');
465+
466+
const twoFileDiff = [
467+
"diff --git a/a.ts b/a.ts",
468+
"--- a/a.ts",
469+
"+++ b/a.ts",
470+
"@@ -1 +1 @@",
471+
"-x",
472+
"+y",
473+
"diff --git a/b.ts b/b.ts",
474+
"--- a/b.ts",
475+
"+++ b/b.ts",
476+
"@@ -1 +1 @@",
477+
"-x",
478+
"+y",
479+
].join("\n");
480+
const html = renderReviewReportHtml({
481+
title: "Fix bug",
482+
generatedAt: "2026-01-01T00:00:00.000Z",
483+
findings: [],
484+
diff: twoFileDiff,
485+
});
486+
expect(html).toContain(
487+
'<h3>Files changed <span class="diff-files-nav-count">2</span></h3>',
488+
);
489+
expect(html).toContain('<a href="#diff-file-0"');
490+
expect(html).toContain('<a href="#diff-file-1"');
491+
expect(html).toContain('id="diff-file-0"');
492+
expect(html).toContain('id="diff-file-1"');
374493
});
375494
});

0 commit comments

Comments
 (0)