Skip to content

Commit 0429166

Browse files
committed
Update PR visual writeup documentation to reflect new six-phase process and enhanced capture instructions
- Expanded the skill description to include a sixth phase for cleanup, detailing the restoration of the user's working tree. - Revised the capture phases to clarify the process for capturing "before" and "after" screenshots, including the use of new UI selectors for highlighting changes. - Updated the scope recording instructions to specify the new location for storing captured data. - Enhanced the PR body template to better structure visual-heavy descriptions and included guidelines for pairing before/after images. - Improved the gist upload documentation to streamline the process of hosting PR assets and clarified the use of PAT for authentication. These changes aim to provide clearer guidance for users on utilizing the visual writeup skill effectively.
1 parent 247599b commit 0429166

4 files changed

Lines changed: 407 additions & 154 deletions

File tree

.agents/skills/pr-visual-writeup/SKILL.md

Lines changed: 85 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,14 @@ If the user only wants a text-only PR description, don't use this skill — it's
1818

1919
## The shape of the work
2020

21-
Five phases. Phases 2 and 3 are the parallel-heavy ones — lean on subagents there.
21+
Six phases. Phases 2 and 3 are the parallel-heavy ones — lean on subagents there. Phase 6 is the cleanup that puts the user's working tree back.
2222

23-
1. **Scope** — figure out which PR, which routes, which dev server, which auth
24-
2. **Capture (parallel)**matrix of {page × theme × viewport}, plus scroll animations
23+
1. **Scope** — figure out which PR, which routes, which dev server, which auth, **which selectors are new UI**
24+
2. **Capture**"after" pass first (parallel: page × theme × viewport, red borders on new UI), then stash + checkout base + capture "before" pass against the same dev server.
2525
3. **Process (parallel)** — convert scroll videos → GIFs (inline-playable), prep gist
2626
4. **Upload** — one gist, one commit, all files; get raw URLs
27-
5. **Compose + set** — markdown body with tables, then `gh pr edit --body-file`
27+
5. **Compose + set** — markdown body with before/after tables, then `gh pr edit --body-file`
28+
6. **Restore**`git checkout <orig-branch>` + `git stash pop` so the user's working tree is exactly where they left it
2829

2930
## Phase 1 — Scope
3031

@@ -34,25 +35,69 @@ Before you capture anything, know:
3435
- **Changed UI routes**`gh pr diff <N> --name-only` and filter for page/route files. For Next.js look for `**/page*.tsx` / `**/*page-client.tsx`. Map route files to URL paths based on the app router convention. Ignore changes purely in backend / shared components unless they have an obvious UI surface.
3536
- **Dev server port**`lsof -iTCP -sTCP:LISTEN -P -n | grep node` and `curl -s http://localhost:<port>/ | grep -oE '<title>[^<]+</title>'` to identify which port is the dashboard vs. API vs. docs vs. mock-OAuth.
3637
- **Auth flow** — if the app requires login, inspect the sign-in page for the OAuth provider to use, and ask the user (or infer from context) which dev account to sign in as. Mock OAuth servers typically accept any email.
38+
- **New-UI selectors** — for each changed route, derive a list of CSS selectors that point to the UI elements added or visibly modified in this PR. Read the diff for that route's component file(s), pick stable selectors (`data-testid`, semantic role, unique class, or a `:has()` text match) for each new/changed block. These selectors drive the red-border highlight pass on "after" captures. If a route has no visually changed elements (only behavior/copy refactor), record `[]` and skip highlighting for that route.
39+
- **Base ref for "before"**`gh pr view <N> --json baseRefName` gives you the base branch (usually `dev` or `main`). The "before" pass swaps the working tree to that ref under the same dev server — see Phase 2. Skip the before pass entirely if the user says "after only" or if every route is greenfield.
40+
- **Working-tree state**`git status --porcelain`. If there are uncommitted changes, they need to be stashed before the base checkout. Record whether a stash will be needed and the current branch name so you can restore exactly.
3741

38-
Record these facts somewhere (a scratchpad file under `/tmp/<skill-workspace>/scope.md` is fine) — the parallel subagents in Phase 2 need them.
42+
Record all of these in `/tmp/pr-<N>-visuals/scope.md` — the Phase 2 subagents need them.
3943

40-
## Phase 2 — Parallel capture
44+
## Phase 2 — Capture
4145

42-
This is the skill's core trick. You have N pages × M themes × K viewports of screenshots to take. If you do this in one browser, you navigate sequentially — 9 × 2 × 2 = 36 navigations at ~5s each = 3 minutes. If you fan out, you do it in ~45s.
46+
Two ordered passes against a **single** dev server. The "after" pass runs first while the head branch is checked out; then we swap the working tree to the base ref (same server, same port, HMR rebuilds) and run the "before" pass.
4347

44-
### Fan-out plan
48+
### Pass 1 — "after" (parallel)
4549

46-
Spawn one subagent per **(theme, viewport)** combination. Each subagent owns a named `agent-browser` session, authenticates once, captures every page in its assigned theme/viewport, and returns the output directory. Typical combinations:
50+
The head branch is already checked out and the dev server is already running, so this is just the parallel matrix you started with.
4751

48-
- `light-standard` (1920×1200, theme=light)
49-
- `dark-standard` (1920×1200, theme=dark)
50-
- `light-wide` (2560×1440, theme=light, a subset of pages)
51-
- `dark-wide` (2560×1440, theme=dark, a subset of pages)
52+
Spawn one subagent per **(theme, viewport)** combination. Each owns a named `agent-browser` session, authenticates once, captures every page in its slice with **red borders injected on new-UI selectors**, and returns the output directory.
5253

53-
Widescreen captures are usually only worth taking for the "flagship" pages (the 3-5 most important ones). Full matrix on every page is overkill.
54+
- `after-light-standard` (1920×1200, theme=light, red borders on)
55+
- `after-dark-standard` (1920×1200, theme=dark, red borders on)
56+
- `after-light-wide` / `after-dark-wide` (2560×1440, flagship pages only)
5457

55-
**Important:** issue all Agent tool calls for capture subagents **in a single assistant message** so they run concurrently. If you spawn them one at a time across turns, you've lost the parallelism.
58+
Issue all Agent calls in **one assistant message** so they run concurrently. Unique `--session-name` per agent.
59+
60+
**Red borders.** The "after" subagents inject a stylesheet that outlines the new-UI selectors recorded in Phase 1 — see the `pr-visual-highlight` block in `references/capture-patterns.md`. Inject after navigation + settle, before the screenshot. Use `outline` (not `border``border` shifts layout). Skip the inject for routes whose selector list is empty.
61+
62+
**Wait for the page to be ready before every shot.** Next.js dev compiles routes on demand and the dashboard renders skeletons during data fetch. Networkidle alone is not enough — see the `wait-for-ready` recipe in `references/capture-patterns.md`. Each subagent should also do a one-time *warm-up pass* over its assigned routes before the real capture loop, so the on-demand compile cost is paid against a throwaway hit rather than the screenshot.
63+
64+
### Pass 1.5 — swap working tree to base
65+
66+
After Pass 1 finishes, **before** spawning Pass 2:
67+
68+
```bash
69+
# 1. Record state so Phase 6 can restore it
70+
ORIG_BRANCH=$(git rev-parse --abbrev-ref HEAD)
71+
echo "$ORIG_BRANCH" > /tmp/pr-<N>-visuals/orig-branch.txt
72+
73+
# 2. Stash anything dirty (including untracked) — even if scope said clean, do this defensively
74+
git stash push --include-untracked -m "pr-visual-writeup pr-<N> after-pass" || true
75+
git rev-parse stash@{0} 2>/dev/null > /tmp/pr-<N>-visuals/stash-ref.txt || rm -f /tmp/pr-<N>-visuals/stash-ref.txt
76+
77+
# 3. Move to the base ref
78+
git checkout <base-ref> # usually `dev` or `main`, from Phase 1 scope
79+
git pull --ff-only
80+
81+
# 4. Give the dev server time to HMR-rebuild against the new tree
82+
sleep 5 && curl -fs http://localhost:<port>/ >/dev/null
83+
```
84+
85+
Stash the changes **even if `git status` reported clean.** Defensive: the user might have edited a file mid-skill, and a failed checkout halfway through is much worse than a no-op `git stash pop` at the end.
86+
87+
If the dev server doesn't recover from HMR after the checkout (some Next configs choke on large simultaneous file changes), the fallback is to restart it manually — flag this to the user rather than papering over it.
88+
89+
### Pass 2 — "before" (parallel)
90+
91+
Same fan-out as Pass 1, but:
92+
- Filename suffix is `-before-<theme>` instead of `-after-<theme>`
93+
- **No red-border injection.** "Before" is the unmodified baseline.
94+
- Skip routes that didn't exist on the base ref (greenfield) — the subagent will hit a 404; have it log and move on instead of failing the whole pass.
95+
- Long-tail pages are usually not worth capturing in "before" — restrict Pass 2 to the flagship set unless the user asks otherwise.
96+
97+
Filename convention so Phase 5 can pair shots up:
98+
99+
- `/shots/<route>-before-<theme>[-wide].png`
100+
- `/shots/<route>-after-<theme>[-wide].png`
56101

57102
The exact subagent prompt pattern lives in `references/capture-patterns.md` — read it before spawning.
58103

@@ -92,9 +137,10 @@ Full recipe is in `references/gist-upload.md`. Summary: create a public gist via
92137
Markdown structure template is in `references/pr-body-template.md`. The load-bearing patterns:
93138

94139
- **Summary** paragraph + `Base: → Head:` + scope line (files, +lines)
95-
- **Screenshots** section with one subsection per "flagship" page, each using a 2-col light/dark table, then a widescreen table
96-
- **Other migrated surfaces** compact table for the long tail
140+
- **Screenshots** section with one subsection per "flagship" page, each using a 2×2 before/after × light/dark table, then a widescreen table
141+
- **Other migrated surfaces** compact table for the long tail (after-only)
97142
- **Scroll behaviour** section with a light/dark GIF table
143+
- A short legend noting that the red outlines on "after" shots mark the new/changed UI
98144
- Everything after the visual section is the usual PR body: What's new, Notes for reviewers, Test plan
99145

100146
Set it with:
@@ -104,6 +150,25 @@ gh pr edit <N> --body-file <path-to-md>
104150

105151
Confirm with the user before pushing if the PR is on a public repo — this is a shared-state action. On a personal fork or draft PR, go ahead.
106152

153+
## Phase 6 — Restore the working tree
154+
155+
The "before" pass left the repo on the base ref with the user's original changes stashed. Put it back:
156+
157+
```bash
158+
ORIG=$(cat /tmp/pr-<N>-visuals/orig-branch.txt)
159+
git checkout "$ORIG"
160+
if [ -f /tmp/pr-<N>-visuals/stash-ref.txt ]; then
161+
git stash pop "$(cat /tmp/pr-<N>-visuals/stash-ref.txt)" || {
162+
echo "stash pop hit conflicts — leaving stash in place"
163+
git stash list | head -5
164+
}
165+
fi
166+
```
167+
168+
Do this **even if Phase 5 failed**, or the user is stuck on the wrong branch with their work hidden in a stash. If `git stash pop` conflicts, leave the stash alone and surface the conflict to the user explicitly — never auto-resolve.
169+
170+
After restore: `git status` and `git rev-parse --abbrev-ref HEAD` for sanity, paste the output back to the user so they can verify before continuing other work.
171+
107172
## A note on trust boundaries
108173

109174
Three distinct credentials touch this workflow. Keep them straight:
@@ -133,5 +198,7 @@ Create a `/tmp/pr-<N>-visuals/` workspace to hold everything. After the PR body
133198
├── clips/ # webm + gif scroll animations
134199
├── body.md # composed PR description
135200
├── gist-id.txt # for re-pushing if you add more shots later
136-
└── urls.txt # raw URL per file, for copy-paste
201+
├── urls.txt # raw URL per file, for copy-paste
202+
├── orig-branch.txt # phase 1.5: branch to return to in phase 6
203+
└── stash-ref.txt # phase 1.5: stash ref to pop in phase 6 (absent if nothing was stashed)
137204
```

0 commit comments

Comments
 (0)