Rework Reviewer Workflow as a screenshot-driven walkthrough#122
Merged
Conversation
Plans an expanded chapter 12 with a Part 1 walkthrough for first-time GitHub users and a Part 2 reference. Includes the annotation pipeline (Pillow), a demo sequential-reveal GIF, and a one-time Playwright login script for capturing logged-in GitHub UI without committing session state.
Previously the install instructions used 'npm exec playwright install chromium', which only fetches the Chromium browser binary — the Playwright Node package itself was never added to node_modules, so 'import { chromium } from "playwright"' failed with ERR_MODULE_NOT_FOUND. A scoped package.json in scripts/capture/ keeps Playwright out of the root install and gives the login script a node_modules to resolve against.
GitHub blocks Playwright's bundled Chromium with 'this browser or app may not be secure'. Launching the system's real Chrome or Edge through Playwright's channel option avoids the block in nearly all cases. A persistent user-data directory at .playwright-auth/browser-profile/ makes the browser look like a returning user, further reducing detection.
The persistent-context approach with channel:'chrome' still triggered GitHub's 'this browser or app may not be secure' block because Playwright's launcher injects automation flags (navigator.webdriver, --enable-automation) even when the channel is real Chrome. This rewrite spawns Chrome as a plain child process with only --remote-debugging-port and --user-data-dir, then attaches Playwright over the DevTools Protocol. The browser carries no automation fingerprints, so GitHub treats it as an ordinary sign-in. A persistent user-data directory at .playwright-auth/chrome-profile/ avoids re-triggering 'new device' verification on subsequent runs.
Edge is pre-installed on Windows and Chromium-based, so the CDP-attach approach works identically. Avoids conflicting with Chrome instances the user may have open for other work. Renames the user-data dir to be browser-agnostic; renames CHROME_PATH override to BROWSER_PATH.
Loads .playwright-auth/github.json into a headless Playwright context, visits github.com, and reports which account the cookies authenticate as. Used to confirm the login script captured a working session before any capture work begins, and to diagnose failures later (expired session vs. broken storageState).
Captures driven by scripts/capture/capture-pr-pages.mjs against the live sandbox PR #121 with the rmctestreviewer session. Each capture writes a full-page PNG plus a JSON sidecar of element bounding boxes, so annotation and capture can be re-run independently. Annotations use the refined style decided during this batch: USACE-blue highlight boxes with numbered circles at the corners, no on-image labels. The legend mapping numbers to descriptions belongs in the figure caption. Shared primitives factored into scripts/annotate-screenshots/annotate_lib.py. Refines the planning doc with the annotation-style refinement. The files- changed figure masks over a 'Customizable line height' announcement toast that GitHub injected past the capture script's hygiene CSS.
Two issues in the first version of fig-04: 1. A 'Customizable line height' announcement popover leaked past the HYGIENE_CSS into the screenshot. The capture script now uses a Playwright locator (not raw evaluate) to click the popover's Dismiss link with retries — reliable and removes the need for a Pillow white-out mask. 2. The 'All commits' dropdown highlight box was placed on top of the 'Open' pill because GitHub's PullRequestFilesToolbar uses sticky positioning and getBoundingClientRect returns the stuck-state coords rather than the rendered position. The annotation script now hand-picks coords for that element from the rendered image with a code comment explaining why. Also added two new probe scripts that diagnose this kind of layout drift.
Captures the 'Finish your review' submit dialog showing all three radio options (Comment, Approve, Request changes) — used in Section 6 of the chapter to teach what each option does. Six numbered callouts on the dialog: title, text area, three radios, submit button. Also captures the PR #121 preview deploy of the TotalRisk Users Guide v1.1 preface page (without the DRAFT watermark — the watermark is suppressed on previews by design, as clarified by the user; updates Section 4 of the outline accordingly to drop the watermark teaching). Deferred from this batch: comment composer (Section 5) and suggested- change block. GitHub's new diff grid mounts the per-line 'Add a comment' button dynamically on hover via React event listeners, and Playwright's synthetic hover events don't reliably trigger the mount. Worth a follow- up with either CDP-level real mouse events or a different capture strategy (e.g., posting a comment via the API and capturing the inline result). Probe script scripts/capture/probe-composer.mjs documents what the DOM looks like for future investigation.
Every figure script now writes a companion .pptx file to planning/assets/ figure-pptx/. The .pptx contains the cropped, un-annotated source image as a slide background with the highlight rectangles and numbered ovals overlaid as PowerPoint shapes that can be dragged, resized, recolored, or deleted in PowerPoint. When positions are happy, export back to PNG from PowerPoint or hand the new coords to the Python script. annotate_lib.py grew a make_pptx_overlay() helper that handles the EMU/DPI conversion and shape construction. All four existing figure scripts (fig-02 overview, fig-03 commits, fig-04 files changed, fig-08 finish-review) opt in via the new pptx_out keyword arg. Also adds gif-composer-storyboard: a 5-frame stepped GIF (and 5-slide pptx) that walks through the Section 5 comment-composer flow using the Files-changed capture plus drawn-on annotations and a mocked composer overlay. Intended as a springboard for the user to re-record with ScreenToGif against the live PR. Real-capture of the composer remains blocked on GitHub's hover-mounted button behavior; see probe-composer.
The original capture pointed at /pull/121/files (all commits view), which showed 219 files dominated by the scaffolding commit's 200 figure copies. That contradicts the chapter's whole point: reviewers should filter to the content commit so they're not drowning in boilerplate. The capture URL now includes the commit-range filter (85e53a4..7ae2022), which renders the 'Commit 7ae2022' toolbar pill, the 4-file file tree (preface, installation, gui, acronyms), and the 01-preface.mdx diff as the first visible file. Updated fig-04 callouts and the storyboard GIF base to match.
Three deliverables for the parallel Option A / Option B pass: 1. planning/section-1-draft.mdx — working draft of the chapter's opening section with four Mermaid diagrams (repository as a filing cabinet, branch as a parallel working copy, commit as a snapshot, pull request as the formal merge proposal) plus a glossary. Will migrate into the main chapter MDX when the chapter is finalized. 2. fig-05-bot-comments — annotated figure showing the three bot comments every reviewer needs to recognize on the PR (assigned-reviewers state, stage progression, preview deployed). Captured from PR #121 via the existing pr-overview.png; coords merged in via the new probe-bot-comments.mjs. 3. Two storyboard GIFs (with editable .pptx variants): - gif-commit-filter-storyboard: walks through clicking the commit selector pill on the Files toolbar and picking the content commit to filter out the scaffolding diff (Section 3). - gif-changes-since-last-review-storyboard: walks through using the commit selector to pick 'Changes since your last review' on a backcheck round (Section 7). Both storyboards use the existing filtered Files-changed capture as the base, with Pillow-drawn dropdown mocks for the in-between states. The user re-records these in ScreenToGif against the real UI when ready.
Annotated capture of PR #121's preview deploy of the TotalRisk Users Guide v1.1 preface page. Five callouts: top site nav, sidebar with section navigation, breadcrumb showing location in the document, main article body (where the reviewer reads), and the version-1.1 mention in the body that confirms which revision is being previewed. Editable .pptx in planning/assets/figure-pptx/.
After the user authorized and ran scripts/capture/submit-and-capture- review.mjs, a Comment-style review was submitted on PR #121 (no Approve, no stage advancement). The review body is marked '[Documentation training sandbox]' so future viewers of the PR know it was synthetic. fig-09-review-submitted: three callouts on the timeline entry — 1. The 'rmctestreviewer reviewed now' event header 2. The 'View reviewed changes' link that jumps to a Files-changed view of what was reviewed 3. The review comment card showing the body the reviewer typed The submit-and-capture-review.mjs selector for the textarea was tightened to scope inside the dialog (originally picked up GitHub's hidden feedback form). Added probe-review-submitted.mjs to pin sub-element coords inside the post-submit timeline block.
Captures the full state of the rewrite for handoff to another machine or another agent: branches, what's done, sandbox PR state, decisions already made, what remains, and a complete script inventory. Anyone picking this up should read that section first.
New chapter structure: linear, beginner-oriented, built around 16 annotated figures and GIFs captured against the permanent training sandbox PR #121. Covers the full reviewer lifecycle from email notification through submitting the review and backchecking the author's revisions. Adds cross-links into the new chapter from the Author and Site Admin Workflow chapters. Removes the planning artifacts and capture/annotation scripts that produced the figures (history preserves them for future re-captures).
|
📋 Assigned reviewers for this PR
The first approval from any assigned reviewer at the current stage advances the PR. Approvals from non-assigned reviewers are logged but do not advance the stage. |
github-actions
Bot
added
lane:dev
|
📋 Lane: Dev Doc No formal review required. @usace-rmc/docs-admin please review, merge, and approve the deploy. |
|
📄 Preview deployed for commit https://usace-rmc.github.io/RMC-Software-Documentation-Previews/pr-122/ This preview updates automatically when new commits are pushed. Deleted when the PR closes. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
planning/,scripts/capture/, andscripts/annotate-screenshots/folders that produced the figures. Git history preserves them for future re-captures.