feat(scripts): a dense pull request gets a readable front page - #1615
feat(scripts): a dense pull request gets a readable front page#1615fairchild wants to merge 5 commits into
Conversation
scripts/pr-review-page.py builds one self-contained HTML page per PR: the Summary's own sentences as plain language, a diagram of the shape, the diff grouped under the sentence that explains it, the evidence shown rather than linked, and where the PR stands. It reads the PR through gh or a recorded fixture, uploads the page to the evidence store, and writes `Review page: <url>` under the body's byline leaving every other byte alone. The evidence store serves an uploaded .html as text/html, so the page hosts directly; upload-evidence.py just needed the extension and its content type. Grouping is a heuristic and says so: a bullet claims a hunk by the names it shares with what the hunk adds, a cited line range claims outright, and what nothing claims lands under "Everything else". The prose all comes from one function, so the factory turn that writes better sentences (#1614) changes that function and nothing else. Closes #1613 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T3CL23nN7Fmu8mE5TB9CdT
fairchild
added
the
author:claude-code
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![april-clearwater[bot]](https://avatars.githubusercontent.com/in/3095587?s=60&v=4){kind=small}
There was a problem hiding this comment.
April Clearwater, Application Lead
🟡 Approve with follow-ups — solid tool, two things worth tracking
scripts/pr-review-page.py turns a dense PR (the #1602 specimen) into a page a person reads in minutes, and all three requested-evidence items from #1613 are answered with specific, checkable detail rather than assertion.
- Worth noting: the diff handed to me cut off at a line limit partway through the
pr.jsonfixture, so I never saw the actual diffs forscripts/upload-evidence.py,scripts/tests/test_pr_review_page.py, orscripts/tests/test_upload_evidence.py— only the generator itself, the docs, and part of the fixture. My code read below is scoped to what was visible. - Worth noting: the grouping heuristic in
group_hunks/claim_scoreis honest about its own limits (already flagged in the PR body and tracked as a follow-up in #1614) — a bullet claims a hunk by shared tokens with what it adds, which is approximate by design. - Worth noting:
body_with_link's insertion point only inspects the first non-blank line as a possible byline; if that line isn't italic, the link goes at line 1, which is fine for this repo's convention but silent about any other case.
Evidence: ✅ all three items from #1613 are [complete] with specifics — hosted URL plus HTTP/2 200/content-type: text/html, before/after test counts (27 failing → 31 passing), and 52/52 suites exit 0. This is a visual product (a page someone reads) plus python logic, so a screenshot for the former and named test counts for the latter is the right bar, and both are here. I can't fetch the hosted screenshot myself from this runner (no browser/network tool), but the author's own check (curl -D -, byte-identical fetch) is the kind of verification that satisfies the gate.
Code read of pr-review-page.py and docs
claim_score/group_hunks: cited line ranges outrank token overlap, which outranks a bare filename match — reasonable priority order, and ties correctly go to the earlier bullet.body_with_link/write_link: re-reads the live body at write time rather than the one captured at build time, so a concurrent edit isn't reverted — matches the PR's own claim, and the code backs it up.render_diagram: falls back cleanly frommmdc(inline SVG) to a<pre class="mermaid">block with the cdnjs script when the renderer isn't present; no external assets beyond that script, matching the "no PNG fallback needed" summary line._run/subprocess calls: all go throughsubprocess.runwith argument lists (no shell) and explicit timeouts, consistent with thecheck-subprocess-timeouts.pypass reported in Validation.DIAGRAM_RE's comment explaining why it reads to a terminator line rather than the first-->is good self-documentation given mermaid diagrams themselves contain-->.- No blocking findings in what I could read. The parts I couldn't see (
upload-evidence.py's one-line extension addition, and the new test file) are lower-risk by description, but I'm flagging that I didn't verify them directly.
…ermaid fence Where an HTML comment ends is not one answer: a browser ends it at `--!>` as well as at `-->`, and `-->` is also a mermaid edge. A parser reading a comment for content therefore disagrees with the renderer about where that content stopped, and the text after the disagreement is prose to a reader of the PR and diagram source here. The `<!-- review-page:diagram -->` marker above a ```mermaid fence has one reading, and GitHub draws it in the body as well, so it is now the only carrier. CodeQL alert #28 (py/bad-tag-filter) on 7dd7478 removed with the comment-held form. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T3CL23nN7Fmu8mE5TB9CdT
Codex triage — head c0e463bAdjudicated: taken 1, declined 0 — head c0e463b (answering CodeQL alert #28 on 7dd7478)
Gate, bare in the worktree: Flipping ready. Merge is the portfolio steward's under B4, or Michael's click. Orchestrator note (workspaces) |
There was a problem hiding this comment.
April Clearwater, Application Lead
🟡 Approve with follow-ups — CodeQL fix lands clean, same visibility gap as my last pass
Head c0e463b2 replaces the comment-scraping diagram parser with a <!-- review-page:diagram --> ```mermaid fence requirement (DIAGRAM_MARKER_RE), closing the py/bad-tag-filter alert from my earlier round. The diff I can see is sound; the diff I still can't see hasn't changed in nature.
- Worth noting: the diff handed to me again truncates partway through the
1602/pr.jsonfixture, so I still haven't seen the actual diffs forscripts/upload-evidence.py,scripts/tests/test_pr_review_page.py, orscripts/tests/test_upload_evidence.py— same gap as my first review. Lower risk by description (a one-line extension add, and test-only files), but unverified by me directly. - Worth noting: the PR body's
Ran 31 testsline predates this fix commit; the author's own progress comment on this head reportsRan 33 tests ... OK, which is a stronger number than what's printed in the body, not a weaker one — not a concern, just worth reconciling in the body text before merge so the two don't disagree. - Worth noting: the fix comment explaining the marker rationale (
DIAGRAM_MARKER_RE's docstring) is good self-documentation — it names the exact disagreement (-->vs--!>vs mermaid's own edge syntax) that made comment-scraping unsound in the first place.
Evidence: ✅ all three items from #1613 remain [complete] with specifics unchanged from my last pass (hosted URL + HTTP/2 200/text/html, before/after test counts, 52/52 suites). This is a visual product plus python logic, so a screenshot for the page and named test counts for the logic is the right bar, and both are present. The author's own triage comment on this head reports the same gate re-run (33/10/52 passing) plus a rebuild-and-byte-diff check against the hosted #1602 page — that's the kind of self-check that satisfies the gate for a change this size.
What changed since my first review, and why it's sufficient
DIAGRAM_MARKER_REnow requires the mermaid content live inside a fenced code block under the marker, rather than reading raw text up to a terminator line inside an HTML comment. A parser and a browser now agree on where the diagram source starts and ends, which is exactly the class of bug CodeQL'spy/bad-tag-filterflags (regex-based HTML/comment parsing disagreeing with a real renderer).- Nothing else in
pr-review-page.pychanged in the parts visible to me —claim_score/group_hunks,body_with_link,render_diagram'smmdcfallback, and the subprocess calls are identical to what I read before and still hold up. - I can't re-verify the two test files or
upload-evidence.py's extension addition directly, but their described scope (a fence-based fixture update, a one-line MIME addition) doesn't raise new concerns beyond what a greentest_pr_review_page.pycount already covers.
Codex pass — 2026-09-12, head c0e463bA senior read over Blocking
Minor
Checked and cleanPR titles, summary sentences, group headings, filenames, diff lines, evidence captions, author logins, check names and review-thread bodies all pass through Verdict: FINDINGS Not verified
No uploads, no edits, no pushes, no deploys were made in this pass. Steward note (steward v9) |
|
This blocks merge. The codex pass found four issues the c0e463b fix didn't reach: no Content-Security-Policy on a page built from untrusted PR text and served from a shared evidence origin; The "ready" flip in the earlier comment predates this review and should be treated as reversed — this needs another pass, not a merge. Whoever picks it up next should add a CSP meta tag (and push for one on the evidence store's responses too), pin mmdc's version and No code, labels, or PR state changed here; that's for whoever takes the next pass. |
…s offline to images, and reports unavailable as unavailable The page is built from text nobody vetted and served from an origin every other evidence artifact shares, so it now emits its own Content-Security-Policy as the first thing in its head and loads nothing from anywhere: no script, no font, no CDN. The diagram is drawn once at build time in a renderer with no route out to a network, and leaves as a PNG, so nothing a mermaid label asked for reaches a reader. A marker and fence nested inside an outer HTML comment now carry no diagram, because GitHub shows that block to nobody and a page that draws it shows a diagram no reviewer can see. Reviews, review threads and checks report three states rather than two: what is there, that there is none, or that the query failed. Also: only http and https become an href or an img src and anything else is shown as text, generated mermaid labels are escaped for mermaid's own syntax, two dead dataclass fields and two unread API fields are gone, and the uploader's html handling has a test of its own. Codex round 1 on c0e463b: four blocking and five minor taken; the origin's headers are #1616. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T3CL23nN7Fmu8mE5TB9CdT
…, and the fallback has a test of its own `mmdc` is on a developer's machine and not on the hosted runner, so two tests asserted a drawn diagram that CI could not produce and read the documented fallback as a failure. A shared `requires_renderer` skip now gates every test that needs a renderer, the way the no-network test already did. The path CI does take is now a contract rather than an absence: with the lookup forced to find nothing, the page shows the mermaid source escaped inside a `<pre>`, says it did not draw the diagram, and that block carries no `<img>`, `<svg>` or `<script>`. That test needs no renderer, so it runs everywhere. CI run 34674313148 on ae4ee8c: two tests assumed a renderer the hosted runner lacks. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T3CL23nN7Fmu8mE5TB9CdT
Codex triage — head 048487fAdjudicated: taken 9, declined 0, filed #1616 — head 048487f (answering round 1 on c0e463b)
Gate, bare in the worktree: Flipping ready. Merge is the portfolio steward's under B4, or Michael's click. Orchestrator note (workspaces) |
There was a problem hiding this comment.
April Clearwater, Application Lead
🟡 Approve with follow-ups — the codex pass's four blockers are fixed and test-verified
Head 048487fc closes all four blocking findings from the codex security pass (no CSP, network-reaching diagram render with unsanitized SVG, a marker nested in an outer comment still rendering, a failed review-thread query reporting as "none") plus the five minor ones. I read the current scripts/pr-review-page.py, scripts/tests/test_pr_review_page.py, and scripts/tests/test_upload_evidence.py directly from the checked-out branch rather than trusting the gate comment's word for it.
- Worth noting: the PR body's own
## Evidence Status/## Validationtext still reads "Ran 49 tests" from before this fix round; the actual suite at this head is 50 tests (3 skipped whenmmdcis absent) — reconcile the number before merge so the body doesn't disagree with the code it's describing. - Worth noting: the hosted screenshots linked in the PR's
## Evidencesection were captured before048487fc's security fixes (timestamps ~04:05, the fix landed at 05:08) — the page's visual layout didn't change, so this doesn't undermine the visual claim, but a fresh capture would remove the doubt. - Worth noting: #1616 (the evidence store's own response headers) and the review-threads
first:100pagination cap are both real, both correctly named as accepted follow-ups rather than silently ignored.
Evidence: ✅ all three requested items from #1613 are [complete]. This PR mixes a security-isolation claim (needs a test that actually proves the property, not just a green suite) with a visual product (needs a screenshot) and ordinary logic (needs named test counts) — all three bars are met: test_the_renderer_reaches_no_network_during_a_build runs a real loopback listener and asserts zero hits, the specimen screenshot exists and is linked, and I independently counted 50 test methods in test_pr_review_page.py and 12 in test_upload_evidence.py by reading the files — both match the gate comment's numbers exactly.
What I checked against each of the codex pass's four blockers, and the fifth-through-ninth minors
- CSP —
CONTENT_SECURITY_POLICYis defined andbuild_pageemits it as the very first element inside<head>, beforecharsetorviewport.Policy.test_the_policy_is_the_first_thing_in_the_headasserts this by locating the first<meta>in the head and checking it's the CSP one with all four directives present. - Untrusted mermaid reaching the network / unsanitized SVG —
render_diagramnow renders to a PNG viabase64.b64encode, never readsout_fileas text, andPUPPETEER_ARGSroutes everything (including loopback, via--proxy-bypass-list=<-loopback>) through a proxy that doesn't exist.test_the_renderer_reaches_no_network_during_a_buildstands up a real HTTP listener on loopback, puts its URL in a diagram label, and asserts the listener saw zero hits — this is exactly the class of proof that "the named tests passed" alone wouldn't give me. - Comment-nested marker —
_marker_is_its_own_commentwalksCOMMENT_RE.finditer(body)and only accepts a marker match whose start coincides with a comment's own start, rejecting one where a comment's span merely contains it.test_a_marker_nested_in_an_outer_comment_carries_nothingreproduces the codex-provided repro nearly verbatim and confirmsdiagram_sourcefalls back to the generated graph, notpayload. - Failed queries reporting as "none" —
pr.get("reviews") is None,source.threads is None, andpr.get("statusCheckRollup") is Noneare each checked before falling to the "no X" branch, all emittingUNAVAILABLE. Five tests inUnavailablecover both the raising path (read_threadscatchingRuntimeError/SubprocessError/JSONDecodeError) and the empty-vs-missing distinction for all three fields. - Scheme check —
_safe_urlnow doesurlsplit(url).scheme.lower()againstSAFE_SCHEMES = {"http", "https"}rather than astartswithprefix test.test_the_scheme_gate_is_a_parse_not_a_prefixdirectly proveshttpjavascript:now fails where a prefix test would pass it. - CDN mermaid fallback removed —
render_diagram's no-renderer branch shows escaped source text and nothing else; there's no secondmmdc-failure path that reaches for cdnjs anymore.test_without_a_renderer_the_page_shows_the_source_and_says_soasserts no<img>,<svg>, or<script>in that block. - Escaped diagram labels —
_mermaid_labelentity-escapes& " < > [ ]before interpolation.test_a_hostile_filename_cannot_escape_a_mermaid_labeluses a filename containing"]--x[and confirms it can't close a label early. - Escaping test coverage — the
Escapingclass now covers a hostile title, a hostile group heading, a hostile filename in a diagram label, markup in a diff hunk, and a hostile evidence caption — five targeted cases where before there were two generic ones. - Dead code —
Bullet.textandGroup.bulletare gone from both dataclasses;PR_FIELDSno longer requestscommentsorlabels. Confirmed by reading the file directly.
Nothing else in the visible code changed shape since my last pass: claim_score/group_hunks's honest-heuristic scoring, body_with_link's live-body re-read, and the subprocess timeout discipline are all unchanged and still hold up.
|
Verdict: FINDINGS on 048487f Codex pass, round 2 of 2 — head 048487fRound 2 of the two-round bound, on head 048487f, after the nine round-1 items on c0e463b were taken (the adjudication at 05:08:36Z). Codex (gpt-5.6-sol, reasoning xhigh) read the diff against round 1 and the adjudication; every claim below was reproduced in a clean worktree at this head before it was written down, and where the codex run overstated something, the corrected version is what appears here. There is no round 3: the in-fence findings are the repo steward's to adjudicate on this PR with an Round 1, item by item
Findings
On the offline proof without the local runWhat the tests prove, on a machine with Outside the fence, for the repo steward to file
Checked and cleanEvery PR-derived string in the page passes through No uploads, no edits, no pushes, no deploys were made in this pass. Steward note (steward v9.6) |
|
Round 2 leaves two blocking items in the fence: local files on the build host reaching the diagram PNG through mmdc's The seven minor findings (fence-hidden markers, head drift between the two |
… and reads nothing from inside comments or fences mmdc renders from a `file://` page, so a bare path in an image shape or an HTML label resolves on the machine doing the build and is rasterised into the PNG the page publishes: any image that user can read at a guessable path could be carried into a hosted page by a PR body. Closing it by sanitising the render is a race against mermaid's feature list, so an authored fence is now drawn only if its syntax is on a strict allowlist — flowchart, graph, sequence and state headers, plain ids, edges, and labels with no markup, no path and no directive. Anything else is shown as escaped source, which costs a picture and keeps the filesystem out of the page. Mermaid's own `securityLevel: strict` and `htmlLabels: false` are passed with `secure` pinning both, as the layer behind that one. The generated graph does not go through the allowlist: its labels are file paths and are escaped by `_mermaid_label` instead. Content inside an HTML comment is content GitHub shows to nobody, so the page no longer reads it either: sections, bullets, evidence and the closing reference all come from a comment-stripped body, and a marker inside a comment, a fenced example, a CDATA block or a processing instruction draws nothing. Also: the head is re-read before the upload and before the body edit and a head that moved refuses both; a malformed review-thread payload reads as unavailable rather than crashing, and the page states the 100-thread cap when it hits it; `body_with_link` rewrites only a line above the first heading and outside comments and fences; images are admitted only for `https:` on the evidence store or GitHub, which is what the page's policy now names instead of all of https. Codex round 2 on 048487f: both blocking taken; minors as recorded. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T3CL23nN7Fmu8mE5TB9CdT
Codex triage — head d4fa12aAdjudicated: taken 9, declined 0, filed #1618 — head d4fa12a (answering round 2 on 048487f)
Gate, bare in the worktree: Flipping ready. The portfolio steward runs its confirmation pass on the diff from 048487f; merge is Michael's click. Orchestrator note (workspaces) |
There was a problem hiding this comment.
April Clearwater, Application Lead
🟡 Approve with follow-ups — the two round-2 blockers read as fixed in the diff; same visibility gap as my last three passes
Head d4fa12ab (per the latest triage comment) closes both blocking findings from the codex round-2 pass — a file://-origin diagram renderer that could pull local build-host images into a published PNG, and comment-hidden Summary/evidence content surfacing on the page while GitHub shows nothing for it. I read the current scripts/pr-review-page.py diff directly to check both.
- Worth noting:
MERMAID_FORBIDDENnow refusesimg,image,href,click,@{,file:,/,\, and..anywhere in an authored fence's lowercased source, so a diagram naming any path — an image shape or an HTML-label<img>— is shown as escaped source instead of rendered. That's a real fix for thefile://-origin path finding 1 named, not a narrower sanitizer. - Worth noting:
body_sections,summary_bullets,evidence_items, andcloses_issueall now readvisible_body(body)(comments stripped) rather than the raw body;authored_diagram's marker scan correctly still uses the raw body, since it needs to know where comments are._quoted_line_numbersandbody_with_link's fence/comment-aware insertion point match the round-2 minors (3 and 6). - Worth noting: same gap as my last three reviews on this PR — the diff handed to me truncates before
scripts/upload-evidence.py,scripts/tests/test_pr_review_page.py, andscripts/tests/test_upload_evidence.py, so I can confirm the fixes exist in the generator but not that a test asserts each one (the filesystem-refusal case, the comment-hidden-content case, the puppeteer-wiring assertion round 2's minor 9 asked for). The gate comments report matching test counts (Ran 72,OK/OK (skipped=6)), which is consistent with the generator's growth since my last pass, but it's their word, not something I read myself.
Evidence: ✅ all three requested items from #1613 remain [complete] with specifics (hosted URL + status/content-type, before/after test counts, 52/52 suites). This PR mixes a security-isolation claim (needs a test proving the property) with a visual product (screenshot) and ordinary logic (named test counts); the first two bars are met on what I can read, the third — the specific tests for round 2's fixes — I can't independently confirm because those files are outside the diff shown to me.
What I checked against each round-2 blocker, and what I couldn't
-
Local files reaching the diagram PNG (blocking, round 2 finding 1) —
is_renderable_mermaidrefuses any authored source containingimg,image,href,click,@{,%%{,file:,/,\, or..(whole-source, lowercased), plus a per-line skeleton check once labels are stripped. Combined withMERMAID_CONFIG's pinnedsecurityLevel: strict/htmlLabels: False/securelist and the existingPUPPETEER_ARGSnetwork closure, this is layered refusal rather than sanitization — matches the PR's stated fix and the triage comment's description. I can't run mmdc myself to reproduce the magenta-pixel probe codex describes, but the code path that would have let it through is now gated by a syntax check, not a runtime sanitizer trying to catch every renderer feature. -
Comment-hidden content everywhere but the diagram (blocking, round 2 finding 2) —
visible_body()stripsCOMMENT_REmatches once, and every prose-reading function (body_sections,summary_bullets,evidence_items,closes_issue) now calls it before scanning. The one function that still reads the raw body isauthored_diagram, correctly, since it needs the comment boundaries to reject a nested marker.
3–9 (minors)** — refuse_if_head_moved is called after the diff fetch in read_pr, and again from main() before --upload/--link; read_threads's except clause now includes KeyError/TypeError; body_with_link only considers lines before the first ## heading and outside comment/fence extents; IMAGE_SCHEMES = {"https"} and _safe_image_url checks both scheme and IMAGE_HOSTS membership, matching the CSP's img-src allowlist. All read consistent with what the round-2 report and the 06:05:53 triage claim was taken.
Nothing else changed shape since my last pass: claim_score/group_hunks's heuristic scoring and the subprocess timeout discipline are unchanged and still hold up.
|
Verdict: CLEAR on d4fa12a Confirmation pass — head d4fa12aThis is the confirmation pass after the two-round bound: round 1 on c0e463b, round 2 on 048487f, adjudicated at 06:05:53Z with Round 2, item by item
Residuals, for the repo steward to file (none blocking)R1. R2. The "Only the first 100 review threads were read" line ( R3. R4. Hardening, not a defect: the allowlist's token refusal can be walked around with entities in the text positions that admit Nothing newThe three files add no script, sink or subprocess beyond the ones round 2 read. Every new string reaching the page passes RunsIn a clean worktree at d4fa12a, Python 3.14.4. The test file declares
No uploads, no edits, no pushes, no deploys were made in this pass. Steward note (steward v9.6) |
|
R1 deserves more than a residual note. The fence-closer regex only checks that a line starts with a backtick or tilde run of any length — it doesn't require the closer to match or exceed the opener's length. CommonMark does require that. So a three-backtick fence closed early by a shorter run stays open on GitHub but closes here, which is the same content-hiding mismatch this PR closed twice already for comments and CDATA, just in the fence-length dimension. Given two full review rounds were spent on exactly this failure mode, R1 reads as a third instance of it rather than ordinary polish, and it's a one-line fix ( R2 and R3 are cosmetic and fine to leave for follow-up. R4 is worth a one-line comment fix at The CLEAR verdict otherwise stands — nothing here reopens rounds 1 or 2. Whoever holds merge authority should either fold R1's one-line fix into this PR before flipping ready, given it's the same bug class as the two blocking findings this PR exists to fix, or file it as a tracked follow-up issue explicitly scoped as "round 3 of the same defect" rather than a generic residual, so it doesn't get deprioritized as polish. |
2 participants
Summary
scripts/pr-review-page.pybuilds one self-contained HTML page per pull request — plain language, a diagram, the diff grouped by concern, the evidence inline, and where it stands — fromghor from a recorded fixture.Content-Security-Policyas the first thing in its<head>, loads nothing from anywhere, and runs no script at all. The origin's own headers are The evidence store serves uploaded HTML as active pages on its shared origin with no content security policy #1616.file://page, so a bare path in an image shape or an HTML label would resolve on the build host and be rasterised into the published image.d13afd8eis hosted and linked from that PR's body: https://evidence.cloudcompute.com/workspaces/pr-1602/jGQMzcy4LMmZuJzoJACaZw/20260912-055959-pr-review-1602.htmlplain_language, so the factory turn in Factory step: build, host, and link the PR review page at every ready flip of an April PR #1614 rewrites one function. Closes A readable front for a dense PR: a generated review page per factory PR, #1602 as the specimen #1613Mergeability
scripts/pr-review-page.py(new),scripts/upload-evidence.py(one extension and its content type), fixtures and tests underscripts/tests/Review page:line, and.htmlis an uploadable evidence type.http:andhttps:become anhref, and an<img>only forhttps:on the evidence store or GitHub, which is what the page's own policy allows — anything else is shown as its address in text; the head is re-read before the upload and before the body edit, and a head that moved refuses both; a hostile filename cannot close a mermaid label;--headrefuses to build a page that would claim a head the PR does not have;--linkre-reads the body at write time and rewrites its own line rather than adding a second; with no renderer present the page shows the escaped diagram source and says it did not draw it.--uploadneedsEVIDENCE_UPLOAD_TOKEN, whichevidence.shalready sources.<meta>policy is the only layer until that lands. The grouping heuristic can file a hunk under a neighbouring sentence (Give WorkspaceServiceTests each their own preferences domain instead of UserDefaults.standard #1602: 5 hunks under the first bullet, 22 under the second, 1 under the third), which Factory step: build, host, and link the PR review page at every ready flip of an April PR #1614's model turn is what fixes. The review-thread query reads the first 100 and does not page; the page says so when it hits that cap.--disable-remote-fontsis documented in the renderer's flags as intended-but-absent: mermaid-cli 11.12 fails every render with it, and a closed network denies remote fonts anyway..gitattributesin the fixture directory turns off the whitespace check there, because a recorded diff's blank context line is a single space.Validation
uv run --script scripts/tests/test_pr_review_page.py—Ran 72 tests in 29.647s/OK; withmmdcoffPATH, the path CI takes,Ran 72 tests in 0.050s/OK (skipped=6)uv run --script scripts/tests/test_upload_evidence.py—Ran 12 tests in 0.009s/OKfor f in scripts/tests/test_*.py; do uv run --script "$f"; done— 52 suites, every one exit 0python3 scripts/check-subprocess-timeouts.py—OK: no un-timed ProcessRunner.run calls in 318 filesuv run --script scripts/pr-readiness.py --body-file <this> --base origin/main—PR readiness passed.git diff --cached --check— cleancurl -s -D -on the hosted specimen —HTTP/2 200,content-type: text/html; charset=utf-8; the fetched bytes are identical to the local build<svg>elements and zero<script>tagsPerformance
Evidence
The specimen page, fetched from its hosted URL and rendered in Chrome: https://evidence.cloudcompute.com/workspaces/pr-1613/27-GHucz5wfotPnTe0G5Sg/20260912-060005-specimen-1602-review-page-browser-v3.png
The synthetic fixture's page, which covers an inline evidence image, an open review thread, a red check and the "Everything else" group: https://evidence.cloudcompute.com/workspaces/pr-1613/dHRx2kNMb6g9fFCozX2p3A/20260912-040055-synthetic-fixture-page-browser.png
Evidence Status
HTTP/2 200,content-type: text/html; charset=utf-8, fetched bytes identical to the local build); linked from Give WorkspaceServiceTests each their own preferences domain instead of UserDefaults.standard #1602's body, where the diff against the body captured beforehand is theReview page:line and nothing else; rendered from that URL in Chrome at https://evidence.cloudcompute.com/workspaces/pr-1613/27-GHucz5wfotPnTe0G5Sg/20260912-060005-specimen-1602-review-page-browser-v3.pngscripts/tests/test_pr_review_page.pycovering the page build from a recorded PR fixture, red before the generator exists -- the fixture is Give WorkspaceServiceTests each their own preferences domain instead of UserDefaults.standard #1602's owngh pr view --jsonandgh pr diff, recorded underscripts/tests/fixtures/pr-review-page/1602/; run against an absent generator it printedRan 27 tests in 0.006s/FAILED (failures=27), every failure readingthe generator does not exist yet, and it now printsRan 72 tests in 29.647s/OKscripts/tests/*.pypasses underuv run --script--for f in scripts/tests/test_*.py; do uv run --script "$f" || echo "FAILED $f"; doneran all 52 suites with exit 0 each and printed no FAILED lineBlockers
🤖 Generated with Claude Code
https://claude.ai/code/session_01T3CL23nN7Fmu8mE5TB9CdT
Orchestrator note (workspaces)