#1feat: add guided-review — turn a pull request into a narrated code review

+2,661 −0

Guided review

A tool that explains pull requests, explaining itself

GitHub opens this pull request at .claude-plugin/plugin.json, then .gitignore, then LICENSE — three files carrying no idea at all — while the parser and the renderer everything is built on sit somewhere in the middle. That gap between the order files are listed in and the order they make sense in is the entire problem this code exists to close.

So it seemed fair to let the tool argue its own case. What follows is guided-review reviewing the pull request that adds guided-review.

Seventeen files, all new. We start with the split that makes everything else possible, then the two pieces that give a diff coordinates — parsing and highlighting — then the pairing that is the whole point, the checks that keep a narrative honest, the design, and the tests.

@MarceloCajueirofeat/guided-review-skill2,661 +0 −17 files
Chapter 01

The split: analysis is not rendering

The first decision, and the one every other decision leans on.

A model reads the pull request and writes a story JSON. build.mjs parses the diff, resolves anchors to line numbers, and renders the page — and does no analysis whatsoever.

That line is drawn deliberately. It is why fixing a clumsy sentence is a re-render measured in milliseconds rather than a re-read of a 2,000-line diff.

build.mjsnew+230 −0
@@ -0,0 +1,230 @@
1#!/usr/bin/env node
2// guided-review — turn a story JSON + a unified diff into one self-contained
3// HTML page for guided code review.
4//
5// Usage:
6// node build.mjs <story.json> <pr.diff> [--out <file.html>] [--lang pt|en]
7//
8// The story JSON is authored by the model (see the skill); this script does no
9// analysis. It parses the diff, pairs each narrative beat with the exact lines
10// it describes, and renders the page. Keeping the two apart means a wording fix
11// is a re-render, not a re-analysis.
12
13import { existsSync, readFileSync, writeFileSync } from 'node:fs';
14import { basename } from 'node:path';
15import { parseDiff, parseRange } from './lib/diff.mjs';
16import { render } from './lib/render.mjs';
17
18const USAGE = `guided-review — build a guided code review page
19
20Usage:
21 node build.mjs <story.json> <diff-file> [options]
22
23Arguments:
24 <story.json> Narrative written by the model (schema in the skill / README).
25 <diff-file> Unified diff, e.g. \`gh pr diff <N> > pr.diff\`.
26
27Options:
28 --out <file> Output HTML (default: guided-review-<pr>.html in the current directory).
29 --lang <code> UI language for fixed labels: \`en\` (default) or \`pt\`.
30 Only chrome — the narrative renders in whatever language it was written in.
31 --help Show this help.
32
33Exit codes: 0 ok, 1 usage/validation error.
34`;
35
36function fail(msg) {
37 console.error(`error: ${msg}`);
38 process.exit(1);
39}
40
41function parseArgs(argv) {
42 const positional = [];
43 const opts = { lang: 'en' };
44 for (let i = 0; i < argv.length; i++) {
45 const a = argv[i];
46 if (a === '--help' || a === '-h') { console.log(USAGE); process.exit(0); }
47 if (!a.startsWith('--')) { positional.push(a); continue; }
48 const key = a.slice(2);
49 if (key !== 'out' && key !== 'lang') fail(`unknown flag --${key}`);
50 const next = argv[i + 1];
51 if (next === undefined || next.startsWith('--')) fail(`--${key} needs a value`);
52 opts[key] = next;
⋯ 178 lines not shown
Showing the passage this section is about — read the full file on GitHub.

The skill is the other half of that split — the part build.mjs deliberately knows nothing about. It tells the model how to read a PR, how to find the order the story wants to be told in, and how to write anchors that point at real lines.

Its firmest rule is a negative one: no findings, no severities, no suggested changes.

skills/guided-review/SKILL.mdnew+200 −0
⋯ 6 lines not shown
7
8# guided-review — a pull request, told as a story
9
10Produces one self-contained HTML page: the PR narrated in **chapters**, in the order the
11decisions were actually made, with each paragraph paired against the exact diff lines it
12describes.
13
14The problem it solves: GitHub sorts files alphabetically, which is never the order the work
15happened in. The description explains the reasoning in one place; the code sits somewhere
16else. The reviewer reads a wall of prose, then a wall of diff, and reassembles the mapping in
17their head. This puts each part of the explanation next to the code it is about.
18
19**Scope: narrative only.** This does not hunt for bugs, rate severity, or suggest changes —
20that is `/cr` (or `/code-review`). Mixing the two produces a document that is neither: a
21reviewer reading for *intent* is doing something different from a reviewer reading for
22*defects*. Keep them separate.
23
24## How it works
25
26You do the analysis and write a story JSON. `build.mjs` does the mechanical part: parsing the
27diff, resolving anchors to line numbers, rendering the page. Keeping them apart means a
28wording fix is a re-render, not a re-analysis.
29
30The script lives at the plugin root. Reference it with `${CLAUDE_PLUGIN_ROOT}`.
31
⋯ 32 lines not shown
64While reading, hunt specifically for:
65
66- **Decisions with a visible alternative** — a rejected approach, a deliberate duplication, a
67 `rescue` that isn't there, a scope that was widened instead of removed. These are what a
68 reviewer cannot reconstruct from the diff alone, and they are the reason this document exists.
69- **Non-obvious constraints** — a measured timing, a database that refuses a query, a feature
70 gate, a framework behavior being worked around.
71- **Load-bearing tests** — the spec that exists because the failure it prevents is silent.
72
73If the code contains a comment explaining *why*, that is your raw material. Quote its
74substance in the narrative; don't paraphrase it into vagueness.
75
76### 3. Find the order the story wants to be told in
77
78Not alphabetical, not the diff order. Ask: **what does a reader need to believe first for the
79next part to make sense?** Common shapes:
80
81- *Definition → measurement → presentation → access* (a new feature)
82- *Symptom → root cause → fix → guard against regression* (a bug fix)
83- *What the old thing couldn't do → the new seam → migrating each caller* (a refactor)
84
85Group into **4–8 chapters**. Fewer than 4 usually means the chapters are too coarse to guide
86anyone; more than 8 means you're narrating files instead of decisions. Every chapter should be
87nameable as a decision or a movement, never as a directory (`"Services"` is not a chapter;
88`"One definition of pending"` is).
89
90### 4. Write the story JSON
91
92```jsonc
93{
94 "number": 3649,
95 "title": "…", // the PR title
96 "url": "https://github.com/…/pull/3649",
97 "author": "octocat",
98 "branch": "feature-branch",
99 "additions": 2790, // from gh; shown in the masthead
100 "deletions": 7,
101 "headSha": "abc123…", // headRefOid — stamps the page
102 "headline": "…", // the one sentence that makes someone want to read
⋯ 98 lines not shown
Showing the passage this section is about — read the full file on GitHub.
Chapter 02

Giving a diff coordinates

Before prose can point at code, something has to make the code addressable.

The parser tracks line numbers on both sides of every hunk. That is what lets a paragraph say app/foo.rb:42 without knowing anything about hunk offsets — the foundation the entire pairing rests on.

The first thing it does is drop the trailing empty string that split('\n') produces for any newline-terminated diff, which is every diff.

lib/diff.mjsnew+141 −0
⋯ 17 lines not shown
18 /(^|\/)dist\//,
19];
20
21export function isGenerated(path) {
22 return GENERATED.some((re) => re.test(path));
23}
24
25/**
26 * Parse a unified diff into structured files.
27 * @param {string} text raw diff
28 * @returns {Array<{path,oldPath,status,isBinary,isGenerated,additions,deletions,hunks}>}
29 */
30export function parseDiff(text) {
31 const files = [];
32 let file = null;
33 let hunk = null;
34 let oldLine = 0;
35 let newLine = 0;
36
37 const pushFile = () => {
38 if (file) files.push(file);
39 };
40
41 // A newline-terminated diff — which is every `git diff` / `gh pr diff` —
42 // splits into a trailing '' that is not a line of the file. Consumed as
43 // context it fabricates a row, bumps both line counters, and registers a
44 // line number that does not exist as a valid anchor target.
45 const lines = text.split('\n');
46 if (lines[lines.length - 1] === '') lines.pop();
47
48 for (const raw of lines) {
49 const header = FILE_HEADER.exec(raw);
50 if (header) {
51 pushFile();
52 const [, oldPath, newPath] = header;
53 file = {
54 path: newPath,
55 oldPath,
56 status: 'modified',
57 isBinary: false,
58 isGenerated: isGenerated(newPath),
59 additions: 0,
60 deletions: 0,
61 hunks: [],
62 };
63 hunk = null;
64 continue;
65 }
66 if (!file) continue; // preamble before the first file
67
68 // Metadata only exists between the file header and the first hunk. Testing
69 // for it inside a hunk eats real content: a deleted line `--- foo` in a
70 // changelog is `--- foo` on the wire, indistinguishable from a `---` header
71 // by prefix alone, and it would vanish from the diff without a trace.
72 if (!hunk) {
⋯ 69 lines not shown
Showing the passage this section is about — read the full file on GitHub.

Metadata prefixes — ---, +++, index — are only tested before the first hunk, because that is the only place they occur.

lib/diff.mjsnew+141 −0
⋯ 55 lines not shown
56 status: 'modified',
57 isBinary: false,
58 isGenerated: isGenerated(newPath),
59 additions: 0,
60 deletions: 0,
61 hunks: [],
62 };
63 hunk = null;
64 continue;
65 }
66 if (!file) continue; // preamble before the first file
67
68 // Metadata only exists between the file header and the first hunk. Testing
69 // for it inside a hunk eats real content: a deleted line `--- foo` in a
70 // changelog is `--- foo` on the wire, indistinguishable from a `---` header
71 // by prefix alone, and it would vanish from the diff without a trace.
72 if (!hunk) {
73 if (raw.startsWith('new file mode')) { file.status = 'added'; continue; }
74 if (raw.startsWith('deleted file mode')) { file.status = 'deleted'; continue; }
75 if (raw.startsWith('rename from')) { file.status = 'renamed'; continue; }
76 if (raw.startsWith('Binary files')) { file.isBinary = true; continue; }
77 // `+++`/`---` carry no information the file header lacks.
78 if (raw.startsWith('index ') || raw.startsWith('--- ') || raw.startsWith('+++ ') ||
79 raw.startsWith('similarity index') || raw.startsWith('rename to') ||
80 raw.startsWith('old mode') || raw.startsWith('new mode')) continue;
81 }
82
83 const hh = HUNK_HEADER.exec(raw);
84 if (hh) {
85 oldLine = Number(hh[1]);
86 newLine = Number(hh[2]);
87 hunk = { header: raw, lines: [] };
88 file.hunks.push(hunk);
89 continue;
90 }
91 if (!hunk) continue;
92
93 // `\ No newline at end of file` annotates the previous line; it occupies no
94 // line number on either side.
⋯ 47 lines not shown
Showing the passage this section is about — read the full file on GitHub.

Anchors are split by parseRange, which flags a colon suffix it cannot parse instead of quietly folding it into the path.

lib/diff.mjsnew+141 −0
⋯ 99 lines not shown
100 hunk.lines.push({ type: 'add', content, oldNo: null, newNo: newLine++ });
101 file.additions++;
102 } else if (marker === '-') {
103 hunk.lines.push({ type: 'del', content, oldNo: oldLine++, newNo: null });
104 file.deletions++;
105 } else if (marker === ' ' || raw === '') {
106 hunk.lines.push({ type: 'ctx', content, oldNo: oldLine++, newNo: newLine++ });
107 }
108 }
109 pushFile();
110 return files;
111}
112
113/**
114 * Split an anchor into its parts.
115 *
116 * `path/to/f.rb:17-42` → `{ path, from: 17, to: 42 }`
117 * `path/to/f.rb:17` → `{ path, from: 17, to: 17 }`
118 * `path/to/f.rb` → `{ path, from: null, to: null }` — whole file
119 *
120 * A colon suffix that is not a valid range sets `malformed`, and the whole spec
121 * is kept as the path. That distinction matters: without it, `f.rb:L17-L42`
122 * (GitHub's own fragment syntax, an easy thing to write by mistake) is reported
123 * as "no such file in the diff" — an error naming the wrong cause and pointing
124 * the author at the filename instead of at the range they mistyped.
125 *
126 * A range is never reordered or clamped here; `validate` rejects `from > to`
127 * and `from < 1` outright, since both are authoring errors with no sensible
128 * interpretation.
129 */
130export function parseRange(spec) {
131 const at = spec.lastIndexOf(':');
132 if (at === -1) return { path: spec, from: null, to: null };
133 const path = spec.slice(0, at);
134 const range = spec.slice(at + 1);
135 const m = /^(\d+)(?:-(\d+))?$/.exec(range);
136 // Could be a path that legitimately contains a colon, or a typo'd range —
137 // indistinguishable here, so flag it and let the caller decide.
138 if (!m) return { path: spec, from: null, to: null, malformed: true };
139 const from = Number(m[1]);
140 return { path, from, to: m[2] ? Number(m[2]) : from };
141}
Showing the passage this section is about — read the full file on GitHub.

Some files are machine-written. structure.sql, lockfiles, dist/, vendor/ — reading them line by line teaches a reviewer nothing, so they are detected by path and collapsed on sight.

lib/diff.mjsnew+141 −0
@@ -0,0 +1,141 @@
1// Parser for unified diffs (`git diff` / `gh pr diff` output).
2//
3// Produces one entry per file, each holding its hunks and lines. Line numbers
4// are tracked for both sides so the renderer can anchor a narrative beat to
5// "app/foo.rb:42" without the narrative having to know about hunk offsets.
6
7const FILE_HEADER = /^diff --git a\/(.+?) b\/(.+)$/;
8const HUNK_HEADER = /^@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@/;
9
10// Paths whose diff is machine-generated: shown collapsed by default, since
11// reading them line by line teaches a reviewer nothing.
12const GENERATED = [
13 /(^|\/)db\/structure\.sql$/,
14 /(^|\/)db\/schema\.rb$/,
15 /(^|\/)(package-lock\.json|yarn\.lock|pnpm-lock\.yaml|Gemfile\.lock|poetry\.lock|composer\.lock|Cargo\.lock|uv\.lock)$/,
16 /\.min\.(js|css)$/,
17 /(^|\/)vendor\//,
18 /(^|\/)dist\//,
19];
20
21export function isGenerated(path) {
22 return GENERATED.some((re) => re.test(path));
23}
24
25/**
26 * Parse a unified diff into structured files.
27 * @param {string} text raw diff
28 * @returns {Array<{path,oldPath,status,isBinary,isGenerated,additions,deletions,hunks}>}
29 */
30export function parseDiff(text) {
31 const files = [];
32 let file = null;
33 let hunk = null;
34 let oldLine = 0;
35 let newLine = 0;
⋯ 106 lines not shown
Showing the passage this section is about — read the full file on GitHub.

Syntax highlighting is regex tokenization, not parsing. This looks like a shortcut and is the only thing that works here.

The rules are joined into one alternation, and each capture-group slot is mapped back to the rule that owns it.

lib/highlight.mjsnew+189 −0
@@ -0,0 +1,189 @@
1// Minimal, dependency-free syntax highlighting.
2//
3// This is deliberately not a real parser. A diff shows fragments — half a
4// function, a hunk starting mid-string — so any parser strict enough to be
5// "correct" would fail on most input. Regex tokenization degrades gracefully:
6// the worst case is a missed keyword, never a broken line.
7//
8// Scope is limited to what actually shows up in pull requests. Unknown
9// extensions fall through to `escapeHtml`, which is always safe.
10
11const KEYWORDS = {
12 ruby: 'def|end|class|module|if|elsif|else|unless|while|until|for|in|do|then|begin|rescue|ensure|raise|return|yield|self|nil|true|false|and|or|not|require|require_relative|include|extend|attr_accessor|attr_reader|attr_writer|private|public|protected|lambda|proc|case|when|next|break|super|new|defined',
13 js: 'const|let|var|function|return|if|else|for|while|do|switch|case|break|continue|new|class|extends|super|this|import|export|from|default|async|await|try|catch|finally|throw|typeof|instanceof|null|undefined|true|false|delete|void|yield|static|get|set|of|in',
14 python: 'def|class|return|if|elif|else|for|while|in|is|not|and|or|import|from|as|with|try|except|finally|raise|lambda|None|True|False|pass|break|continue|yield|global|nonlocal|assert|async|await|self',
15 sql: 'SELECT|FROM|WHERE|JOIN|LEFT|RIGHT|INNER|OUTER|ON|GROUP|BY|ORDER|HAVING|LIMIT|OFFSET|INSERT|INTO|VALUES|UPDATE|SET|DELETE|CREATE|TABLE|INDEX|ALTER|DROP|ADD|COLUMN|PRIMARY|KEY|FOREIGN|REFERENCES|NOT|NULL|DEFAULT|UNIQUE|CONSTRAINT|AS|AND|OR|IN|EXISTS|CASE|WHEN|THEN|ELSE|END|DISTINCT|COUNT|SUM|AVG|MAX|MIN|COALESCE|WITH|UNION|ALL',
16 css: 'important|media|import|supports|keyframes|font-face|root|and|not|only',
17 go: 'func|package|import|return|if|else|for|range|switch|case|default|break|continue|var|const|type|struct|interface|map|chan|go|defer|select|nil|true|false|error|string|int|bool|make|new|append|len|cap',
18 rust: 'fn|let|mut|const|static|if|else|match|for|while|loop|break|continue|return|struct|enum|impl|trait|pub|use|mod|crate|self|Self|super|where|as|dyn|move|ref|type|unsafe|async|await|true|false|Some|None|Ok|Err',
19 php: 'function|class|extends|implements|interface|trait|public|private|protected|static|return|if|else|elseif|foreach|for|while|do|switch|case|break|continue|new|echo|print|use|namespace|require|include|try|catch|finally|throw|null|true|false|array|const|abstract|final|global',
20 java: 'public|private|protected|class|interface|extends|implements|static|final|void|return|if|else|for|while|do|switch|case|break|continue|new|try|catch|finally|throw|throws|import|package|abstract|synchronized|this|super|null|true|false|instanceof|enum|record',
21 shell: 'if|then|else|elif|fi|for|while|do|done|case|esac|function|return|export|local|source|echo|cd|set|unset|readonly|shift|exit|trap',
⋯ 121 lines not shown
143 */
144export function highlight(line, lang) {
145 if (!lang || !line) return escapeHtml(line);
146 // The string rules backtrack badly on pathological input — 5,000 escaped
147 // quotes on one line costs ~90ms. Generated files are usually collapsed
148 // before reaching here, but "usually" is not a guarantee, and no line this
149 // long is being read by a human anyway.
150 if (line.length > 2000) return escapeHtml(line);
151 if (!RULE_CACHE.has(lang)) {
152 const rules = rulesFor(lang);
153 // One combined regex; the matched alternative tells us which class to use.
154 const source = rules.map(([re]) => `(${re.source})`).join('|');
155 // Map each capture-group slot back to the rule that owns it. Rules are free
156 // to contain groups of their own — several do — so slot N is not rule N,
157 // and assuming it was silently emitted class="" for every rule declared
158 // after the first one with an inner group.
159 const owner = [];
160 rules.forEach(([re], i) => {
161 // The wrapper group, plus however many the rule declares itself. Counted
162 // by compiling an always-matching pattern and reading its group count.
163 const inner = new RegExp(`${re.source}|`).exec('').length - 1;
164 for (let n = 0; n <= inner; n++) owner.push(i);
165 });
166 RULE_CACHE.set(lang, {
167 re: new RegExp(source, 'g' + (rules.some(([r]) => r.flags.includes('i')) ? 'i' : '')),
168 classes: rules.map(([, cls]) => cls),
169 owner,
170 });
171 }
172 const { re, classes, owner } = RULE_CACHE.get(lang);
173 re.lastIndex = 0;
174
175 let out = '';
176 let last = 0;
177 let m;
178 while ((m = re.exec(line)) !== null) {
179 // A zero-length match would spin forever; step past it.
180 if (m[0] === '') { re.lastIndex++; continue; }
181 out += escapeHtml(line.slice(last, m.index));
182 const slot = m.slice(1).findIndex((g) => g !== undefined);
⋯ 7 lines not shown
Showing the passage this section is about — read the full file on GitHub.
Chapter 03

The pairing

The one feature everything else is in service of.

A beat is the unit: one passage of prose, plus the lines it is about. Anchors resolve to a set of new-side line numbers, and those lines are highlighted beside the paragraph discussing them.

Everything else in these 2,584 lines exists so that this reads well.

lib/render.mjsnew+509 −0
⋯ 212 lines not shown
213 body = `<div class="code"><table>${rows.join('')}</table></div>`;
214 if (focused) {
215 body += `<div class="elide">Showing the passage this section is about — ` +
216 `<a href="${escapeHtml(prUrl)}">read the full file on GitHub</a>.</div>`;
217 }
218 }
219
220 const isOpen = open && !file.isGenerated ? ' open' : '';
221 return `<details class="file"${isOpen}>${head}${body}</details>`;
222}
223
224/** Collect NEW-side line numbers covered by a beat's anchors, per file. */
225function marksFor(anchors, byPath) {
226 const perFile = new Map();
227 for (const spec of anchors) {
228 const { path, from, to } = parseRange(spec);
229 const file = byPath.get(path);
230 if (!file) continue;
231 if (!perFile.has(path)) perFile.set(path, new Set());
232 if (from == null) continue; // whole-file anchor: no line highlight
233 const set = perFile.get(path);
234 for (let n = from; n <= to; n++) set.add(n);
235 }
236 return perFile;
237}
238
239function renderBeat(beat, byPath, used, labels, { key, prUrl }) {
240 const anchors = Array.isArray(beat.files) ? beat.files : [];
241 const perFile = marksFor(anchors, byPath);
242
243 const prose = `<div class="beat-prose">${paragraphs(beat.text)}${renderNotes(beat.notes, labels)}</div>`;
244
245 const files = [];
246 for (const [path, marks] of perFile) {
247 const file = byPath.get(path);
248 used.add(path);
249 files.push(renderFile(file, { marks, idScope: `${key}-L${slugPath(path)}`, prUrl }));
250 }
251 // The wrapper is emitted even with no files: it is a grid track, and dropping
252 // it collapses the two-column beat layout.
253 const code = `<div class="beat-code">${files.join('')}</div>`;
254
255 return `<div class="beat">${prose}${code}</div>`;
256}
257
258function renderChapter(ch, n, byPath, used, L, prUrl) {
259 const beats = (ch.beats || [])
260 .map((b, i) => renderBeat(b, byPath, used, L.notes, { key: `c${n}b${i + 1}`, prUrl }))
261 .join('');
262 const sub = ch.summary ? `<p class="sub">${inline(ch.summary)}</p>` : '';
263 return `
264<section class="ch" id="ch-${n}">
265 <header class="ch-head">
266 <span class="ch-num">${escapeHtml(L.chapter)} ${String(n).padStart(2, '0')}</span>
267 <h3>${inline(ch.title)}</h3>
268 ${sub}
⋯ 241 lines not shown
Showing the passage this section is about — read the full file on GitHub.

focusHunks is what makes the pairing survive a large file. It keeps a window around each anchored line and drops the rest — because an 800-line file has one or two passages the narrative points at, and rendering all of it buries them while collapsing it hides them entirely.

Every dropped line is announced. Leading gaps, gaps between kept regions, and the tail after the last one.

lib/render.mjsnew+509 −0
⋯ 91 lines not shown
92 return path.replace(/[^\w.-]/g, '_');
93}
94
95// Only http(s) reaches an href. Anything else — `javascript:`, `data:` — is
96// replaced rather than sanitized, since there is no valid reason for a PR URL
97// to carry another scheme.
98function safeUrl(u) {
99 return /^https?:\/\//i.test(String(u ?? '')) ? String(u) : '#';
100}
101
102/**
103 * Reduce a file's hunks to the neighbourhoods around `marks`, dropping the rest.
104 *
105 * An 800-line new file has one or two passages the narrative actually points
106 * at. Showing all of it buries them; collapsing the file hides them entirely.
107 * Focusing keeps the anchored passage readable in place.
108 *
109 * EVERY dropped line is accounted for by a marker — leading, between, and
110 * trailing. A segment with no lines is a pure trailing marker. Hiding code
111 * without saying so is the one failure this tool must never commit: a reviewer
112 * who reaches the end of a panel and sees no marker concludes the file ended
113 * there, and approves what they never read.
114 */
115function focusHunks(file, marks) {
116 const out = [];
117 for (const hunk of file.hunks) {
118 let keep = [];
119 let skipped = 0;
120 const flush = () => {
121 if (!keep.length) return;
122 out.push({ header: hunk.header, lines: keep, skippedBefore: skipped });
123 keep = [];
124 skipped = 0;
125 };
126 // Mark a window of FOCUS_PAD lines around every anchored line.
127 const near = new Array(hunk.lines.length).fill(false);
128 hunk.lines.forEach((l, i) => {
129 if (l.newNo == null || !marks.has(l.newNo)) return;
130 const from = Math.max(0, i - FOCUS_PAD);
131 const to = Math.min(hunk.lines.length - 1, i + FOCUS_PAD);
132 for (let n = from; n <= to; n++) near[n] = true;
133 });
134 for (let i = 0; i < hunk.lines.length; i++) {
135 if (near[i]) { keep.push(hunk.lines[i]); continue; }
136 // flush() zeroes the counter, so this line is counted once — assigning 1
137 // here instead double-counted the first line of every gap.
138 if (keep.length) flush();
139 skipped++;
140 }
141 flush();
142 // Lines trailing the last kept run: a marker with no lines of its own.
143 if (skipped) out.push({ header: hunk.header, lines: [], skippedBefore: skipped });
144 }
145 return out;
146}
147
148/**
149 * Render a file's diff. `marks` is a Set of NEW-side line numbers to highlight.
150 * Anchored files are focused to those anchors — MAX_LINES does not apply to
151 * them. Un-anchored files over MAX_LINES, and generated files, collapse to a
152 * summary line.
153 *
154 * `idScope` prefixes the per-line DOM ids; pass a value unique to this panel,
155 * or omit it to emit no ids at all (appendix panels, which nothing links to).
156 */
157function renderFile(file, { marks = new Set(), open = true, idScope = '', prUrl = '#' } = {}) {
158 const lang = languageOf(file.path);
159 const counts =
160 `<span class="counts"><span class="stat-add">+${file.additions}</span> ` +
⋯ 349 lines not shown
Showing the passage this section is about — read the full file on GitHub.

Line ids are scoped to the beat that renders them.

lib/render.mjsnew+509 −0
⋯ 183 lines not shown
184 } else if (file.isGenerated || (total > MAX_LINES && marks.size === 0)) {
185 const why = file.isGenerated
186 ? 'Generated file — collapsed by default.'
187 : `${total.toLocaleString('en-US')} changed lines — collapsed by default.`;
188 body = `<div class="elide">${why} Open on <a href="${escapeHtml(prUrl)}">GitHub</a> to read it in full.</div>`;
189 } else {
190 const hunks = focused ? focusHunks(file, marks) : file.hunks;
191 const rows = [];
192 for (const hunk of hunks) {
193 const label = focused && hunk.skippedBefore
194 ? `⋯ ${hunk.skippedBefore.toLocaleString('en-US')} lines not shown`
195 : escapeHtml(hunk.header);
196 rows.push(`<tr class="hunk"><td colspan="3">${label}</td></tr>`);
197 for (const l of hunk.lines) {
198 const row = ROW[l.type];
199 const marked = l.newNo != null && marks.has(l.newNo) ? ' mark' : '';
200 // Only the panel that owns a line mints its id. The same file is
201 // rendered once per beat that anchors it, so an unscoped id would be
202 // emitted several times — invalid HTML, and getElementById would send
203 // every deep link to whichever panel happened to render first.
204 const id = l.newNo != null && idScope ? ` id="${idScope}-${l.newNo}"` : '';
205 rows.push(
206 `<tr class="${row.cls}${marked}"${id}>` +
207 `<td class="no">${l.oldNo ?? ''}</td>` +
208 `<td class="no">${l.newNo ?? ''}</td>` +
209 `<td class="src" data-m="${row.marker}">${highlight(l.content, lang)}</td></tr>`
210 );
211 }
212 }
213 body = `<div class="code"><table>${rows.join('')}</table></div>`;
214 if (focused) {
215 body += `<div class="elide">Showing the passage this section is about — ` +
216 `<a href="${escapeHtml(prUrl)}">read the full file on GitHub</a>.</div>`;
217 }
218 }
219
220 const isOpen = open && !file.isGenerated ? ' open' : '';
221 return `<details class="file"${isOpen}>${head}${body}</details>`;
222}
223
224/** Collect NEW-side line numbers covered by a beat's anchors, per file. */
225function marksFor(anchors, byPath) {
226 const perFile = new Map();
227 for (const spec of anchors) {
⋯ 282 lines not shown
Showing the passage this section is about — read the full file on GitHub.

Author text is escaped before inline markup is applied, and code spans are lifted out before the emphasis passes run.

lib/render.mjsnew+509 −0
@@ -0,0 +1,509 @@
1// Renders the story JSON + parsed diff into one self-contained HTML page.
2
3import { CSS, REST } from './theme.mjs';
4import { escapeHtml, highlight, languageOf } from './highlight.mjs';
5import { parseRange } from './diff.mjs';
6
7// Inline markup for prose: `code`, **bold**, _italic_. Applied AFTER escaping,
8// so author text can never inject HTML.
9function inline(s) {
10 const spans = [];
11 // Code spans are lifted out before the emphasis passes and restored after.
12 // Three independent sweeps cannot see each other's boundaries, so prose like
13 // "see `a**b` and **x**" used to emit <code>a<strong>b</code> — tags crossing
14 // each other, which some browsers recover from by swallowing the rest of the
15 // paragraph. Prose about code containing ** (kwargs, pointers) is ordinary.
16 //
17 // The placeholder is delimited by U+E000 (private use area): a printable
18 // sentinel would collide with author text (" 1 " occurs in ordinary prose),
19 // and NUL would make this file read as binary to grep and diff.
20 const SENTINEL = '\uE000';
21 const escaped = escapeHtml(s ?? '').replace(/`([^`]+)`/g, (_, code) => {
22 spans.push(code);
23 return `${SENTINEL}${spans.length - 1}${SENTINEL}`;
24 });
25 return escaped
26 .replace(/\*\*([^*]+)\*\*/g, '<strong>$1</strong>')
27 .replace(/(^|\s)_([^_]+)_(?=\s|$|[.,;:!?])/g, '$1<em>$2</em>')
28 .replace(/\uE000(\d+)\uE000/g, (_, i) => `<code>${spans[Number(i)]}</code>`);
29}
30
31function paragraphs(text) {
32 return String(text ?? '')
33 .split(/\n\s*\n/)
34 .map((p) => p.trim())
35 .filter(Boolean)
36 .map((p) => `<p>${inline(p.replace(/\n/g, ' '))}</p>`)
37 .join('');
38}
39
40const NOTE_LABELS = { why: 'Why', tradeoff: 'Trade-off', rejected: 'Considered and rejected' };
41
42// `kind` defaults to `why` when absent, but an unrecognized value is rejected by
43// validate() rather than silently relabelled — presenting an author's trade-off
44// as a justification invents intent they never expressed.
45function renderNotes(notes = [], labels = NOTE_LABELS) {
46 return notes
47 .filter((n) => n && n.text)
48 .map((n) => {
⋯ 461 lines not shown
Showing the passage this section is about — read the full file on GitHub.

Only http(s) reaches an href.

lib/render.mjsnew+509 −0
⋯ 83 lines not shown
84 add: { cls: 'add', marker: '+' },
85 del: { cls: 'del', marker: '−' },
86 ctx: { cls: '', marker: ' ' },
87};
88
89// Paths go into DOM ids and URL fragments, so anything outside [\w.-] has to
90// go — a space alone makes the fragment unaddressable and breaks deep links.
91function slugPath(path) {
92 return path.replace(/[^\w.-]/g, '_');
93}
94
95// Only http(s) reaches an href. Anything else — `javascript:`, `data:` — is
96// replaced rather than sanitized, since there is no valid reason for a PR URL
97// to carry another scheme.
98function safeUrl(u) {
99 return /^https?:\/\//i.test(String(u ?? '')) ? String(u) : '#';
100}
101
102/**
103 * Reduce a file's hunks to the neighbourhoods around `marks`, dropping the rest.
104 *
105 * An 800-line new file has one or two passages the narrative actually points
106 * at. Showing all of it buries them; collapsing the file hides them entirely.
107 * Focusing keeps the anchored passage readable in place.
108 *
109 * EVERY dropped line is accounted for by a marker — leading, between, and
110 * trailing. A segment with no lines is a pure trailing marker. Hiding code
111 * without saying so is the one failure this tool must never commit: a reviewer
112 * who reaches the end of a panel and sees no marker concludes the file ended
113 * there, and approves what they never read.
114 */
115function focusHunks(file, marks) {
116 const out = [];
⋯ 393 lines not shown
Showing the passage this section is about — read the full file on GitHub.
Chapter 04

The build reviews the narrative

Two silent failures that would make the output worse than nothing.

An anchor pointing at a line that does not exist means the number was guessed rather than read off the diff. The build says so.

Ranges that cannot mean anything — running backwards, starting below line 1 — are rejected outright rather than warned about.

build.mjsnew+230 −0
⋯ 47 lines not shown
48 const key = a.slice(2);
49 if (key !== 'out' && key !== 'lang') fail(`unknown flag --${key}`);
50 const next = argv[i + 1];
51 if (next === undefined || next.startsWith('--')) fail(`--${key} needs a value`);
52 opts[key] = next;
53 i++;
54 }
55 return { positional, opts };
56}
57
58const NOTE_KINDS = new Set(['why', 'tradeoff', 'rejected']);
59
60// Validate the story up front. A malformed field here would otherwise surface
61// as a silently empty chapter in the output, which is worse than an error.
62//
63// Problems abort the run; warnings print and rendering continues. The split is
64// deliberate: a wrong anchor still produces a usable page, a wrong structure
65// does not. Every problem is collected before exiting so an author fixing a
66// story sees the whole list, not one error per run.
67//
68// Returns the paths no beat anchored.
69function validate(story, files) {
70 const byPath = new Map(files.map((f) => [f.path, f]));
71 const problems = [];
72 const warnings = [];
73 // Valid new-side line numbers, computed once per file rather than per anchor.
74 const validLines = new Map();
75 const linesOf = (path) => {
76 if (!validLines.has(path)) {
77 const set = new Set();
78 for (const h of byPath.get(path).hunks) {
79 for (const l of h.lines) if (l.newNo != null) set.add(l.newNo);
80 }
81 validLines.set(path, set);
82 }
83 return validLines.get(path);
84 };
85
86 for (const k of ['number', 'title', 'chapters']) {
87 if (story[k] === undefined) problems.push(`missing required field: ${k}`);
88 }
89 // `number` becomes part of the default output filename, so a value carrying a
90 // path separator would write somewhere the user did not ask for.
91 if (story.number !== undefined && !Number.isInteger(story.number)) {
92 problems.push(`number must be an integer, got ${JSON.stringify(story.number)}`);
93 }
94 if (!Array.isArray(story.chapters) || story.chapters.length === 0) {
95 problems.push('chapters must be a non-empty array');
96 }
97
98 const covered = new Set();
99 (story.chapters || []).forEach((ch, ci) => {
100 const where = `chapters[${ci}]`;
101 if (!ch.title) problems.push(`${where}: missing title`);
102 if (!Array.isArray(ch.beats) || ch.beats.length === 0) {
103 problems.push(`${where}: beats must be a non-empty array`);
104 return;
105 }
106 ch.beats.forEach((b, bi) => {
107 const w = `${where}.beats[${bi}]`;
108 if (!b.text) problems.push(`${w}: missing text`);
109 // An unrecognized kind used to fall back to "why", relabelling the
110 // author's trade-off as a justification — inventing intent is worse than
111 // refusing to render.
112 for (const n of b.notes || []) {
113 if (n && n.kind !== undefined && !NOTE_KINDS.has(n.kind)) {
114 problems.push(`${w}: note kind "${n.kind}" — expected why, tradeoff or rejected`);
115 }
116 }
117 for (const spec of b.files || []) {
118 const { path, from, to, malformed } = parseRange(spec);
119 if (malformed && !byPath.has(path)) {
120 problems.push(`${w}: anchor "${spec}" — malformed range (expected path:N or path:N-M)`);
121 continue;
122 }
123 if (!byPath.has(path)) {
124 problems.push(`${w}: anchor "${spec}" — no such file in the diff`);
125 continue;
126 }
127 covered.add(path);
128 if (from == null) continue;
129 // Both are authoring mistakes with no valid reading, and both would
130 // otherwise make every `for (n = from; n <= to)` loop run zero times —
131 // an empty highlight that never trips the "matches no line" warning.
132 if (from < 1) {
133 problems.push(`${w}: anchor "${spec}" — line numbers start at 1`);
134 continue;
135 }
136 if (to < from) {
137 problems.push(`${w}: anchor "${spec}" — range runs backwards`);
138 continue;
139 }
140 // Anchors point at NEW-side lines. A range that hits nothing means the
141 // model guessed line numbers instead of reading them off the diff —
142 // the highlight would silently render nothing.
143 const valid = linesOf(path);
144 let hit = 0;
145 for (let n = from; n <= to; n++) if (valid.has(n)) hit++;
146 if (hit === 0 && valid.size > 0) {
147 warnings.push(`${w}: anchor "${spec}" matches no line in the new file`);
148 }
149 }
150 });
151 });
152
153 const uncovered = [...byPath.keys()].filter((p) => !covered.has(p));
154 if (problems.length) {
155 console.error('story validation failed:');
156 for (const p of problems) console.error(` - ${p}`);
157 process.exit(1);
158 }
159 for (const w of warnings) console.error(`warning: ${w}`);
160 return { uncovered };
161}
162
⋯ 68 lines not shown
Showing the passage this section is about — read the full file on GitHub.

The second check reads the appendix as evidence. Files no beat anchored land there — right for structure.sql and one-line includes, wrong for a new 50-line helper, which the page would then present as carrying no decision worth explaining.

build.mjsnew+230 −0
⋯ 197 lines not shown
198const html = render(story, files, { lang: opts.lang });
199const out = opts.out || `guided-review-${story.number}.html`;
200try {
201 writeFileSync(out, html);
202} catch (e) {
203 // The skill drives this from a mktemp directory; when that is gone the raw
204 // ENOENT stack trace is the least useful thing to hand back.
205 fail(`cannot write ${out}: ${e.message}`);
206}
207
208const beats = story.chapters.reduce((n, c) => n + (c.beats || []).length, 0);
209console.log(`✓ ${out}`);
210console.log(` ${story.chapters.length} chapters · ${beats} beats · ${files.length} files · ${(html.length / 1024).toFixed(0)} KB`);
211if (uncovered.length) {
212 console.log(` ${uncovered.length} file(s) in the appendix: ${uncovered.slice(0, 5).map((p) => basename(p)).join(', ')}${uncovered.length > 5 ? '…' : ''}`);
213
214 // The appendix is for files that carry no decision: generated output, lockfiles,
215 // one-line includes. Anything substantial landing there means the narrative
216 // skipped it — which reads to the reviewer as "this file didn't matter".
217 const substantial = uncovered
218 .map((p) => files.find((f) => f.path === p))
219 .filter((f) => f && !f.isGenerated && f.additions + f.deletions >= 20);
220 if (substantial.length) {
221 console.error(
222 `\nwarning: ${substantial.length} substantial file(s) have no place in the narrative.\n` +
223 'The appendix is meant for generated and mechanical changes. Either give these a beat,\n' +
224 'or accept that the review presents them as carrying no decision:'
225 );
226 for (const f of substantial) {
227 console.error(` - ${f.path} (+${f.additions} −${f.deletions})`);
228 }
229 }
230}
Showing the passage this section is about — read the full file on GitHub.

A note whose kind is not one of the three recognized values is a hard failure.

build.mjsnew+230 −0
⋯ 45 lines not shown
46 if (a === '--help' || a === '-h') { console.log(USAGE); process.exit(0); }
47 if (!a.startsWith('--')) { positional.push(a); continue; }
48 const key = a.slice(2);
49 if (key !== 'out' && key !== 'lang') fail(`unknown flag --${key}`);
50 const next = argv[i + 1];
51 if (next === undefined || next.startsWith('--')) fail(`--${key} needs a value`);
52 opts[key] = next;
53 i++;
54 }
55 return { positional, opts };
56}
57
58const NOTE_KINDS = new Set(['why', 'tradeoff', 'rejected']);
59
60// Validate the story up front. A malformed field here would otherwise surface
61// as a silently empty chapter in the output, which is worse than an error.
62//
63// Problems abort the run; warnings print and rendering continues. The split is
64// deliberate: a wrong anchor still produces a usable page, a wrong structure
65// does not. Every problem is collected before exiting so an author fixing a
66// story sees the whole list, not one error per run.
67//
68// Returns the paths no beat anchored.
69function validate(story, files) {
70 const byPath = new Map(files.map((f) => [f.path, f]));
⋯ 23 lines not shown
94 if (!Array.isArray(story.chapters) || story.chapters.length === 0) {
95 problems.push('chapters must be a non-empty array');
96 }
97
98 const covered = new Set();
99 (story.chapters || []).forEach((ch, ci) => {
100 const where = `chapters[${ci}]`;
101 if (!ch.title) problems.push(`${where}: missing title`);
102 if (!Array.isArray(ch.beats) || ch.beats.length === 0) {
103 problems.push(`${where}: beats must be a non-empty array`);
104 return;
105 }
106 ch.beats.forEach((b, bi) => {
107 const w = `${where}.beats[${bi}]`;
108 if (!b.text) problems.push(`${w}: missing text`);
109 // An unrecognized kind used to fall back to "why", relabelling the
110 // author's trade-off as a justification — inventing intent is worse than
111 // refusing to render.
112 for (const n of b.notes || []) {
113 if (n && n.kind !== undefined && !NOTE_KINDS.has(n.kind)) {
114 problems.push(`${w}: note kind "${n.kind}" — expected why, tradeoff or rejected`);
115 }
116 }
117 for (const spec of b.files || []) {
118 const { path, from, to, malformed } = parseRange(spec);
119 if (malformed && !byPath.has(path)) {
120 problems.push(`${w}: anchor "${spec}" — malformed range (expected path:N or path:N-M)`);
121 continue;
122 }
123 if (!byPath.has(path)) {
124 problems.push(`${w}: anchor "${spec}" — no such file in the diff`);
125 continue;
126 }
127 covered.add(path);
128 if (from == null) continue;
129 // Both are authoring mistakes with no valid reading, and both would
130 // otherwise make every `for (n = from; n <= to)` loop run zero times —
131 // an empty highlight that never trips the "matches no line" warning.
132 if (from < 1) {
⋯ 98 lines not shown
Showing the passage this section is about — read the full file on GitHub.
Chapter 05

Designing for reading

A technical essay, not a dashboard — and the details that decide which one you get.

Three typefaces for three voices: serif for the prose, sans for the interface, mono for code. The author's voice, the tool's, and the machine's — kept physically distinct so the eye never has to work out which it is reading.

No web fonts: the page has to open offline and from a file:// URL.

lib/theme.mjsnew+670 −0
@@ -0,0 +1,670 @@
1// The stylesheet, as a single exported string.
2//
3// Design direction: a technical essay, not a dashboard. The prose is the
4// subject and the diff is the illustration beside it — so the narrative gets a
5// serif at reading size and generous measure, while the code stays visually
6// quiet until a beat points at it.
7//
8// Deliberate choices worth keeping:
9// - Serif prose / sans chrome / mono code. Three voices, three roles: the
10// author's, the tool's, the machine's. Never blur them.
11// - Diff colors are desaturated. Full-strength red/green on every changed line
12// turns the page into a Christmas tree and drowns the anchor highlight, which
13// is the one color that must cut through.
14// - No web fonts. The page must open offline and from a file:// URL, so the
15// stacks below are curated system faces, not a downloaded family.
16// - light-dark() with a [data-theme] override: follows the OS by default, and
17// the toggle wins in both directions.
18
19// Where a chapter comes to rest below the sticky masthead, in px.
20//
21// Shared with the page script deliberately: the scrollspy treats a chapter as
22// active once its top crosses this same line. When the two drifted apart, j/k
23// oscillated between two chapters forever — the destination landed just outside
24// the threshold that decides which chapter you are in.
25export const REST = 76;
26
27export const CSS = String.raw`
28:root {
29 color-scheme: light dark;
30
31 /* Neutrals are tinted — warm parchment in light, cool slate in dark. A pure
32 gray next to the amber accent reads as dirty. */
33 --bg: light-dark(oklch(98.6% 0.006 85), oklch(19% 0.014 255));
34 --bg-sunk: light-dark(oklch(96.2% 0.009 85), oklch(16% 0.014 255));
35 --bg-raised: light-dark(oklch(100% 0 0), oklch(23% 0.015 255));
36 --bg-hover: light-dark(oklch(94.5% 0.012 85), oklch(27% 0.017 255));
37 --line: light-dark(oklch(89% 0.012 85), oklch(30% 0.016 255));
38 --line-soft: light-dark(oklch(93.5% 0.010 85), oklch(25.5% 0.015 255));
39
40 --ink: light-dark(oklch(24% 0.020 75), oklch(92% 0.012 250));
41 --ink-2: light-dark(oklch(44% 0.018 75), oklch(74% 0.012 250));
42 --ink-3: light-dark(oklch(58% 0.016 75), oklch(58% 0.012 250));
43
44 /* One accent, used sparingly: chapter numbers, progress, active nav. */
45 --accent: light-dark(oklch(52% 0.145 52), oklch(76% 0.135 62));
46 --accent-bg: light-dark(oklch(94% 0.045 62), oklch(30% 0.055 55));
47
48 --add: light-dark(oklch(45% 0.095 150), oklch(78% 0.105 155));
49 --add-bg: light-dark(oklch(95.5% 0.035 155), oklch(26% 0.045 158));
50 --add-gut: light-dark(oklch(88% 0.055 155), oklch(34% 0.055 158));
51 --del: light-dark(oklch(48% 0.115 22), oklch(76% 0.105 22));
52 --del-bg: light-dark(oklch(96% 0.028 22), oklch(26% 0.045 18));
53 --del-gut: light-dark(oklch(90% 0.045 22), oklch(34% 0.050 18));
54
55 /* The anchor highlight must beat every other color on the page. */
56 --mark: light-dark(oklch(85% 0.130 88), oklch(62% 0.115 88));
57 --mark-bg: light-dark(oklch(96% 0.070 92), oklch(32% 0.060 90));
58
59 --serif: "Iowan Old Style", "Palatino Linotype", Palatino, Charter, "Source Serif 4", Georgia, serif;
60 --sans: ui-sans-serif, -apple-system, BlinkMacSystemFont, "Segoe UI Variable", "Segoe UI", Helvetica, sans-serif;
61 --mono: ui-monospace, "SF Mono", "JetBrains Mono", "IBM Plex Mono", Menlo, Consolas, monospace;
62
63 /* Vertical rhythm: everything is a multiple of the prose line box (28px). */
64 --u: 0.5rem;
65 --rail: 19rem;
66 --radius: 10px;
67 --ease: cubic-bezier(0.22, 0.9, 0.28, 1);
68}
69:root[data-theme="light"] { color-scheme: light; }
70:root[data-theme="dark"] { color-scheme: dark; }
71
72* { box-sizing: border-box; }
⋯ 598 lines not shown
Showing the passage this section is about — read the full file on GitHub.

Diff colours are deliberately muted, and the anchor highlight carries its weight in the gutter rail rather than the fill.

lib/theme.mjsnew+670 −0
⋯ 27 lines not shown
28:root {
29 color-scheme: light dark;
30
31 /* Neutrals are tinted — warm parchment in light, cool slate in dark. A pure
32 gray next to the amber accent reads as dirty. */
33 --bg: light-dark(oklch(98.6% 0.006 85), oklch(19% 0.014 255));
34 --bg-sunk: light-dark(oklch(96.2% 0.009 85), oklch(16% 0.014 255));
35 --bg-raised: light-dark(oklch(100% 0 0), oklch(23% 0.015 255));
36 --bg-hover: light-dark(oklch(94.5% 0.012 85), oklch(27% 0.017 255));
37 --line: light-dark(oklch(89% 0.012 85), oklch(30% 0.016 255));
38 --line-soft: light-dark(oklch(93.5% 0.010 85), oklch(25.5% 0.015 255));
39
40 --ink: light-dark(oklch(24% 0.020 75), oklch(92% 0.012 250));
41 --ink-2: light-dark(oklch(44% 0.018 75), oklch(74% 0.012 250));
42 --ink-3: light-dark(oklch(58% 0.016 75), oklch(58% 0.012 250));
43
44 /* One accent, used sparingly: chapter numbers, progress, active nav. */
45 --accent: light-dark(oklch(52% 0.145 52), oklch(76% 0.135 62));
46 --accent-bg: light-dark(oklch(94% 0.045 62), oklch(30% 0.055 55));
47
48 --add: light-dark(oklch(45% 0.095 150), oklch(78% 0.105 155));
49 --add-bg: light-dark(oklch(95.5% 0.035 155), oklch(26% 0.045 158));
50 --add-gut: light-dark(oklch(88% 0.055 155), oklch(34% 0.055 158));
51 --del: light-dark(oklch(48% 0.115 22), oklch(76% 0.105 22));
52 --del-bg: light-dark(oklch(96% 0.028 22), oklch(26% 0.045 18));
53 --del-gut: light-dark(oklch(90% 0.045 22), oklch(34% 0.050 18));
54
55 /* The anchor highlight must beat every other color on the page. */
56 --mark: light-dark(oklch(85% 0.130 88), oklch(62% 0.115 88));
57 --mark-bg: light-dark(oklch(96% 0.070 92), oklch(32% 0.060 90));
58
59 --serif: "Iowan Old Style", "Palatino Linotype", Palatino, Charter, "Source Serif 4", Georgia, serif;
60 --sans: ui-sans-serif, -apple-system, BlinkMacSystemFont, "Segoe UI Variable", "Segoe UI", Helvetica, sans-serif;
61 --mono: ui-monospace, "SF Mono", "JetBrains Mono", "IBM Plex Mono", Menlo, Consolas, monospace;
62
63 /* Vertical rhythm: everything is a multiple of the prose line box (28px). */
64 --u: 0.5rem;
65 --rail: 19rem;
66 --radius: 10px;
67 --ease: cubic-bezier(0.22, 0.9, 0.28, 1);
68}
⋯ 602 lines not shown
Showing the passage this section is about — read the full file on GitHub.

Breakpoints are container queries. The beat responds to the column it occupies, not to the window.

lib/theme.mjsnew+670 −0
⋯ 242 lines not shown
243 font-variant-numeric: tabular-nums;
244}
245.rail .n {
246 margin-right: calc(var(--u) * 0.875);
247 font-family: var(--mono);
248 font-size: 0.6875rem;
249 color: var(--ink-3);
250 font-variant-numeric: tabular-nums;
251}
252.rail a.on .n { color: var(--accent); }
253
254/* ─────────────────────────── Reading column ─────────────────────────── */
255
256/* The reading column is the query container: the beat layout must react to the
257 space it actually has, not to the viewport. With the rail taking ~19rem, a
258 viewport-based breakpoint keeps a two-column beat long after the code panel
259 has been squeezed to an unreadable sliver. */
260.read {
261 container: read / inline-size;
262 padding: calc(var(--u) * 5) calc(var(--u) * 5) 40vh;
263 min-width: 0;
264}
265
266.lede {
267 max-width: 62ch;
268 margin: 0 0 calc(var(--u) * 8);
269 padding-bottom: calc(var(--u) * 5);
270 border-bottom: 1px solid var(--line);
271}
272.lede .kicker {
273 margin: 0 0 calc(var(--u) * 1.5);
274 font-size: 0.6875rem;
275 font-weight: 600;
276 letter-spacing: 0.09em;
277 text-transform: uppercase;
⋯ 65 lines not shown
343 describes, which is the whole point of the pairing. The trigger is the
344 reading column, not the viewport; see the container query below. */
345.beat {
346 display: grid;
347 grid-template-columns: minmax(0, 27rem) minmax(0, 1fr);
348 gap: calc(var(--u) * 4);
349 align-items: start;
350 margin-bottom: calc(var(--u) * 5);
351}
352/* Sticky needs room to travel: when the prose is the taller half there is
353 nothing to scroll past, and a sticky card just floats out of line with the
354 panel it belongs to. Grid tracks report their own height, so the pairing
355 stays aligned when the code is short. */
356.beat-code { min-width: 0; }
357/* Below ~54rem of column there isn't room for prose AND a code panel wide
358 enough to read: stack them, prose first, so each paragraph still sits
359 directly above the code it describes. */
360@container read (max-width: 54rem) {
361 .beat { grid-template-columns: minmax(0, 1fr); gap: calc(var(--u) * 2.5); }
362 /* Stacked, the prose sits directly above its code — there is no second
363 column travelling behind it, so sticky would only detach it from the panel
364 it introduces. */
365 .beat-prose { position: static; z-index: auto; max-width: 62ch; }
366}
367
368/* The prose column carries its own surface. It is sticky, so as the reader
369 scrolls the code panel travels behind it — on a transparent background the
370 two sets of text overlap and neither is readable. The z-index keeps it above
371 that panel for the same reason. */
372.beat-prose {
373 position: sticky;
374 top: ${REST}px;
375 z-index: 1;
376 padding: calc(var(--u) * 2.5) calc(var(--u) * 3) calc(var(--u) * 3);
377 border-radius: var(--radius);
378 background: var(--bg-raised);
379 border: 1px solid var(--line-soft);
380 font-family: var(--serif);
381 font-size: 1.0625rem;
382 line-height: 1.68;
⋯ 288 lines not shown
Showing the passage this section is about — read the full file on GitHub.

REST — where a chapter comes to rest below the sticky masthead — is one exported constant, interpolated into both the stylesheet and the page script.

lib/theme.mjsnew+670 −0
⋯ 6 lines not shown
7//
8// Deliberate choices worth keeping:
9// - Serif prose / sans chrome / mono code. Three voices, three roles: the
10// author's, the tool's, the machine's. Never blur them.
11// - Diff colors are desaturated. Full-strength red/green on every changed line
12// turns the page into a Christmas tree and drowns the anchor highlight, which
13// is the one color that must cut through.
14// - No web fonts. The page must open offline and from a file:// URL, so the
15// stacks below are curated system faces, not a downloaded family.
16// - light-dark() with a [data-theme] override: follows the OS by default, and
17// the toggle wins in both directions.
18
19// Where a chapter comes to rest below the sticky masthead, in px.
20//
21// Shared with the page script deliberately: the scrollspy treats a chapter as
22// active once its top crosses this same line. When the two drifted apart, j/k
23// oscillated between two chapters forever — the destination landed just outside
24// the threshold that decides which chapter you are in.
25export const REST = 76;
26
27export const CSS = String.raw`
28:root {
29 color-scheme: light dark;
30
31 /* Neutrals are tinted — warm parchment in light, cool slate in dark. A pure
32 gray next to the amber accent reads as dirty. */
33 --bg: light-dark(oklch(98.6% 0.006 85), oklch(19% 0.014 255));
34 --bg-sunk: light-dark(oklch(96.2% 0.009 85), oklch(16% 0.014 255));
35 --bg-raised: light-dark(oklch(100% 0 0), oklch(23% 0.015 255));
36 --bg-hover: light-dark(oklch(94.5% 0.012 85), oklch(27% 0.017 255));
37 --line: light-dark(oklch(89% 0.012 85), oklch(30% 0.016 255));
⋯ 633 lines not shown
Showing the passage this section is about — read the full file on GitHub.
lib/render.mjsnew+509 −0
⋯ 292 lines not shown
293 });
294
295 var chapters = [].slice.call(document.querySelectorAll('.ch'));
296 var links = [].slice.call(document.querySelectorAll('.rail a'));
297 var bar = document.getElementById('progress');
298 // -1, not 0: the first onScroll() must be able to paint chapter 0 as active.
299 // Starting at 0 makes that call a no-op and the rail loads with nothing marked.
300 var current = -1;
301
302 function setCurrent(i) {
303 if (i === current) return;
304 current = i;
305 links.forEach(function (a, n) { a.classList.toggle('on', n === i); });
306 }
307
308 // Where a chapter comes to rest below the masthead. Interpolated from the
309 // REST constant in theme.mjs, which the stylesheet uses for the matching
310 // scroll-margin — the two must not drift apart.
311 var REST = ${REST};
312
313 // Active chapter = the last one whose top has crossed the masthead. Chapters
314 // are taller than the viewport, so IntersectionObserver would flip between
315 // neighbours mid-scroll; a position read is stable.
316 //
317 // Measured against the viewport, not offsetTop: the chapters sit inside a
318 // positioned column, so offsetTop is relative to that column while scrollY is
319 // absolute — comparing them is off by the column's own offset.
320 function onScroll() {
321 var i = 0;
322 for (var n = 0; n < chapters.length; n++) {
323 // +1 absorbs the sub-pixel rounding of a smooth scroll landing.
324 if (chapters[n].getBoundingClientRect().top <= REST + 1) i = n; else break;
325 }
326 setCurrent(i);
327 var max = document.body.scrollHeight - innerHeight;
328 bar.style.width = (max > 0 ? Math.min(100, (scrollY / max) * 100) : 0) + '%';
329 }
330 addEventListener('scroll', onScroll, { passive: true });
331 addEventListener('resize', onScroll, { passive: true });
332 onScroll();
333
334 // Scroll to an absolute position rather than scrollIntoView: the target is
335 // computed from the viewport rect, so it lands under the masthead regardless
336 // of the element's offset parent, and it honours prefers-reduced-motion.
337 var SMOOTH = !matchMedia('(prefers-reduced-motion: reduce)').matches;
338
339 function go(i) {
340 if (i < 0 || i >= chapters.length) return;
341 var top = chapters[i].getBoundingClientRect().top + window.scrollY - REST;
342 window.scrollTo({ top: Math.max(0, top), behavior: SMOOTH ? 'smooth' : 'auto' });
343 // Claim the destination immediately. A smooth scroll fires many scroll
344 // events on the way, and reading "current" off them mid-flight makes the
345 // next keypress compute its step from wherever the animation happens to be.
346 setCurrent(i);
347 }
348
349 // One implementation for both the E key and the button: two copies of this
350 // drift apart the moment either is touched.
351 function toggleAll() {
352 var all = document.querySelectorAll('.file');
353 var anyClosed = [].some.call(all, function (d) { return !d.open; });
354 [].forEach.call(all, function (d) { d.open = anyClosed; });
355 }
356
357 addEventListener('keydown', function (e) {
⋯ 152 lines not shown
Showing the passage this section is about — read the full file on GitHub.
Chapter 06

What holds it together

The tests, and the reason they are the shape they are.

The parser tests assert line numbers against the hunk header's own declared counts — git's ground truth — across added, deleted, renamed and binary files, multiple hunks, and content that looks like diff metadata.

test/diff.test.mjsnew+179 −0
@@ -0,0 +1,179 @@
1// Line-number tracking is the load-bearing contract of this tool: a narrative
2// anchored to the wrong lines points the reviewer at code the paragraph is not
3// about, and nothing crashes. These tests pin it.
4
5import { test } from 'node:test';
6import assert from 'node:assert/strict';
7import { parseDiff, parseRange, isGenerated } from '../lib/diff.mjs';
8
9// Every `git diff` / `gh pr diff` ends in a newline. Splitting on '\n' leaves a
10// trailing '' that is not a line of the file — consumed as context, it invented
11// a row and registered a line number that does not exist as a valid anchor.
12test('a trailing newline does not fabricate a line', () => {
13 const diff = [
14 'diff --git a/x.rb b/x.rb',
15 'new file mode 100644',
16 '--- /dev/null',
17 '+++ b/x.rb',
18 '@@ -0,0 +1,2 @@',
19 '+a',
20 '+b',
21 '',
22 ].join('\n');
23
24 const [file] = parseDiff(diff);
25 assert.equal(file.hunks[0].lines.length, 2);
26 assert.deepEqual(file.hunks[0].lines.map((l) => l.newNo), [1, 2]);
27 assert.equal(file.additions, 2);
28});
29
30test('line numbers match the hunk header on both sides', () => {
31 const diff = [
32 'diff --git a/x.rb b/x.rb',
33 '--- a/x.rb',
34 '+++ b/x.rb',
35 '@@ -10,3 +10,4 @@ def existing',
36 ' ctx',
37 '-gone',
38 '+kept',
39 '+added',
40 ' tail',
41 '',
42 ].join('\n');
43
44 const [file] = parseDiff(diff);
45 const lines = file.hunks[0].lines;
46 assert.deepEqual(lines.map((l) => [l.type, l.oldNo, l.newNo]), [
47 ['ctx', 10, 10],
48 ['del', 11, null],
49 ['add', null, 11],
50 ['add', null, 12],
51 ['ctx', 12, 13],
52 ]);
53 assert.equal(file.additions, 2);
54 assert.equal(file.deletions, 1);
55});
56
57test('a deleted file has no new-side line numbers', () => {
58 const diff = [
59 'diff --git a/old.rb b/old.rb',
60 'deleted file mode 100644',
61 '--- a/old.rb',
62 '+++ /dev/null',
63 '@@ -1,2 +0,0 @@',
64 '-class Old',
65 '-end',
66 '',
67 ].join('\n');
68
69 const [file] = parseDiff(diff);
70 assert.equal(file.status, 'deleted');
71 // newNo 0 is not a line number; every consumer tests `!= null`, so a zero
72 // would pass as a real line and mint a dead deep-link target.
73 assert.ok(file.hunks[0].lines.every((l) => l.newNo === null));
74});
75
76// `--- ` and `+++ ` are file metadata only before the first hunk. Inside one,
77// a deleted line reading `--- foo` is content, and skipping it drops real code.
78test('content that looks like diff metadata survives', () => {
79 const diff = [
80 'diff --git a/CHANGELOG.md b/CHANGELOG.md',
81 '--- a/CHANGELOG.md',
82 '+++ b/CHANGELOG.md',
83 '@@ -1,2 +1,2 @@',
84 '---- old heading',
85 '+--- new heading',
86 ' body',
87 '',
88 ].join('\n');
89
90 const [file] = parseDiff(diff);
91 const lines = file.hunks[0].lines;
92 assert.equal(lines.length, 3);
93 assert.equal(lines[0].content, '--- old heading');
94 assert.equal(lines[1].content, '--- new heading');
95});
96
97test('binary, renamed and added files are recognised', () => {
98 const diff = [
99 'diff --git a/logo.png b/logo.png',
100 'new file mode 100644',
101 'Binary files /dev/null and b/logo.png differ',
102 'diff --git a/a.rb b/b.rb',
103 'similarity index 90%',
104 'rename from a.rb',
105 'rename to b.rb',
106 '@@ -1,1 +1,1 @@',
107 '-old',
108 '+new',
109 '',
110 ].join('\n');
111
112 const files = parseDiff(diff);
113 assert.equal(files.length, 2);
114 assert.equal(files[0].status, 'added');
115 assert.equal(files[0].isBinary, true);
116 assert.equal(files[1].status, 'renamed');
117 assert.equal(files[1].oldPath, 'a.rb');
118 assert.equal(files[1].path, 'b.rb');
119});
120
121test('"\\ No newline at end of file" occupies no line number', () => {
122 const diff = [
123 'diff --git a/x.txt b/x.txt',
124 '--- a/x.txt',
125 '+++ b/x.txt',
126 '@@ -1 +1 @@',
127 '-a',
128 '\\ No newline at end of file',
129 '+b',
130 '\\ No newline at end of file',
131 '',
132 ].join('\n');
133
134 const [file] = parseDiff(diff);
135 assert.equal(file.hunks[0].lines.length, 2);
136});
137
138test('multiple hunks each restart from their own header', () => {
139 const diff = [
140 'diff --git a/x.rb b/x.rb',
141 '--- a/x.rb',
142 '+++ b/x.rb',
143 '@@ -1,1 +1,1 @@',
144 '+first',
145 '@@ -50,1 +60,1 @@',
146 '+second',
147 '',
148 ].join('\n');
149
150 const [file] = parseDiff(diff);
151 assert.equal(file.hunks.length, 2);
152 assert.equal(file.hunks[0].lines[0].newNo, 1);
153 assert.equal(file.hunks[1].lines[0].newNo, 60);
154});
155
156test('parseRange splits anchors, and flags the malformed ones', () => {
157 assert.deepEqual(parseRange('app/foo.rb'), { path: 'app/foo.rb', from: null, to: null });
158 assert.deepEqual(parseRange('app/foo.rb:42'), { path: 'app/foo.rb', from: 42, to: 42 });
159 assert.deepEqual(parseRange('app/foo.rb:17-42'), { path: 'app/foo.rb', from: 17, to: 42 });
160
161 // GitHub's own fragment syntax is an easy thing to write by mistake. Reported
162 // as malformed rather than as a missing file, which names the wrong cause.
163 const bad = parseRange('app/foo.rb:L17-L42');
164 assert.equal(bad.malformed, true);
165 assert.equal(bad.from, null);
166
167 // A reversed range is kept as written; validate() rejects it. Silently
168 // reordering would hide an authoring mistake behind a plausible highlight.
169 assert.deepEqual(parseRange('app/foo.rb:42-17'), { path: 'app/foo.rb', from: 42, to: 17 });
170});
171
172test('isGenerated matches machine-written paths only', () => {
173 assert.ok(isGenerated('db/structure.sql'));
174 assert.ok(isGenerated('package-lock.json'));
175 assert.ok(isGenerated('app/assets/x.min.js'));
176 assert.ok(isGenerated('vendor/gems/thing.rb'));
177 assert.ok(!isGenerated('app/models/user.rb'));
178 assert.ok(!isGenerated('lib/distribution.js'));
179});

The highlighter tests assert that no language emits an empty class, and that escaping holds for every language including the unknown-extension fallthrough.

test/highlight.test.mjsnew+73 −0
@@ -0,0 +1,73 @@
1// The class-mapping bug these pin was invisible by inspection: JSON and YAML
2// numbers rendered with class="" — unhighlighted, but not obviously broken —
3// because a rule containing its own capture group shifted every index after it.
4
5import { test } from 'node:test';
6import assert from 'node:assert/strict';
7import { highlight, escapeHtml, languageOf } from '../lib/highlight.mjs';
8
9const classesIn = (html) => [...html.matchAll(/<i class="([^"]*)">/g)].map((m) => m[1]);
10
11test('no language emits an empty class', () => {
12 const samples = {
13 ruby: 'def x; return 42; end',
14 js: 'const a = 42;',
15 python: 'def f(): return 42',
16 sql: 'SELECT 42 FROM t',
17 css: '.a { width: 42px; }',
18 go: 'func f() int { return 42 }',
19 rust: 'fn f() -> i32 { 42 }',
20 php: 'function f() { return 42; }',
21 java: 'public int f() { return 42; }',
22 shell: 'if true; then echo 42; fi',
23 json: '{"k": 12, "ok": true}',
24 yaml: 'key: 5\nflag: true',
25 html: '<b class="x">hi</b>',
26 markdown: '# Title with `code`',
27 };
28 for (const [lang, src] of Object.entries(samples)) {
29 const classes = classesIn(highlight(src, lang));
30 assert.ok(classes.length > 0, `${lang}: nothing highlighted`);
31 assert.ok(!classes.includes(''), `${lang}: emitted an empty class`);
32 }
33});
34
35// Both languages declare a rule with an inner group before the number rule.
36test('numbers are classed in JSON and YAML', () => {
37 assert.match(highlight('{"k": 12}', 'json'), /<i class="n">12<\/i>/);
38 assert.match(highlight('a: 5', 'yaml'), /<i class="n">5<\/i>/);
39});
40
41test('escaping holds for every language, including the unknown fallthrough', () => {
42 const hostile = '"<script>alert(1)</script>" & \'x\'';
43 for (const lang of ['ruby', 'js', 'json', 'yaml', 'html', 'sql', null]) {
44 const out = highlight(hostile, lang);
45 assert.ok(!/<script>/.test(out), `${lang}: raw <script> survived`);
46 assert.ok(!out.includes('alert(1)</script>'), `${lang}: tag reconstructed`);
47 }
48});
49
50test('escapeHtml covers every character that can break an attribute', () => {
51 assert.equal(escapeHtml(`&<>"'`), '&amp;&lt;&gt;&quot;&#39;');
52});
53
54test('a keyword inside a string is not highlighted as a keyword', () => {
55 // Rule order is precedence: strings must win over keywords.
56 const out = highlight('const a = "return false";', 'js');
57 assert.match(out, /<i class="s">&quot;return false&quot;<\/i>/);
58});
59
60test('very long lines skip tokenization but stay escaped', () => {
61 const line = '"'.repeat(3000) + '<b>';
62 const out = highlight(line, 'js');
63 assert.ok(!out.includes('<b>'));
64 assert.ok(!out.includes('<i class='));
65});
66
67test('languageOf reads the last meaningful extension', () => {
68 assert.equal(languageOf('app/views/_bar.html.erb'), 'html');
69 assert.equal(languageOf('app/models/user.rb'), 'ruby');
70 assert.equal(languageOf('Gemfile'), 'ruby');
71 assert.equal(languageOf('Dockerfile'), 'shell');
72 assert.equal(languageOf('README'), null);
73});

The renderer tests pin the failures that produce a page which *looks fine*: hidden lines with no marker, crossed tags, duplicate ids, a live javascript: href.

The accounting test is the important one — every hidden line plus every shown line must equal the file.

test/render.test.mjsnew+168 −0
@@ -0,0 +1,168 @@
1// The failures pinned here all produced a page that looked fine: hidden lines
2// with no marker, crossed tags, duplicate ids, a live javascript: href. For a
3// tool whose value is "this paragraph is about exactly these lines", output
4// that is confidently wrong is the most expensive failure mode there is.
5
6import { test } from 'node:test';
7import assert from 'node:assert/strict';
8import { parseDiff } from '../lib/diff.mjs';
9import { render } from '../lib/render.mjs';
10
11function addedFile(path, count) {
12 const lines = [];
13 for (let i = 1; i <= count; i++) lines.push(`+line ${i}`);
14 return [
15 `diff --git a/${path} b/${path}`,
16 'new file mode 100644',
17 '--- /dev/null',
18 `+++ b/${path}`,
19 `@@ -0,0 +1,${count} @@`,
20 ...lines,
21 '',
22 ].join('\n');
23}
24
25const story = (chapters, extra = {}) => ({
26 number: 1,
27 title: 'T',
28 url: 'https://example.com/pr/1',
29 chapters,
30 ...extra,
31});
32
33// Content rows only: the `hunk` class marks the "N lines not shown" separators.
34const countRows = (html) =>
35 [...html.matchAll(/<tr class="([^"]*)"/g)].filter((m) => !m[1].includes('hunk')).length;
36
37const skipTotals = (html) =>
38 [...html.matchAll(/⋯ ([\d,]+) lines not shown/g)]
39 .reduce((n, m) => n + Number(m[1].replace(/,/g, '')), 0);
40
41// The marker is the one guardrail the focus feature depends on. Lines dropped
42// after the last kept region used to vanish with nothing announcing them, so a
43// reviewer reaching the end of a panel concluded the file ended there.
44test('every hidden line is announced, including the trailing ones', () => {
45 const files = parseDiff(addedFile('big.rb', 200));
46 const html = render(
47 story([{ title: 'C', beats: [{ text: 'x', files: ['big.rb:100'] }] }]),
48 files
49 );
50
51 const shown = countRows(html);
52 assert.ok(shown < 200, 'the file should be focused, not rendered whole');
53 assert.equal(skipTotals(html) + shown, 200);
54});
55
56test('a mark near the start still accounts for the tail', () => {
57 const files = parseDiff(addedFile('big.rb', 120));
58 const html = render(
59 story([{ title: 'C', beats: [{ text: 'x', files: ['big.rb:2'] }] }]),
60 files
61 );
62 assert.equal(skipTotals(html) + countRows(html), 120);
63});
64
65// Three independent replace() passes could not see each other's boundaries.
66test('interleaved code and bold markers do not cross', () => {
67 const html = render(
68 story([{ title: 'C', beats: [{ text: 'see `a**b` and **x**' }] }]),
69 parseDiff(addedFile('x.rb', 3))
70 );
71 assert.match(html, /<code>a\*\*b<\/code> and <strong>x<\/strong>/);
72 assert.ok(!/<code>[^<]*<strong>[^<]*<\/code>/.test(html), 'tags crossed');
73});
74
75test('bare numbers in prose are not swallowed by the code-span placeholder', () => {
76 const html = render(
77 story([{ title: 'C', beats: [{ text: 'about 1 of 10 items, see `x`' }] }]),
78 parseDiff(addedFile('x.rb', 3))
79 );
80 assert.match(html, /about 1 of 10 items, see <code>x<\/code>/);
81});
82
83// The same file renders once per beat that anchors it, so an id built only from
84// path and line was emitted several times — invalid HTML, and getElementById
85// sent every deep link to whichever panel happened to render first.
86test('line ids are unique across beats citing the same file', () => {
87 const files = parseDiff(addedFile('a.rb', 40));
88 const html = render(
89 story([
90 {
91 title: 'C',
92 beats: [
93 { text: 'one', files: ['a.rb:5'] },
94 { text: 'two', files: ['a.rb:6'] },
95 ],
96 },
97 ]),
98 files
99 );
100 const ids = [...html.matchAll(/ id="([^"]+)"/g)].map((m) => m[1]);
101 assert.equal(new Set(ids).size, ids.length, 'duplicate ids emitted');
102});
103
104test('ids stay addressable when the path contains a space', () => {
105 const files = parseDiff(addedFile('a b.rb', 5));
106 const html = render(
107 story([{ title: 'C', beats: [{ text: 'x', files: ['a b.rb:2'] }] }]),
108 files
109 );
110 for (const [, id] of html.matchAll(/ id="(L[^"]*|c\d+b\d+-L[^"]*)"/g)) {
111 assert.ok(!/\s/.test(id), `id contains whitespace: ${id}`);
112 }
113});
114
115// Escaping stops attribute breakout but says nothing about the scheme, and this
116// page is built to be handed to other reviewers.
117test('a javascript: story url never reaches an href', () => {
118 const html = render(
119 story([{ title: 'C', beats: [{ text: 'x' }] }], { url: 'javascript:alert(1)' }),
120 parseDiff(addedFile('x.rb', 3))
121 );
122 assert.ok(!/href="javascript:/i.test(html));
123 assert.match(html, /href="#"/);
124});
125
126test('an ordinary https url survives intact', () => {
127 const html = render(
128 story([{ title: 'C', beats: [{ text: 'x' }] }]),
129 parseDiff(addedFile('x.rb', 3))
130 );
131 assert.match(html, /href="https:\/\/example\.com\/pr\/1"/);
132});
133
134test('markup in narrative prose renders as text, not HTML', () => {
135 const html = render(
136 story([{ title: 'C', beats: [{ text: 'an <img src=x onerror=alert(1)> tag' }] }]),
137 parseDiff(addedFile('x.rb', 3))
138 );
139 assert.ok(!/<img/.test(html));
140 assert.match(html, /&lt;img src=x/);
141});
142
143test('files no beat anchors land in the appendix', () => {
144 const files = parseDiff(addedFile('used.rb', 5) + addedFile('spare.rb', 5));
145 const html = render(
146 story([{ title: 'C', beats: [{ text: 'x', files: ['used.rb:1'] }] }]),
147 files
148 );
149 assert.match(html, /class="appendix"/);
150 assert.match(html, /spare\.rb/);
151});
152
153test('the pt locale switches the chrome, not the narrative', () => {
154 const html = render(
155 story([{ title: 'Capítulo um', beats: [{ text: 'texto' }] }]),
156 parseDiff(addedFile('x.rb', 3)),
157 { lang: 'pt' }
158 );
159 assert.match(html, /lang="pt-BR"/);
160 assert.match(html, /Capítulo 01/);
161});
162
163test('render does not mutate the parsed diff it is given', () => {
164 const files = parseDiff(addedFile('x.rb', 3));
165 const before = JSON.stringify(files);
166 render(story([{ title: 'C', beats: [{ text: 'x', files: ['x.rb:1'] }] }]), files);
167 assert.equal(JSON.stringify(files), before);
168});

node:test, no dependencies, and a package.json that exists only to name the runner.

package.jsonnew+13 −0
@@ -0,0 +1,13 @@
1{
2 "name": "guided-review",
3 "version": "1.0.0",
4 "description": "Turn a pull request into a guided, narrated code review.",
5 "type": "module",
6 "license": "MIT",
7 "scripts": {
8 "test": "node --test \"test/*.test.mjs\""
9 },
10 "engines": {
11 "node": ">=18"
12 }
13}

The README carries the argument the code cannot make on its own: that reviewing got harder because writing got cheaper, that the bottleneck moved from producing code to understanding it, and that the fix is not asking reviewers to try harder.

It is also where the honest limits go — what this is not, which is a code review.

README.mdnew+251 −0
@@ -0,0 +1,251 @@
1# guided-review
2
3**Make code review great again.**
4
5Turn a pull request into a guided, narrated code review — one self-contained HTML page where
6the PR is told as a story, in the order the decisions were made, with each paragraph sitting
7beside the exact diff lines it explains.
8
9![The pairing: prose on the left, the lines it describes highlighted on the right](docs/pairing-light.jpg)
10
11---
12
13## Let's be honest about code review
14
15Nobody's favourite part of the week.
16
17It used to be survivable. A couple of PRs, most of them small, written by someone whose
18reasoning you could reconstruct — or just ask about. Then something changed: **writing code got
19dramatically cheaper, and reading it didn't.**
20
21The bottleneck moved. It's no longer "how fast can we ship this" — it's "who has three
22uninterrupted hours to understand what this actually does." And here's the uncomfortable part:
23the code is often *fine*. Well structured, tested, idiomatic. That's not the problem. The
24problem is **volume against a fixed budget of human attention**.
25
26So what actually happens, in every team, every week:
27
28- You open a 40-file PR at 4pm.
29- GitHub greets you with `.eslintrc` and `.gitignore`, because it sorts alphabetically and has
30 no idea which file carries the idea.
31- The description explains the reasoning beautifully — in one place, 800 words, scrolled far
32 away from any of the code it describes.
33- You read the prose. You read the diff. You try to hold the mapping between them in your head.
34- Somewhere around file 18, the honest part of your brain goes quiet and you start **scrolling
35 faster**.
36- ✅ LGTM.
37
38That last step is the expensive one. Not because reviewers are lazy — because **reconstructing
39intent from a diff is genuinely hard work**, and we ask people to redo it from scratch for every
40PR, after the author already did it and wrote it down somewhere else.
41
42Skimming doesn't miss the bugs a linter catches. It misses the ones that need someone awake
43enough to ask *"wait, why is it done this way?"*
44
45### The bet
46
47The fix isn't a better diff viewer, and it isn't asking reviewers to try harder.
48
49It's putting the explanation **next to the code it explains**, in the order the decisions were
50actually made — so that following the reasoning costs *less* than skipping it.
51
52A big PR stops being a wall of unfamiliar code and becomes something closer to a well-written
53article about a codebase you happen to work on. You still review. You just get to enjoy it.
54
55---
56
57## What it does
58
59- 📖 **Chapters, not files** — ordered by what you need to understand first, never alphabetically
60- 🔗 **Prose anchored to lines** — each paragraph sits beside the exact passage it describes, highlighted
61- 💡 **The reasoning, made explicit** — `why`, `trade-off` and `considered and rejected` callouts
62- ✂️ **Focused panels** — an 800-line file shows the passage under discussion, and says how much it skipped
63- 🧭 **A reading order that exists** — chapter rail, progress, `J`/`K` navigation
64- 📦 **One HTML file** — no network, no assets, no build step; opens offline
65- 🤖 Installable as a [Claude Code](https://docs.claude.com/en/docs/claude-code) skill
66
67![A callout carrying the reasoning, beside the rule it explains](docs/note-dark.jpg)
68
69Above: a `considered and rejected` callout next to the exact line where the decision lives —
70plus two `lines not shown` markers, because a tool that hides code from reviewers to save space
71would be arguing against itself.
72
73---
74
75## See it working
76
77This tool reviewing the pull request that adds this tool:
78
79### **→ [Read the guided review](https://guided-review-example.pages.dev/)**
80
81Then open [the same PR on GitHub](https://github.com/MarceloCajueiro/guided-review/pull/1) and
82compare. The GitHub tab starts at `.claude-plugin/plugin.json`, `.gitignore`, `LICENSE` — three
83files carrying no idea at all. The guided one starts at the idea everything else is built on.
84
85![The opening: six chapters ordered by what you need to understand first](docs/opening-light.jpg)
86
87---
88
89## Requirements
90
91- [Node](https://nodejs.org) v18+ — no `npm install`, zero dependencies
92- [GitHub CLI](https://cli.github.com) (`gh`) authenticated with access to the repo
93
94## Quick start
95
96As a Claude Code skill — the intended path, since writing the narrative is the part that needs
97a reader:
98
99```
100/guided-review 3649
101```
102
103Claude reads the PR and its diff, works out the order the story should be told in, writes the
104narrative, builds the page, and opens it.
105
106Standalone, if you already have a story JSON:
107
108```bash
109gh pr diff 3649 > pr.diff
110node build.mjs story.json pr.diff --out review.html
111```
112
113### Options
114
115| Option | Default | What it does |
116|---|---|---|
117| `--out <file>` | `guided-review-<pr>.html` | Output path. |
118| `--lang <code>` | `en` | UI labels only (`en` or `pt`). The narrative renders in whatever language it was written in. |
119| `--help` | | Usage. |
120
121---
122
123## How it works
124
125A model reads the PR and writes a **story JSON**. `build.mjs` does the mechanical part —
126parsing the diff, resolving anchors to line numbers, rendering the page. Nothing in the script
127does analysis.
128
129That split is why a wording fix is a re-render instead of a re-read of a 2,000-line diff. It's
130also what lets the build *check* the narrative instead of just printing it.
131
132### The unit is a beat
133
134A chapter groups beats; a beat is one passage of prose plus the lines it's about.
135
136```jsonc
137{
138 "number": 3649,
139 "title": "Data readiness onboarding",
140 "url": "https://github.com/org/repo/pull/3649",
141 "headline": "The panel said \"1 family\" where there were hundreds",
142 "lede": "The problem, and how to read this page.",
143 "chapters": [
144 {
145 "title": "One definition of \"pending\"",
146 "beats": [
147 {
148 "text": "Each check declares **two** forms of itself: how it is counted, and how it is listed…",
149 "files": ["app/services/readiness_checks.rb:17-42"],
150 "notes": [
151 { "kind": "why", "text": "Two representations of one rule diverge silently…" }
152 ]
153 }
154 ]
155 }
156 ]
157}
158```
159
160**Anchors** are what make the pairing work:
161
162| Anchor | Effect |
163|---|---|
164| `path/to/file.rb:17-42` | Highlights those new-side lines; the panel focuses on that passage. |
165| `path/to/file.rb:42` | A single line. |
166| `path/to/file.rb` | The whole file, no highlight — for one-line includes. |
167
168**Notes** carry what a diff cannot: `why` (the reasoning behind a decision that had an
169alternative), `tradeoff` (what was knowingly given up), `rejected` (an approach dropped, and
170why).
171
172---
173
174## The build reviews your narrative
175
176It's not just a renderer. It refuses malformed input, and it flags the failures that would
177otherwise ship silently — the dangerous kind, because the output still *looks* trustworthy:
178
179```
180warning: anchor "app/foo.rb:120-140" matches no line in the new file
181```
182
183Somebody guessed a line number instead of reading it off the diff. A wrong-but-valid number
184confidently highlights the wrong code and nothing complains.
185
186```
187warning: 2 substantial file(s) have no place in the narrative.
188 - app/helpers/readiness_helper.rb (+49 −0)
189```
190
191Uncovered files land in an appendix. Right for `db/structure.sql` and one-line includes —
192wrong for a new 49-line helper, which the page would then present as carrying no decision worth
193explaining. Silence about a file is itself a claim; this makes the narrator prove it.
194
195Pointed at a real 27-file PR, that check caught five skipped files on the first run. Pointed at
196this project's own PR, it caught the README.
197
198Ranges that cannot mean anything — running backwards, starting below line 1 — are rejected
199outright rather than warned about, because every one of them produces an empty highlight that
200looks exactly like a correct one.
201
202---
203
204## Reading the page
205
206- **J / K** (or ← / →) move between chapters; **E** expands or collapses every file
207- The left rail tracks where you are; the hairline at the top is reading progress
208- Light and dark follow the OS, with a toggle that overrides and persists
209- Line-anchored panels show that passage plus context, and **always say how much they skipped**
210- Generated files (`structure.sql`, lockfiles, `dist/`, `vendor/`) are detected and collapsed
211
212---
213
214## What it is not
215
216**Not a code review.** No findings, no severity ratings, no suggested changes. Reading for
217*intent* and reading for *defects* are different jobs, and a document that tries to be both
218serves neither. For defects, use [`agentic-cr`](https://github.com/MarceloCajueiro/agentic-cr)
219or Claude Code's built-in `/code-review`.
220
221**Not a substitute for reading the code.** It's a reading order and an explanation — you still
222review. That's the point: it makes the reviewing part worth doing properly.
223
224---
225
226## Tests
227
228```bash
229npm test
230```
231
232`node:test`, no dependencies. The suite covers the places where a silent bug produces a
233confidently wrong page: line-number tracking in the parser, class mapping in the highlighter,
234and — most importantly — that **every hidden line is accounted for** by a marker. A guided
235review that quietly drops code is worse than no guided review at all.
236
237---
238
239## Install
240
241```bash
242git clone https://github.com/MarceloCajueiro/guided-review.git
243```
244
245Then add it as a plugin directory in Claude Code, and `/guided-review` becomes available.
246
247---
248
249## License
250
251MIT © Marcelo Cajueiro — [cajueiro.tech](https://cajueiro.tech)

The screenshots exist because the argument is visual. A page whose whole claim is *"this paragraph is about exactly these lines"* has to be seen to be believed — a feature list cannot make that case.

docs/pairing-light.jpgnew+0 −0
Binary file — no textual diff.
docs/note-dark.jpgnew+0 −0
Binary file — no textual diff.
docs/opening-light.jpgnew+0 −0
Binary file — no textual diff.

Appendix — files not covered by the narrative

These changed with the PR but carry no decision of their own: generated output, mechanical renames, and boilerplate. Listed for completeness.

.claude-plugin/plugin.jsonnew+10 −0
@@ -0,0 +1,10 @@
1{
2 "name": "guided-review",
3 "version": "1.0.0",
4 "description": "Turn a pull request into a guided, narrated code review: a self-contained HTML page where the PR is told as a story, each paragraph paired with the exact diff lines it explains.",
5 "author": { "name": "Marcelo Cajueiro" },
6 "homepage": "https://cajueiro.tech",
7 "repository": "https://github.com/MarceloCajueiro/guided-review",
8 "license": "MIT",
9 "keywords": ["code-review", "pull-request", "github", "storytelling", "developer-experience", "documentation"]
10}
.gitignorenew+7 −0
@@ -0,0 +1,7 @@
1.DS_Store
2node_modules/
3.env
4# generated review pages and their working files
5guided-review-*.html
6*.diff
7story*.json
LICENSEnew+21 −0
@@ -0,0 +1,21 @@
1MIT License
2
3Copyright (c) 2026 Marcelo Cajueiro
4
5Permission is hereby granted, free of charge, to any person obtaining a copy
6of this software and associated documentation files (the "Software"), to deal
7in the Software without restriction, including without limitation the rights
8to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
9copies of the Software, and to permit persons to whom the Software is
10furnished to do so, subject to the following conditions:
11
12The above copyright notice and this permission notice shall be included in all
13copies or substantial portions of the Software.
14
15THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
16IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
17FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
18AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
19LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
20OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
21SOFTWARE.
JKchapter E