Skip to content

docs: refresh usage guide and add Playwright screenshot pipeline#12

Merged
tomusdrw merged 2 commits intomainfrom
td-usage-guide
Apr 22, 2026
Merged

docs: refresh usage guide and add Playwright screenshot pipeline#12
tomusdrw merged 2 commits intomainfrom
td-usage-guide

Conversation

@tomusdrw
Copy link
Copy Markdown
Member

Summary

  • Rewrite docs/usage-guide.md to match the current UI (sprints 38–49): redesigned host-call drawer with two-column layout, fetch handler with three modes, pending-changes banner, generic text editor DSL, Ananas PVM, error boundary recovery, and trace link-scroll. Every screenshot regenerated in dark mode.
  • Add apps/web/screenshots/ — a Playwright-based capture pipeline so the screenshot set can be refreshed via npm run screenshots after future UI changes. Shared helpers document the non-obvious traps (Next vs Run for host-call advancement, PC-diff for pause detection).
  • Complete sprint-49's ω → φ rename in PendingChanges.tsx and DetectionSummary.tsx — both stored the symbol as \u03C9, which sprint-49's literal-character grep missed. Sprint-49's verification regex is broadened to catch both forms so the next rename sprint doesn't repeat the miss.
  • Session captured in spec/ui/sprint-50-usage-guide-refresh.md.

Test plan

  • npm test — all 683 unit tests pass
  • npm run build — builds clean
  • cd apps/web && npm run screenshots — 18/18 captures pass in ~25s
  • Every image reference in docs/usage-guide.md resolves to an existing file
  • Pre-push hook (build + Biome + tests) passes locally
  • grep -rE '(ω|\\u03C9|\\u03c9|U\+03C9|omega)' --include='*.ts' --include='*.tsx' apps/ packages/ returns nothing
  • Reviewer: open docs/usage-guide.md in a preview (VS Code Cmd+Shift+V or npx markserv docs/usage-guide.md) to verify screenshots render correctly
  • Reviewer: run cd apps/web && npm run screenshots to confirm the pipeline reproduces the committed PNGs

Notes

  • One screenshot (host-call-storage.png) was retired: the redesigned layout is adequately shown by host-call-overview (fetch) and host-call-log (log), and driving the app to a storage pause was brittle enough to repeatedly trip a residual React error #185 render loop.
  • That render-loop bug is a known follow-up item (noted in the sprint-50 spec). Sprint-47 suppressed the common trace-replay trigger but rapid Run clicks with auto-continue=never can still hit it. Out of scope here.

🤖 Generated with Claude Code

tomusdrw and others added 2 commits April 22, 2026 11:38
Rewrite docs/usage-guide.md to match the current UI (sprints 38-49):
redesigned host-call drawer, fetch handler, pending-changes banner,
Ananas PVM, error boundary, and link-scroll. Every screenshot
regenerated in dark mode.

Add apps/web/screenshots/ with a dedicated Playwright config so the
screenshot set can be regenerated via `npm run screenshots` after
future UI changes. Shared helpers cover the non-obvious traps
(Next vs Run, PC-diff for pause advancement).

Also complete sprint-49's ω → φ rename in PendingChanges.tsx and
DetectionSummary.tsx (both stored the symbol as \u03C9, which the
original verification grep missed) and broaden sprint-49's
verification regex so future rename sprints don't repeat the miss.

Session captured in spec/ui/sprint-50-usage-guide-refresh.md.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pre-push hook caught two issues:
- helpers.ts: collapse `a && a.b` to `a?.b`.
- 03-debugger.screenshot.ts: import sort order and line length.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 22, 2026

Deploy Preview for pvm-debugger ready!

Name Link
🔨 Latest commit e335fc6
🔍 Latest deploy log https://app.netlify.com/projects/pvm-debugger/deploys/69e8979705210d0008516e42
😎 Deploy Preview https://deploy-preview-12--pvm-debugger.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 22, 2026

📝 Walkthrough

Walkthrough

This PR introduces a comprehensive Playwright-based screenshot capture system for the web app, enabling automated visual testing and documentation generation. It adds dedicated test files for load, config, debugger, settings, persistence, host-calls, and fetch-trace features, shared test utilities, a dedicated Playwright configuration, an npm script, updated usage documentation, and minor UI symbol updates from omega to phi.

Changes

Cohort / File(s) Summary
Screenshot Infrastructure
apps/web/package.json, apps/web/playwright.screenshots.config.ts
Added screenshots npm script and new Playwright config targeting ./screenshots directory with single-worker execution, 1440×900 viewport at 2x scale, dark theme, and local vite preview server on port 4199.
Screenshot Test Suite
apps/web/screenshots/01-load.screenshot.ts, apps/web/screenshots/02-config.screenshot.ts, apps/web/screenshots/03-debugger.screenshot.ts, apps/web/screenshots/04-settings.screenshot.ts, apps/web/screenshots/05-persistence.screenshot.ts, apps/web/screenshots/06-host-calls.screenshot.ts, apps/web/screenshots/07-fetch-trace.screenshot.ts
Seven new Playwright screenshot test files capturing deterministic UI states across loader, config, debugger, settings/multi-PVM, persistence, host-call, and fetch-trace workflows with page interactions, waits, and assertions.
Screenshot Helpers
apps/web/screenshots/helpers.ts, apps/web/screenshots/README.md
Shared Playwright utilities for dark-mode injection, font readiness, settling, capturing, and host-call navigation (PC detection, auto-continue control, run-to-host-call polling, host-call advancement); includes comprehensive documentation of conventions and debugging gotchas.
UI Symbol Updates
apps/web/src/components/debugger/PendingChanges.tsx, apps/web/src/components/load/DetectionSummary.tsx
Changed register label symbol from omega (\u03C9) to phi (\u03C6) in pending-changes and register-preview components.
Documentation Updates
docs/usage-guide.md, spec/ui/sprint-49-register-symbol-omega-to-phi.md, spec/ui/sprint-50-usage-guide-refresh.md
Significantly expanded usage guide with current UI terminology, updated loader/config/debugger/host-call/fetch sections with new features; added sprint-50 spec documenting screenshot pipeline and UI refresh; updated sprint-49 verification to forbid omega symbol in all representations.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 76.92% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the two main objectives of the PR: refreshing the usage guide documentation and adding a Playwright-based screenshot pipeline.
Description check ✅ Passed The description comprehensively covers the changeset, detailing the usage guide rewrite, Playwright pipeline addition, symbol rename completion, and test results.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch td-usage-guide

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (2)
apps/web/screenshots/06-host-calls.screenshot.ts (1)

22-28: Simplify the single-value trace loader helper.

loadTraceExample is currently parameterized but only accepts "io-trace". A dedicated helper name makes intent clearer and avoids a needless argument.

♻️ Proposed cleanup
-async function loadTraceExample(page: Page, id: "io-trace") {
+async function loadIoTrace(page: Page) {
   await page.goto("/#/load");
-  await page.getByTestId(`example-card-${id}`).click();
+  await page.getByTestId("example-card-io-trace").click();
   await expect(page.getByTestId("debugger-page")).toBeVisible({
     timeout: 15000,
   });
 }
@@
-    await loadTraceExample(page, "io-trace");
+    await loadIoTrace(page);
@@
-    await loadTraceExample(page, "io-trace");
+    await loadIoTrace(page);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/screenshots/06-host-calls.screenshot.ts` around lines 22 - 28, The
helper loadTraceExample is parameterized but only ever called with the literal
"io-trace"; replace it with a dedicated, no-argument helper (e.g.,
loadIoTraceExample) that removes the id parameter and hardcodes the test id
string, update its implementation to navigate to "/#/load", click
page.getByTestId("example-card-io-trace"), and wait for
page.getByTestId("debugger-page") to be visible with the same timeout; also
update any call sites that invoke loadTraceExample("io-trace") to call the new
loadIoTraceExample instead.
apps/web/screenshots/03-debugger.screenshot.ts (1)

5-11: Narrow example id type for safer fixture selection.

Typing id as string allows silent typos in test ids. A narrow union keeps this helper self-validating.

🔧 Suggested type tightening
-async function loadGeneric(page: Page, id: string) {
+type GenericExampleId = "fibonacci" | "game-of-life";
+
+async function loadGeneric(page: Page, id: GenericExampleId) {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/screenshots/03-debugger.screenshot.ts` around lines 5 - 11, The
helper loadGeneric currently accepts id: string which permits silent typos when
selecting test fixtures; change the id parameter to a narrowed union type (or an
exported type alias, e.g., ExampleId = 'exampleA' | 'exampleB' | ...) listing
the valid example ids used in your tests, update the function signature of
loadGeneric(page: Page, id: ExampleId) and any call sites to use those exact
constants, and keep the existing selectors (example-card-${id} and
debugger-page) unchanged so the test only accepts known valid ids.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/web/screenshots/01-load.screenshot.ts`:
- Around line 45-47: The hex payload string passed into
.getByTestId("manual-input-field").fill(...) is hardcoded; move that payload
into a fixtures file (e.g., fixtures/manual-hex-payload.hex), update the test to
read the fixture (via your test fixture helper or a simple fs.readFileSync/await
readFile) and pass the file contents to .fill, trimming any trailing newline;
replace the literal string in the .fill call with the variable holding the
fixture content so the test complies with the repo's test-data rule and is
reusable.

In `@apps/web/screenshots/04-settings.screenshot.ts`:
- Around line 29-31: After clicking the switch when isChecked is false,
explicitly assert the control is now checked to avoid flakiness: after await
ananasSwitch.click() (in the code that references isChecked and ananasSwitch)
add an await that verifies the switch state (e.g., await
ananasSwitch.isChecked() asserted to true or using your test framework's
expect(await ananasSwitch.isChecked()).toBe(true)) before proceeding to close
the drawer so the enabled state is deterministic.

In `@apps/web/screenshots/07-fetch-trace.screenshot.ts`:
- Around line 61-63: The test ignores the boolean returned by
advanceToNextHostCall which can be false and cause the trace capture to be
out-of-sync; update the flow to capture its return value (e.g., const advanced =
await advanceToNextHostCall(page)) and assert it before proceeding (use an
expect or throw if advanced is false) so the subsequent click on
page.getByTestId("drawer-tab-ecalli_trace") and
expect(page.getByTestId("ecalli-trace-tab")).toBeVisible() only run when
advancement succeeded.

In `@apps/web/screenshots/helpers.ts`:
- Around line 79-85: The type guard in capture() is unsafe because both Page and
Locator have screenshot()/evaluate(); change the discriminator to check for
"goto" in target (since goto exists only on Page) so the Page branch is only
taken for actual Page instances; then call waitForFontsReady(page) and
page.screenshot({ path, fullPage: false, animations: "disabled" }) inside the
"goto" branch, and call (target as Locator).screenshot({ path, animations:
"disabled" }) in the else branch to avoid passing fullPage to Locator; update
any casts and references to Page and Locator accordingly (capture, Page,
Locator, waitForFontsReady).

In `@docs/usage-guide.md`:
- Around line 114-123: The fenced command block containing the example commands
(setreg φ7 <- 0x2a, memwrite 0x100 len=4 <- 0xdeadbeef, setgas <- 500000) is
missing a language tag and triggers MD040; update the triple-backtick fence to
include a language identifier (e.g., ```text) so the block is explicitly marked
as plaintext. Locate the block around the commands in docs/usage-guide.md and
change the opening fence to include the tag without altering the example lines
or semantics.

---

Nitpick comments:
In `@apps/web/screenshots/03-debugger.screenshot.ts`:
- Around line 5-11: The helper loadGeneric currently accepts id: string which
permits silent typos when selecting test fixtures; change the id parameter to a
narrowed union type (or an exported type alias, e.g., ExampleId = 'exampleA' |
'exampleB' | ...) listing the valid example ids used in your tests, update the
function signature of loadGeneric(page: Page, id: ExampleId) and any call sites
to use those exact constants, and keep the existing selectors
(example-card-${id} and debugger-page) unchanged so the test only accepts known
valid ids.

In `@apps/web/screenshots/06-host-calls.screenshot.ts`:
- Around line 22-28: The helper loadTraceExample is parameterized but only ever
called with the literal "io-trace"; replace it with a dedicated, no-argument
helper (e.g., loadIoTraceExample) that removes the id parameter and hardcodes
the test id string, update its implementation to navigate to "/#/load", click
page.getByTestId("example-card-io-trace"), and wait for
page.getByTestId("debugger-page") to be visible with the same timeout; also
update any call sites that invoke loadTraceExample("io-trace") to call the new
loadIoTraceExample instead.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f482a662-bb84-4b60-b570-67279a957657

📥 Commits

Reviewing files that changed from the base of the PR and between 7b38b62 and e335fc6.

⛔ Files ignored due to path filters (19)
  • docs/usage-screenshots/config-jam-builder.png is excluded by !**/*.png
  • docs/usage-screenshots/config-jam-raw.png is excluded by !**/*.png
  • docs/usage-screenshots/debugger-full.png is excluded by !**/*.png
  • docs/usage-screenshots/debugger-stepping.png is excluded by !**/*.png
  • docs/usage-screenshots/host-call-fetch.png is excluded by !**/*.png
  • docs/usage-screenshots/host-call-log.png is excluded by !**/*.png
  • docs/usage-screenshots/host-call-overview.png is excluded by !**/*.png
  • docs/usage-screenshots/host-call-pending-changes.png is excluded by !**/*.png
  • docs/usage-screenshots/host-call-storage.png is excluded by !**/*.png
  • docs/usage-screenshots/load-examples.png is excluded by !**/*.png
  • docs/usage-screenshots/load-manual.png is excluded by !**/*.png
  • docs/usage-screenshots/load-upload.png is excluded by !**/*.png
  • docs/usage-screenshots/load-url.png is excluded by !**/*.png
  • docs/usage-screenshots/memory-panel.png is excluded by !**/*.png
  • docs/usage-screenshots/multiple-pvms.png is excluded by !**/*.png
  • docs/usage-screenshots/persistence-restored.png is excluded by !**/*.png
  • docs/usage-screenshots/registers-edit.png is excluded by !**/*.png
  • docs/usage-screenshots/settings.png is excluded by !**/*.png
  • docs/usage-screenshots/trace-comparison.png is excluded by !**/*.png
📒 Files selected for processing (16)
  • apps/web/package.json
  • apps/web/playwright.screenshots.config.ts
  • apps/web/screenshots/01-load.screenshot.ts
  • apps/web/screenshots/02-config.screenshot.ts
  • apps/web/screenshots/03-debugger.screenshot.ts
  • apps/web/screenshots/04-settings.screenshot.ts
  • apps/web/screenshots/05-persistence.screenshot.ts
  • apps/web/screenshots/06-host-calls.screenshot.ts
  • apps/web/screenshots/07-fetch-trace.screenshot.ts
  • apps/web/screenshots/README.md
  • apps/web/screenshots/helpers.ts
  • apps/web/src/components/debugger/PendingChanges.tsx
  • apps/web/src/components/load/DetectionSummary.tsx
  • docs/usage-guide.md
  • spec/ui/sprint-49-register-symbol-omega-to-phi.md
  • spec/ui/sprint-50-usage-guide-refresh.md

Comment on lines +45 to +47
.getByTestId("manual-input-field")
.fill("0x00 03 00 01 00 0d 00 08 00 02 00 07 00 01");
// Blur so the byte count appears.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Move embedded manual hex payload to a fixture file.

Line 46 hardcodes a hex payload in source, which breaks the repo’s test-data rule and makes this scenario harder to maintain/reuse.

Proposed fix
+import { readFile } from "node:fs/promises";
 import path from "node:path";
 import { fileURLToPath } from "node:url";
 import { capture, expect, settle, test } from "./helpers";
@@
 const HERE = path.dirname(fileURLToPath(import.meta.url));
 const FIXTURES = path.resolve(HERE, "..", "..", "..", "fixtures");
+const MANUAL_HEX_FIXTURE = path.join(FIXTURES, "generic", "load-manual.hex");
@@
     await page
       .getByTestId("manual-input-field")
-      .fill("0x00 03 00 01 00 0d 00 08 00 02 00 07 00 01");
+      .fill((await readFile(MANUAL_HEX_FIXTURE, "utf8")).trim());

As per coding guidelines, Use fixtures/ directory for example programs and test data; do NOT embed hex strings in source code.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
.getByTestId("manual-input-field")
.fill("0x00 03 00 01 00 0d 00 08 00 02 00 07 00 01");
// Blur so the byte count appears.
import { readFile } from "node:fs/promises";
import path from "node:path";
import { fileURLToPath } from "node:url";
import { capture, expect, settle, test } from "./helpers";
const HERE = path.dirname(fileURLToPath(import.meta.url));
const FIXTURES = path.resolve(HERE, "..", "..", "..", "fixtures");
const MANUAL_HEX_FIXTURE = path.join(FIXTURES, "generic", "load-manual.hex");
const hexPayload = (await readFile(MANUAL_HEX_FIXTURE, "utf8")).trim();
await page
.getByTestId("manual-input-field")
.fill(hexPayload);
// Blur so the byte count appears.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/screenshots/01-load.screenshot.ts` around lines 45 - 47, The hex
payload string passed into .getByTestId("manual-input-field").fill(...) is
hardcoded; move that payload into a fixtures file (e.g.,
fixtures/manual-hex-payload.hex), update the test to read the fixture (via your
test fixture helper or a simple fs.readFileSync/await readFile) and pass the
file contents to .fill, trimming any trailing newline; replace the literal
string in the .fill call with the variable holding the fixture content so the
test complies with the repo's test-data rule and is reusable.

Comment on lines +29 to +31
if (!isChecked) {
await ananasSwitch.click();
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Assert toggle state after enabling ananas to reduce flakiness.

After clicking the switch, assert it is checked before closing the drawer; this makes capture state deterministic.

✅ Suggested reliability fix
     if (!isChecked) {
       await ananasSwitch.click();
+      await expect(ananasSwitch).toBeChecked();
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!isChecked) {
await ananasSwitch.click();
}
if (!isChecked) {
await ananasSwitch.click();
await expect(ananasSwitch).toBeChecked();
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/screenshots/04-settings.screenshot.ts` around lines 29 - 31, After
clicking the switch when isChecked is false, explicitly assert the control is
now checked to avoid flakiness: after await ananasSwitch.click() (in the code
that references isChecked and ananasSwitch) add an await that verifies the
switch state (e.g., await ananasSwitch.isChecked() asserted to true or using
your test framework's expect(await ananasSwitch.isChecked()).toBe(true)) before
proceeding to close the drawer so the enabled state is deterministic.

Comment on lines +61 to +63
await advanceToNextHostCall(page);
await page.getByTestId("drawer-tab-ecalli_trace").click();
await expect(page.getByTestId("ecalli-trace-tab")).toBeVisible();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Assert host-call advancement before capturing trace comparison.

The boolean result from advanceToNextHostCall is currently ignored. If it returns false, the capture can drift from the intended scenario.

🧪 Suggested guard
-    await advanceToNextHostCall(page);
+    const advanced = await advanceToNextHostCall(page);
+    await expect(advanced).toBe(true);
     await page.getByTestId("drawer-tab-ecalli_trace").click();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
await advanceToNextHostCall(page);
await page.getByTestId("drawer-tab-ecalli_trace").click();
await expect(page.getByTestId("ecalli-trace-tab")).toBeVisible();
const advanced = await advanceToNextHostCall(page);
await expect(advanced).toBe(true);
await page.getByTestId("drawer-tab-ecalli_trace").click();
await expect(page.getByTestId("ecalli-trace-tab")).toBeVisible();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/screenshots/07-fetch-trace.screenshot.ts` around lines 61 - 63, The
test ignores the boolean returned by advanceToNextHostCall which can be false
and cause the trace capture to be out-of-sync; update the flow to capture its
return value (e.g., const advanced = await advanceToNextHostCall(page)) and
assert it before proceeding (use an expect or throw if advanced is false) so the
subsequent click on page.getByTestId("drawer-tab-ecalli_trace") and
expect(page.getByTestId("ecalli-trace-tab")).toBeVisible() only run when
advancement succeeded.

Comment on lines +79 to +85
if ("screenshot" in target && "evaluate" in target) {
const page = target as Page;
await waitForFontsReady(page);
await page.screenshot({ path, fullPage: false, animations: "disabled" });
} else {
await (target as Locator).screenshot({ path, animations: "disabled" });
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In Playwright v1.50, does Locator expose evaluate(), and does Locator.screenshot support a fullPage option?

💡 Result:

In Playwright v1.50, Locator exposes an evaluate method, but Locator.screenshot does not support a fullPage option. Locator.evaluate is listed in the official API documentation among the methods available on the Locator class. This has been a standard method since earlier versions and remains in v1.50, as confirmed by the release notes which do not indicate its removal. Locator.screenshot captures a screenshot of the specific element or bounding box represented by the locator. The fullPage option is exclusively for Page.screenshot, which captures the full scrollable page. Official screenshots documentation explicitly shows page.screenshot({ fullPage: true }) for full page captures and locator.screenshot for element screenshots without mentioning fullPage for locators. Older API docs (e.g., v1.22) and recent issues confirm Locator.ScreenshotOptions lacks fullPage, and v1.50 release notes introduce no such change.

Citations:


Fix Page/Locator type guard in capture() (currently ambiguous).

Both Page and Locator expose screenshot() and evaluate(), so the discriminator is unsafe. If a Locator is misclassified as Page and routed through the first branch, it will receive the fullPage option, which Locator.screenshot() does not support, causing a failure.

Use "goto" in target instead, since goto() exists only on Page:

Proposed fix
 export async function capture(
   target: Page | Locator,
   name: string,
 ): Promise<void> {
   const path = screenshotPath(name);
-  if ("screenshot" in target && "evaluate" in target) {
-    const page = target as Page;
-    await waitForFontsReady(page);
-    await page.screenshot({ path, fullPage: false, animations: "disabled" });
+  if ("goto" in target) {
+    await waitForFontsReady(target);
+    await target.screenshot({ path, fullPage: false, animations: "disabled" });
   } else {
-    await (target as Locator).screenshot({ path, animations: "disabled" });
+    await target.screenshot({ path, animations: "disabled" });
   }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if ("screenshot" in target && "evaluate" in target) {
const page = target as Page;
await waitForFontsReady(page);
await page.screenshot({ path, fullPage: false, animations: "disabled" });
} else {
await (target as Locator).screenshot({ path, animations: "disabled" });
}
if ("goto" in target) {
await waitForFontsReady(target);
await target.screenshot({ path, fullPage: false, animations: "disabled" });
} else {
await target.screenshot({ path, animations: "disabled" });
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/screenshots/helpers.ts` around lines 79 - 85, The type guard in
capture() is unsafe because both Page and Locator have screenshot()/evaluate();
change the discriminator to check for "goto" in target (since goto exists only
on Page) so the Page branch is only taken for actual Page instances; then call
waitForFontsReady(page) and page.screenshot({ path, fullPage: false, animations:
"disabled" }) inside the "goto" branch, and call (target as
Locator).screenshot({ path, animations: "disabled" }) in the else branch to
avoid passing fullPage to Locator; update any casts and references to Page and
Locator accordingly (capture, Page, Locator, waitForFontsReady).

Comment thread docs/usage-guide.md
Comment on lines +114 to +123
```
# Return value in φ7
setreg φ7 <- 0x2a

# Write 4 bytes to memory
memwrite 0x100 len=4 <- 0xdeadbeef

![Debugger paused on a host call with the Host Call drawer open](usage-screenshots/host-call-overview.png)
# Set gas after the call
setgas <- 500000
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add a language tag to the fenced command block.

The fenced block in this section is missing a language identifier (MD040), which can fail lint/docs checks.

📝 Minimal fix
-  ```
+  ```text
   # Return value in φ7
   setreg φ7 <- 0x2a
@@
   # Set gas after the call
   setgas <- 500000
</details>

<!-- suggestion_start -->

<details>
<summary>📝 Committable suggestion</summary>

> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

```suggestion

🧰 Tools
🪛 markdownlint-cli2 (0.22.0)

[warning] 114-114: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/usage-guide.md` around lines 114 - 123, The fenced command block
containing the example commands (setreg φ7 <- 0x2a, memwrite 0x100 len=4 <-
0xdeadbeef, setgas <- 500000) is missing a language tag and triggers MD040;
update the triple-backtick fence to include a language identifier (e.g.,
```text) so the block is explicitly marked as plaintext. Locate the block around
the commands in docs/usage-guide.md and change the opening fence to include the
tag without altering the example lines or semantics.

@tomusdrw tomusdrw merged commit 05cce8c into main Apr 22, 2026
7 checks passed
@tomusdrw tomusdrw deleted the td-usage-guide branch April 22, 2026 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant