Skip to content

feat(screenshot-change): add shoot runner and fix stale verbs - #238

Closed
moritzwilksch wants to merge 2 commits into
mainfrom
mw/fix-screenshot-harness
Closed

moritzwilksch wants to merge 2 commits into
mainfrom
mw/fix-screenshot-harness

Conversation

@moritzwilksch

@moritzwilksch moritzwilksch commented Sep 30, 2026 •

Copy link
Copy Markdown
Owner

Makes the screenshot skill cheap for agents to use. It also fixes the per-file verbs, which had timed out on main.

Runner

A scenario is a small module in the agent's scratch directory:

export const repo = { before: { 'a.py': 'x = 1\n' }, after: { 'a.py': 'x = 2\n' } };
export default async ({ page, shot, selectLines }) => {
  await selectLines('a.py', 1);
  await page.keyboard.type('Wait...');
  await shot('composer', page.locator('textarea').first());
};

node .agents/skills/screenshot-change/scripts/shoot.mjs scenario.mjs --before origin/main runs it and prints only this:

/tmp/…/lig
  ellipsis-after.png 752×128
  ellipsis-before.png 752×128
  ellipsis-compare.png 788×107  ← read this to verify

compare

  • Demo repos: demoRepo creates them in the temp dir with an explicit -C and inline identity, so nothing can touch the checkout or its config.
  • Before state: --before runs a git archive copy of the base revision with its own server and client. It's cached per commit, so re-runs take about 4 s, and it never swaps dist/client or registers a worktree.
  • Quiet builds: build and ffmpeg output is captured and shown only on failure. It used to be about 370 lines per build, and a before/after run built three times. The client rebuilds only when its sources change.
  • One read to verify: one labelled compare image per shot.
  • Short failures: the error, the scenario line, and a 1x failure-<side>.png; exit status 1.
  • Self-test: scripts/selftest.mjs exercises every verb, and lookup errors name the likely cause.
  • Docs: SKILL.md is about a third smaller and is organised around the runner, with a verb table that matches the harness.

💥 Breaking: withBaseClient, installClient, CLIENT_DIR and scenario.template.mjs are removed. Write scenarios for shoot.mjs, or call baseCheckout(rev) and startDiffle({ root }) directly.

Stale verbs (first commit)

header, viewed, collapsed, setViewed, toggleCollapse and selectLines all timed out on main:

  • Header lookup: fileItem looked for the viewer's default [data-title] header, which diffle has replaced with its own slotted FileHeader. It now matches the path span's title there.
  • Tooltips: TooltipHost removes title from the hovered control, so the verbs now move the pointer to the top-left corner first.
  • Cursor line: pressing the number of the cursor's selected line toggled the selection off, so selectLines presses Escape first. fix(review): comment on the cursor's line when its number is pressed #239 fixes the app side.

Verified

  • selftest.mjs: all verbs pass, including a comment stored on a.py line 2.
  • A before/after run against c9dc8d2506 shows the ligature bug fixed by fix(client): disable font ligatures #237 (image above).
  • A video scenario and a failing scenario behave as described.
  • Also tested: an existing repo path with revs, setup seeding threads, and an uncommitted demo repo.
  • Lint, format and npm test pass.

The per-file verbs looked for the viewer's default [data-title] header,
which diffle replaced with its own slotted FileHeader, so header, viewed,
collapsed, setViewed, toggleCollapse and selectLines all timed out.

Two further failures surfaced once they resolved: the app's tooltip lifts
`title` off a hovered control, and a press on the cursor's selected line
toggles it off. The verbs now park the pointer, and selectLines presses
Escape first.
A scenario now exports a demo repo and one interaction function; the
runner owns the server, browser, before/after runs, and cleanup, and
prints only output paths plus a labelled compare image.

- demoRepo builds throwaway repos with explicit -C and inline identity.
- --before runs a cached git-archive checkout of the base revision with
  its own server and client, so dist/client is never swapped.
- Builds and ffmpeg are silent unless they fail; the client rebuilds
  only when its sources change.
- selftest.mjs exercises every verb; lookups name the likely cause.

BREAKING CHANGE: withBaseClient, installClient, CLIENT_DIR and
scenario.template.mjs are gone; write scenarios for shoot.mjs.
@moritzwilksch moritzwilksch changed the title fix(screenshot-change): find files by diffle's custom header feat(screenshot-change): add shoot runner and fix stale verbs Sep 30, 2026
@github-actions github-actions Bot added the enhancement New feature or request label Sep 30, 2026
@moritzwilksch

Copy link
Copy Markdown
Owner Author

Superseded by #242, which ports this onto the e2e harness from #214.

@pavelzw
pavelzw deleted the mw/fix-screenshot-harness branch September 30, 2026 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant