Skip to content

Commit 762f866

Browse files
ericzakariassonericzakariasson
authored
Merge pull request #33 from cursor/add-pr-review-canvas-skill
Add pr-review-canvas skill to Cursor Team Kit
2 parents 7a5018d + 469d9f7 commit 762f866

5 files changed

Lines changed: 461 additions & 0 deletions

File tree

‎cursor-team-kit/README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ Internal-style workflows for CI, code review, shipping, and test reliability.
1616
|:------|:------------|
1717
| `loop-on-ci` | Watch CI runs and iterate on failures until checks pass |
1818
| `review-and-ship` | Run a structured review, commit changes, and open a PR |
19+
| `pr-review-canvas` | Generate an interactive HTML PR walkthrough with annotated, categorized diffs |
1920
| `run-smoke-tests` | Run Playwright smoke tests and triage failures |
2021
| `fix-ci` | Find failing CI jobs, inspect logs, and apply focused fixes |
2122
| `new-branch-and-pr` | Create a fresh branch, complete work, and open a pull request |

‎cursor-team-kit/skills/pr-review-canvas/SKILL.md‎

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
---
2+
name: pr-review-canvas
3+
disable-model-invocation: true
4+
description: Generate an interactive PR review walkthrough as an HTML page. Fetches PR data via gh API, categorizes files into core vs mechanical changes, adds reviewer annotations, and renders diffs with moved-code detection. Use when the user pastes a GitHub PR URL and asks for a review, walkthrough, or summary, or says "review this PR".
5+
---
6+
7+
# PR Review Canvas
8+
9+
Generate an interactive HTML review of a GitHub PR that reads like a peer walking you through what matters.
10+
11+
## Workflow
12+
13+
### 1. Fetch PR data
14+
15+
Run these `gh api` calls in parallel:
16+
17+
```bash
18+
gh api repos/{owner}/{repo}/pulls/{number} --jq '{title, body, user: .user.login, state, additions, deletions, changed_files, base: .base.ref, head: .head.ref}'
19+
gh api repos/{owner}/{repo}/pulls/{number}/files --paginate --jq '.[] | {filename, status, additions, deletions, patch}'
20+
gh api repos/{owner}/{repo}/pulls/{number}/comments --jq '.[] | {user: .user.login, body, path, line}'
21+
```
22+
23+
### 2. Analyze the PR and write the body HTML
24+
25+
Read the diffs, understand the PR, and write the `<body>` content directly as HTML. You have full creative freedom -- the goal is to explain the PR clearly to a reviewer. Use whatever structure best fits the PR.
26+
27+
**Typical structure** (adapt as needed):
28+
- Header with title, PR number, author, stats
29+
- Summary box explaining what the PR does in plain English
30+
- Core file sections with annotations and diffs
31+
- Mechanical/boilerplate files collapsed by default
32+
- Review checklist at the bottom
33+
34+
**But you can also add:**
35+
- **Pseudocode summaries** for verbose code -- show the algorithm in plain English or short pseudocode, with the real diff collapsed below (use a `.bp-section` card labeled "Show full implementation"). Great when 150 lines of retry/backoff/error-handling code is really just "fetch with exponential backoff and circuit breaker."
36+
- Diagrams (inline SVG, mermaid via CDN, ASCII art in `<pre>`)
37+
- Flowcharts showing before/after control flow
38+
- Tables comparing old vs new behavior
39+
- Callout boxes for warnings, questions, or gotchas
40+
- Interactive widgets if they help
41+
- Anything else that makes the review clearer
42+
43+
**Pseudocode pattern example:**
44+
```html
45+
<div class="file-card">
46+
<div class="file-hdr" onclick="toggle(this)">
47+
<span class="fname">retryClient.ts</span>
48+
<div class="fstats"><span class="pill add">+173</span><span class="pill del">&minus;11</span><span class="chev open">&#9654;</span></div>
49+
</div>
50+
<div class="file-body open">
51+
<div class="file-note">
52+
<strong>What this does in plain English:</strong>
53+
<pre style="margin-top:8px;color:var(--text);font-size:12px;line-height:1.6;">
54+
fetch(url):
55+
if circuit breaker is open → fail fast
56+
retry up to N times:
57+
try fetch with timeout
58+
on success → close circuit breaker, return
59+
on retryable error → wait (exponential backoff + jitter)
60+
on non-retryable error → throw
61+
circuit breaker records failure</pre>
62+
</div>
63+
<div class="bp-section" style="margin:0;border:0;border-radius:0;">
64+
<div class="bp-hdr" onclick="toggleBP(this)">
65+
<span>Show full implementation (+173 lines)</span><span class="chev">&#9654;</span>
66+
</div>
67+
<div class="bp-body"><div data-diff="retryClient"></div></div>
68+
</div>
69+
</div>
70+
</div>
71+
```
72+
73+
### 3. Available CSS classes and JS utilities
74+
75+
Read [styles.css](styles.css) and [renderer.js](renderer.js) from this skill directory. These give you a prebuilt dark-themed toolkit. Inject them into [template.html](template.html) verbatim.
76+
77+
**CSS classes you can use:**
78+
79+
| Class | Purpose |
80+
|-------|---------|
81+
| `.header`, `.header h1`, `.header-meta` | Page header |
82+
| `.pill.add`, `.pill.del`, `.pill.files` | Stat badges (+N, -N, N files) |
83+
| `.content` | Centered content wrapper (max 900px) |
84+
| `.summary` | Summary/TL;DR box |
85+
| `.section-title` | Section heading with bottom border |
86+
| `.ic` | Inline code reference (mono, blue, dark bg) |
87+
| `.file-card`, `.file-hdr`, `.file-body` | Collapsible file card (use `onclick="toggle(this)"` on `.file-hdr`) |
88+
| `.file-note` | Sticky reviewer annotation inside a file card |
89+
| `.bp-section`, `.bp-hdr`, `.bp-body` | Collapsed boilerplate card (use `onclick="toggleBP(this)"`) |
90+
| `.bp-note` | Note inside a boilerplate card |
91+
| `.verdict` | Review checklist box |
92+
93+
**JS functions available:**
94+
95+
| Function | Usage |
96+
|----------|-------|
97+
| `toggle(hdrElement)` | Toggle a `.file-body` open/closed |
98+
| `toggleBP(hdrElement)` | Toggle a `.bp-body` open/closed |
99+
| `renderDiff(target, diffInput)` | Render a unified diff. `target` can be a DOM element, string ID, or CSS selector. `diffInput` can be a raw patch string OR an array of lines -- both work. Automatically filters imports, collapses whitespace-only changes, detects moved code (blue/purple tint). |
100+
| `esc(string)` | HTML-escape a string |
101+
102+
**Rendering diffs -- use `data-diff` attributes with auto-discovery.**
103+
Put `<div data-diff="KEY"></div>` placeholders in your body HTML wherever you want a diff rendered. The renderer finds them automatically after DOM load and fills them from the `<script id="pr-diffs-json" type="application/json">` element in `template.html`.
104+
105+
**CRITICAL: Patch strings can contain `</script>` in addition to newlines, backslashes, and quotes.** Even `json.dumps(...)` is not enough if you paste raw output into executable `<script>` because HTML parsing can terminate the tag early. Never manually embed patch strings in JS/JSON. Instead, use this safe approach:
106+
107+
1. During the fetch step, save patches to a JSON file using `jq` (which handles escaping correctly):
108+
```bash
109+
gh api repos/{owner}/{repo}/pulls/{number}/files --paginate \
110+
--jq '[.[] | {key: (.filename | gsub("[^a-zA-Z0-9]"; "_")), value: (.patch // "")}] | from_entries' \
111+
> /tmp/pr-patches-{number}.json
112+
```
113+
114+
2. During assembly, use Python to safely inject the JSON into `template.html`:
115+
```bash
116+
python3 <<'PY'
117+
import json
118+
from pathlib import Path
119+
120+
patches = json.loads(Path('/tmp/pr-patches-{number}.json').read_text())
121+
html = Path('/tmp/pr-review-{number}-body.html').read_text()
122+
css = Path('styles.css').read_text()
123+
js = Path('renderer.js').read_text()
124+
tmpl = Path('template.html').read_text()
125+
126+
# Prevent literal </script> from terminating HTML script tags early.
127+
safe_json = json.dumps(patches).replace('<', '\\u003c').replace('>', '\\u003e').replace('&', '\\u0026')
128+
129+
out = (
130+
tmpl.replace('/* INJECT_CSS */', css)
131+
.replace('/* INJECT_JS */', js)
132+
.replace('<!-- INJECT_BODY -->', html)
133+
.replace('{"__PR_DIFFS_PLACEHOLDER__":true}', safe_json)
134+
)
135+
136+
Path('/tmp/pr-review-{number}.html').write_text(out)
137+
PY
138+
```
139+
140+
This guarantees valid JSON and script-safe HTML embedding. The agent writes body HTML to a temp file, then Python assembles everything safely.
141+
142+
The diff data keys should match the `data-diff` attribute values in the HTML:
143+
```html
144+
<div data-diff="path_to_file_ts"></div>
145+
```
146+
147+
Since renderer.js loads in `<head>`, you can also call `renderDiff(target, lines)` directly from inline `<script>` tags if needed for custom use cases. The function accepts a DOM element, ID string, or CSS selector as `target`, and a string or array as `lines`.
148+
149+
**You're not limited to these.** Add your own inline `<style>` blocks, `<script>` blocks, SVGs, diagrams, or anything else. The prebuilt pieces save time but don't constrain you.
150+
151+
### 4. Assemble and serve
152+
153+
1. Write your body HTML (everything that goes inside `<body>`) to `/tmp/pr-review-{number}-body.html`
154+
2. Save patches to `/tmp/pr-patches-{number}.json` using the `jq` command from step 3 above
155+
3. Run the Python assembly script from step 3 above (reads styles.css, renderer.js, template.html from this skill directory, injects body + patches safely, writes final HTML)
156+
4. Start a local server on a fixed port:
157+
```bash
158+
cd /tmp && python3 -m http.server 8432 --bind 127.0.0.1
159+
```
160+
Run this backgrounded, then navigate the in-app browser to `http://127.0.0.1:8432/pr-review-{number}.html`.
161+
162+
**Why a fixed port and `cd /tmp`:** Background shells have no TTY, so Python buffers its startup message ("Serving HTTP on...") indefinitely — using port 0 means you can never read which port was chosen. And `--directory /tmp` works but `cd /tmp` is more robust across Python versions. If port 8432 is taken, try 8433, 8434, etc.
163+
164+
### Diff features (handled automatically by renderer.js)
165+
166+
- Filters out import-only lines
167+
- Collapses whitespace-only changes into context lines
168+
- Detects moved code blocks (3+ consecutive lines deleted in one place and added identically elsewhere) -- renders in blue/purple instead of red/green
169+
- Near-matches (moved + small edit) get a different purple tint
170+
171+
### Style notes
172+
173+
- Dark theme: `#1a1a1a` background, Inter body font, IBM Plex Mono for code
174+
- Use `var(--warning)` for orange, `var(--success)` for green, `var(--danger)` for red, `var(--accent)` for blue
175+
- Sticky file headers (`position: sticky; top: 0`) and notes (`top: 35px`) pin while scrolling
176+
- Core files expanded by default (`.file-body.open`), mechanical files collapsed

‎cursor-team-kit/skills/pr-review-canvas/renderer.js‎

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
function toggle(hdr) {
2+
var b = hdr.nextElementSibling, c = hdr.querySelector('.chev');
3+
b.classList.toggle('open'); c.classList.toggle('open');
4+
}
5+
function toggleBP(hdr) {
6+
var b = hdr.nextElementSibling, c = hdr.querySelector('.chev');
7+
b.classList.toggle('open'); c.classList.toggle('open');
8+
}
9+
10+
function isImport(line) {
11+
var s = line.replace(/^[+ -]/, '').trim();
12+
return s.startsWith('import ') || s.startsWith('import{') || s.startsWith('} from ');
13+
}
14+
15+
function isWhitespaceOnly(del, add) {
16+
return del.replace(/^-/, '').replace(/\s/g, '') === add.replace(/^\+/, '').replace(/\s/g, '');
17+
}
18+
19+
function esc(s) {
20+
return s.replace(/&/g, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;');
21+
}
22+
23+
function normWs(s) {
24+
return s.replace(/\s+/g, ' ').trim();
25+
}
26+
27+
function toLines(input) {
28+
if (!input) return [];
29+
if (Array.isArray(input)) return input;
30+
if (typeof input === 'string') return input.split('\n');
31+
return [];
32+
}
33+
34+
function loadPrDiffs() {
35+
var el = document.getElementById('pr-diffs-json');
36+
if (!el) return null;
37+
try {
38+
return JSON.parse(el.textContent || '');
39+
} catch (err) {
40+
console.error('Failed to parse PR diff JSON payload', err);
41+
return null;
42+
}
43+
}
44+
45+
function detectMoves(dels, adds) {
46+
var TH = 3;
47+
var md = {}, ma = {};
48+
for (var di = 0; di < dels.length; di++) {
49+
if (md[di]) continue;
50+
var db = [di];
51+
for (var d2 = di+1; d2 < dels.length && d2-di < 40; d2++) {
52+
if (dels[d2].consecutive && !md[d2]) db.push(d2); else break;
53+
}
54+
if (db.length < TH) continue;
55+
var dn = db.map(function(i){ return normWs(dels[i].code); });
56+
for (var ai = 0; ai < adds.length; ai++) {
57+
if (ma[ai]) continue;
58+
var ab = [ai];
59+
for (var a2 = ai+1; a2 < adds.length && a2-ai < 40; a2++) {
60+
if (adds[a2].consecutive && !ma[a2]) ab.push(a2); else break;
61+
}
62+
if (ab.length < TH) continue;
63+
var an = ab.map(function(i){ return normWs(adds[i].code); });
64+
var ml = Math.min(dn.length, an.length), mc = 0;
65+
for (var m = 0; m < ml; m++) { if (dn[m] === an[m]) mc++; }
66+
if (mc >= TH && mc >= ml * 0.7) {
67+
for (var k = 0; k < ml; k++) {
68+
md[db[k]] = { exact: dn[k] === an[k] };
69+
ma[ab[k]] = { exact: dn[k] === an[k] };
70+
}
71+
break;
72+
}
73+
}
74+
}
75+
return { movedDels: md, movedAdds: ma };
76+
}
77+
78+
/**
79+
* renderDiff(target, diffInput)
80+
* target: DOM element, string ID, or CSS selector
81+
* diffInput: array of diff lines, OR a single string (will be split on \n)
82+
*/
83+
function renderDiff(target, diffInput) {
84+
var el;
85+
if (typeof target === 'string') {
86+
el = document.getElementById(target) || document.querySelector(target);
87+
} else {
88+
el = target;
89+
}
90+
if (!el) return;
91+
92+
var lines = toLines(diffInput);
93+
if (!lines.length) { el.innerHTML = '<div style="padding:12px;color:#777;font-size:12px;">No diff data</div>'; return; }
94+
95+
var filtered = lines.filter(function(l) {
96+
if (l.startsWith('--- ') || l.startsWith('+++ ') || l.startsWith('@@') || l.startsWith('diff ')) return true;
97+
return !isImport(l);
98+
});
99+
100+
var wsOut = [];
101+
for (var wi = 0; wi < filtered.length; wi++) {
102+
if (filtered[wi].startsWith('-')) {
103+
var dr = [filtered[wi]], wj = wi+1;
104+
while (wj < filtered.length && filtered[wj].startsWith('-')) { dr.push(filtered[wj]); wj++; }
105+
var ar = [], wk = wj;
106+
while (wk < filtered.length && filtered[wk].startsWith('+')) { ar.push(filtered[wk]); wk++; }
107+
if (dr.length === ar.length && dr.length > 0) {
108+
var allWs = true;
109+
for (var wc = 0; wc < dr.length; wc++) { if (!isWhitespaceOnly(dr[wc], ar[wc])) { allWs = false; break; } }
110+
if (allWs) { for (var wx = 0; wx < ar.length; wx++) wsOut.push(' ' + ar[wx].slice(1)); wi = wk-1; continue; }
111+
}
112+
}
113+
wsOut.push(filtered[wi]);
114+
}
115+
116+
var dels = [], adds = [], parsed = [];
117+
var oL = 0, nL = 0, pD = false, pA = false;
118+
for (var pi = 0; pi < wsOut.length; pi++) {
119+
var line = wsOut[pi];
120+
if (line.startsWith('--- ') || line.startsWith('+++ ') || line.startsWith('diff ')) continue;
121+
if (line.startsWith('@@')) {
122+
var hm = line.match(/@@ -(\d+)(?:,\d+)? \+(\d+)/);
123+
if (hm) { oL = parseInt(hm[1]); nL = parseInt(hm[2]); }
124+
parsed.push({ type: 'hunk', text: line }); pD = false; pA = false; continue;
125+
}
126+
if (line.startsWith('+')) {
127+
var ae = { type:'add', code:line.slice(1), newLine:nL, consecutive:pA, idx:parsed.length };
128+
adds.push(ae); parsed.push(ae); nL++; pA = true; pD = false;
129+
} else if (line.startsWith('-')) {
130+
var de = { type:'del', code:line.slice(1), oldLine:oL, consecutive:pD, idx:parsed.length };
131+
dels.push(de); parsed.push(de); oL++; pD = true; pA = false;
132+
} else {
133+
var c = line.startsWith(' ') ? line.slice(1) : line;
134+
parsed.push({ type:'ctx', code:c, oldLine:oL, newLine:nL }); oL++; nL++; pD = false; pA = false;
135+
}
136+
}
137+
138+
var mv = detectMoves(dels, adds);
139+
var rows = [];
140+
for (var ri = 0; ri < parsed.length; ri++) {
141+
var p = parsed[ri];
142+
if (p.type === 'hunk') {
143+
rows.push('<tr class="diff-hunk"><td class="diff-ln"></td><td class="diff-ln"></td><td class="diff-code">' + esc(p.text) + '</td></tr>');
144+
} else if (p.type === 'add') {
145+
var ai2 = -1; for (var fa=0;fa<adds.length;fa++) if(adds[fa].idx===p.idx){ai2=fa;break;}
146+
var cls = (mv.movedAdds[ai2]) ? (mv.movedAdds[ai2].exact ? 'diff-moved-add' : 'diff-moved-add-edited') : 'diff-add';
147+
rows.push('<tr class="'+cls+'"><td class="diff-ln"></td><td class="diff-ln">'+p.newLine+'</td><td class="diff-code">'+esc(p.code)+'</td></tr>');
148+
} else if (p.type === 'del') {
149+
var di2 = -1; for(var fd=0;fd<dels.length;fd++) if(dels[fd].idx===p.idx){di2=fd;break;}
150+
var cls2 = (mv.movedDels[di2]) ? (mv.movedDels[di2].exact ? 'diff-moved-del' : 'diff-moved-del-edited') : 'diff-del';
151+
rows.push('<tr class="'+cls2+'"><td class="diff-ln">'+p.oldLine+'</td><td class="diff-ln"></td><td class="diff-code">'+esc(p.code)+'</td></tr>');
152+
} else {
153+
rows.push('<tr class="diff-ctx"><td class="diff-ln">'+p.oldLine+'</td><td class="diff-ln">'+p.newLine+'</td><td class="diff-code">'+esc(p.code)+'</td></tr>');
154+
}
155+
}
156+
el.innerHTML = '<table class="diff-table"><tbody>' + rows.join('') + '</tbody></table>';
157+
}
158+
159+
/* Auto-discovery: after DOM loads, find all [data-diff] elements and render diffs from pr-diffs-json. */
160+
document.addEventListener('DOMContentLoaded', function() {
161+
var prDiffs = loadPrDiffs();
162+
if (!prDiffs) return;
163+
var els = document.querySelectorAll('[data-diff]');
164+
for (var i = 0; i < els.length; i++) {
165+
var key = els[i].getAttribute('data-diff');
166+
if (key && Object.prototype.hasOwnProperty.call(prDiffs, key)) {
167+
renderDiff(els[i], prDiffs[key]);
168+
}
169+
}
170+
});

0 commit comments

Comments
 (0)