Add /address-review: work every pre-merge review finding to a decision

[CamSoper]

Proposed changes

The review pipeline is good at finding things and has no way to make anyone look at them. scrape-review-outcomes.py carries an ignored_low_confidence column precisely because authors clear 🚨 and stop reading. This adds the author-side counterweight: a skill that works a PR's pinned review to zero, and the meta-file changes that make doing so the expectation rather than an option.

New skill — /address-review (.claude/commands/address-review/)

Watches for the pinned review to land after a PR goes ready for review, then walks every item with the author — 🚨 blockers, ⚠️ low-confidence, and the ✏️ inline style suggestions — one at a time, with a proposed fix for each. Fixes batch into a single push (keeping the shape auto-refresh-gate.py recognizes), disputes and fix-responses go out in one @claude #update-review mention, and the skill won't call a PR done while anything is undecided. It's also explicitly pushy: it offers to watch at ready-for-review without being asked, and says once — with a count — when a merge would leave items open. It never blocks a merge the user has decided on; "just merge it" becomes accepted with the user's reason on each open item, so the post-merge scrape reads the truth.

Two references: watching.md (event subscription where available, bounded polling otherwise, plus the short-circuit labels that mean no review is coming at all) and dispositions.md (the closed set — fixed / refuted / deferred / accepted / not-applicable, the last three note-required).

New script — review-worklist.py (.claude/commands/docs-review/scripts/)

The deterministic half. Parses the pinned body through validate-pinned.py's helpers so the comment format keeps exactly one parser, merges in the inline one-click suggestions, assigns stable per-finding ids (outstanding:L40-50, style:content/docs/a.md:L88), and answers "is anything still open?" via --require-clean. Splits the ✏️ style block out of the ⚠️ section so advisory polish isn't miscounted as low-confidence findings, and never reports an unparseable body as an all-clear. State lives in a gitignored .review-worklist-<PR>.json, so a lost session resumes instead of restarting.

Meta changes

• AGENTS.md §PR Lifecycle — shipping a PR includes finishing its review; names the skill and the --require-clean check. (Also completes a sentence that had been left truncated mid-clause.)
• CONTRIBUTING.md — new §Working the review to zero: the five outcomes, and why a fix that never refreshed the review scrapes as ignored.
• /shipit — new mandatory Step 9 hands off to the review loop (8 → 9 steps, counters renumbered). PRs now open as drafts, matching CONTRIBUTING §Draft-first, which is what Step 9's ready-for-review offer assumes.
• /pr-review — Step 6 now discloses what an approval would be merging over; disclosure, not a gate, and the maintainer's own PRs route through /address-review first.
• /docs-review — one line pointing at the post-push half.

Testing

./scripts/test-review-pipeline.sh passes; the runner's --self-test discovery picks up review-worklist.py automatically (no change needed there). Also verified against the real pr20079-pinned-review.md fixture and confirmed /address-review is discovered by docs-tools/scripts/scrape-metadata.py. make lint is unaffected — nothing here is under content/, and prettier ignores *.md.

Related issues (optional)

None.

---

Generated by Claude Code

[unblocked]

✅ No issues found

About Unblocked

Unblocked has been set up to automatically review your team's pull requests to identify genuine bugs and issues.

📖 Documentation — Learn more in our docs.

💬 Ask questions — Mention @unblocked to request a review or summary, or ask follow-up questions.

👍 Give feedback — React to comments with 👍 or 👎 to help us improve.

⚙️ Customize — Adjust settings in your preferences.

[pulumi-bot]

Your site preview for commit 7f15832 is ready! 🎉

http://www-testing-pulumi-docs-origin-pr-21061-7f158321.s3-website.us-west-2.amazonaws.com

[github-actions]

Pre-merge Review — Last updated 2026-08-21T23:15:33Z

Tip

Summary: Agent-tooling PR, not reader-facing content: it adds the /address-review skill (SKILL.md plus dispositions and watching references), a new deterministic enumerator review-worklist.py, a mandatory Step 9 in /shipit, an unresolved-findings disclosure in /pr-review, and the AGENTS.md + CONTRIBUTING.md prose that makes "work the review to zero" the house expectation. Nothing here renders on the site, so the wrongness that matters is behavioral: an enumerator that under-reports items, or --require-clean answering "clean" when it hasn't actually seen everything, would quietly defeat the exact failure mode the PR exists to close. 7f158321 closes both remaining notes. fetch_inline_suggestions now fails closed on a partial pagination decode — failed_any is set by a JSONDecodeError and by valid-JSON-that-isn't-a-page, so a short list can no longer ride out as ok=True while still being returned for inspection. And the bullet-recognition regex is now one object: FINDING_START_RE is exported from validate-pinned.py and referenced by its two internal call sites, by scrape-review-outcomes.py, and by review-worklist.py's _bullet_blocks; a repo-wide search at HEAD finds exactly one re.compile of that pattern. The docstring now describes what is actually shared (same recognition rule, wider capture) instead of naming a helper that had stopped being called. Nothing outstanding.

Review confidence:

Dimension	Level	Notes
mechanics	HIGH
facts	HIGH	Cross-file references (validate-pinned.py helpers, CONTRIBUTING.md §Draft-first pull requests, the --self-test glob in scripts/test-review-pipeline.sh) all resolve.
code correctness	MEDIUM	The round-2 changes were read against their contracts and the single-definition claim confirmed by a repo-wide search at HEAD (git grep finds one re.compile of the bullet pattern, in validate-pinned.py L209), and the four new --self-test cases were read line by line — but the interpreter is still unavailable in this environment, so nothing was executed. The author reports ./scripts/test-review-pipeline.sh green and 184 pytest cases passing on 7f158321.

Investigation log

• Cross-sibling reads: not run (not in a templated section)
• External claim verification: carried from the initial review (43 claims extracted, 5 verified, 0 contradicted); the round-2 commit touches code, comments, and docstrings only, so no new external claims were extracted
• Cited-claim spot-checks: not run (no cited claims)
• Frontmatter sweep: not run (no frontmatter in diff)
• Temporal-trigger sweep: ran (no new recency words in the round-2 diff)
• Code execution: not run (no static/programs/ change; the embedded --self-test could not be executed — no interpreter available in this environment)
• Code-examples checks: ran (3 specialists: structural, existence, body-code-coverage); 0 findings
• Editorial-balance pass: not run (not under content/blog/)

🚨 Outstanding	⚠️ Low-confidence	💡 Pre-existing	✅ Resolved
0	0	0	6

🔍 Verification trail

43 claims extracted · 5 verified · 0 unverifiable · 0 contradicted

• L12 in .claude/commands/address-review/SKILL.md "The review pipeline is good at finding things and has no way to make anyone look at them. The measured failure mode is real: scrape-review-outcomes.py…" → ✅ verified (evidence: (escalated from pass3) scrape-review-outcomes.py defines the outcome taxonomy including ignored_low_confidence — still in ⚠️ Low-confidence when the PR merged, confirming the script does track this outcome precisely as the claim states.; source: repo:.claude/commands/docs-review/scripts/scrape-review-outcomes.py (lines 19, 114, 325))
• L27 in .claude/commands/address-review/SKILL.md "Worklist state lives in .review-worklist-<PR>.json at the repo root (gitignored). It survives context loss: a fresh session with --resume picks up every…" → ➖ not-a-claim (evidence: (escalated from pass3) This is a faithful description of the PR author's own tooling design, corroborated internally: .gitignore contains /.review-worklist-*.json with the comment "Author-side worklist state for /address-review...…; source: repo:.gitignore (lines 141-145), repo:.claude/commands/address-review/SKILL.md)
• L36 in .claude/commands/address-review/SKILL.md "1. On ready-for-review. Offer to watch, with AskUserQuestion: Watch for the review (recommended) / Ping me when I ask / *Skip — I'll merge without…" → ➖ not-a-claim (evidence: (escalated from pass3) The text is a verbatim line from the SKILL.md file itself, describing the skill's own designed workflow (an AskUserQuestion prompt with three options). This is a faithful self-description of the PR author's own…; source: .claude/commands/address-review/SKILL.md)
• L38 in .claude/commands/address-review/SKILL.md "1. Whenever the user moves to merge with items still open. Say so plainly, once, with the count and the shortest path to clearing it: *'3 findings still…" → ➖ not-a-claim (evidence: This is example/template prompt text within a Claude Code skill definition file (.claude/commands/address-review/SKILL.md), describing the author's own designed interaction pattern ("3 findings still open..." is illustrative phrasing)…; source: .claude/commands/address-review/SKILL.md (reviewed file, self-describing its own workflow design))
• L46 in .claude/commands/address-review/SKILL.md "Steps 1-3 are mostly silent. Step 4 is the skill." → ➖ not-a-claim (evidence: This is a descriptive line in the PR author's own internal skill/command documentation file describing their own workflow steps, not a falsifiable external claim that a public source could verify.; source: .claude/commands/address-review/SKILL.md)
• L60 in .claude/commands/address-review/SKILL.md "| review:in-progress | Workflow running now | Step 2 (watch) |" → ➖ not-a-claim (evidence: This is a table row from the PR author's own internal Claude Code skill/workflow documentation describing a custom label state (review:in-progress) and its associated internal step, not a falsifiable external fact.; source: .claude/commands/address-review/SKILL.md)
• L61-63 in .claude/commands/address-review/SKILL.md "| review:no-blockers | Review posted, 🚨 == 0 | Step 3 — ⚠️ and ✏️ items still need dispositions |" → ➖ not-a-claim (evidence: This is a table row from the PR author's own internal Claude Code skill/workflow documentation describing label semantics (e.g., review:no-blockers) used in their own process. It's a faithful description of the author's own design, not…; source: .claude/commands/address-review/SKILL.md (self-authored workflow doc))
• L64 in .claude/commands/address-review/SKILL.md "| review:error | Workflow failed before publishing | Check the Actions run; @claude #update-review to retry |" → ➖ not-a-claim (evidence: This is a row in an internal documentation table describing the repo's own Claude-based review-automation workflow (label meaning and retry instructions), not a falsifiable external assertion.; source: .claude/commands/address-review/SKILL.md)
• L65 in .claude/commands/address-review/SKILL.md "| review:trivial / review:frontmatter-only / review:oversized | Full review short-circuited | No pinned comment. If review:prose-flagged is also set…" → ➖ not-a-claim (evidence: (escalated from pass3) The line is a verbatim row from the same SKILL.md file's own reference table describing the authors' own address-review workflow/label semantics. It's a faithful self-description of the PR author's own tooling…; source: .claude/commands/address-review/SKILL.md:65)
• L67 in .claude/commands/address-review/SKILL.md "Verify freshness even on a CURRENT-looking label: pushes made with GITHUB_TOKEN or by a coding agent don't fire synchronize, so review:stale can be…" → ✅ verified (framing: GitHub docs state the general rule that GITHUB_TOKEN-triggered pushes don't fire new workflow runs (including push-triggered synchronize-equivalent events)…; evidence: GitHub's own docs confirm the underlying mechanism: "if a workflow run pushes code using the repository's GITHUB_TOKEN, a new workflow will not run even when the repository contains a workflow configured to run when…; source: https://docs.github.com/en/actions/concepts/security/github_token)
• L72 in .claude/commands/address-review/SKILL.md "| grep -oE '' | tail -1 | grep -oE '[0-9a-f]{7,40}')" → ➖ not-a-claim (evidence: This is a shell/grep command fragment extracted from an internal Claude Code skill script, used to parse a git-hash marker from a review comment. It is not a falsifiable factual assertion, just implementation code.; source: .claude/commands/address-review/SKILL.md L72)
• L79 in .claude/commands/address-review/SKILL.md "Follow address-review:references:watching. It covers both environments (event subscription where available, bounded polling otherwise), what to do while…" → ➖ not-a-claim (evidence: This is an internal reference within the PR author's own Claude Code skill/command documentation (.claude/commands/address-review/SKILL.md) describing its own internal cross-reference structure, not a third-party or externally verifiable…; source: .claude/commands/address-review/SKILL.md (self-referential internal doc))
• L88 in .claude/commands/address-review/SKILL.md "The script enumerates every item needing a disposition and assigns each a stable id (outstanding:L40-50, low:L12, style:content/docs/a.md:L88…" → ➖ not-a-claim (evidence: This is a description of the PR author's own internal tooling (a Claude Code skill script for triaging review comments) in a repo-local .claude/commands file, not a checkable external/product claim.; source: .claude/commands/address-review/SKILL.md)
• L90 in .claude/commands/address-review/SKILL.md "If parse_confidence comes back low, do not proceed as if the list were complete — read the pinned comment yourself and work from it, saying that the…" → ➖ not-a-claim (evidence: This is a procedural instruction within the PR author's own SKILL.md describing their internal review-automation workflow (handling a parse_confidence field from their own enumerator tool), not a falsifiable external claim.; source: .claude/commands/address-review/SKILL.md (self-described internal pipeline))
• L107 in .claude/commands/address-review/SKILL.md "Then AskUserQuestion with the dispositions that plausibly apply to this item, drawn from the closed set in address-review:references:dispositions…" → ➖ not-a-claim (evidence: (escalated from pass3) This is a description of the PR author's own internal workflow design (the address-review skill's use of AskUserQuestion and its 5-item closed set of dispositions, worked around the tool's 4-option limit) — not a…; source: .claude/commands/address-review/SKILL.md (lines 107, 114))
• L112 in .claude/commands/address-review/SKILL.md "- Verify the finding before proposing a fix. The review can be wrong; a fix applied to a false finding is worse than the finding. When you believe it's…" → ➖ not-a-claim (evidence: This is a line from an internal Claude Code skill/workflow document describing the author's own recommended process for handling code review findings (verify before fixing, use "Refute" for disputes). It's a design/process statement, not…; source: .claude/commands/address-review/SKILL.md)
• L144 in .claude/commands/address-review/SKILL.md "Loop Steps 4-6 until --require-clean passes:" → ➖ not-a-claim (evidence: (escalated from pass3) This is an instructional step in the skill's own workflow ("Loop Steps 4-6 until --require-clean passes") describing the author's own process, not a falsifiable external assertion.; source: .claude/commands/address-review/SKILL.md:144)
• L15 in .claude/commands/address-review/references/dispositions.md "| accepted | Knowingly shipping as-is | yes | The note (and, for a blocker, a PR comment) |" → ➖ not-a-claim (evidence: This is a line from an internal process/documentation table defining the PR author's own tooling behavior (disposition types for the address-review command), not a falsifiable external claim.; source: .claude/commands/address-review/references/dispositions.md)
• L16 in .claude/commands/address-review/references/dispositions.md "| not-applicable | The finding misreads the change; nothing to do and nothing to argue | yes | The note |" → ➖ not-a-claim (evidence: This is a row in a disposition table defining internal semantics for Pulumi's own .claude/commands/address-review tooling - a faithful description of the PR author's own design, not a checkable external/factual assertion.; source: .claude/commands/address-review/references/dispositions.md)
• L28 in .claude/commands/address-review/references/dispositions.md "- For an inline ✏️ one-click suggestion: either the user clicks it in the Files-changed tab or you edit the line locally. Never both — the second one…" → ➖ not-a-claim (evidence: This is internal process documentation describing the PR author's own workflow/tooling design (a Claude Code slash-command reference for handling GitHub review suggestions), not a falsifiable third-party technical assertion.; source: .claude/commands/address-review/references/dispositions.md)
• L37 in .claude/commands/address-review/references/dispositions.md "- Verifiable claim ('that was added in v3.0', 'the docs already say this elsewhere') — author authority proves nothing here. Bring the link, the…" → ➖ not-a-claim (evidence: This is internal documentation for a Claude command's review-response guidelines, using "v3.0" purely as a hypothetical illustrative example of a verifiable claim type, not an actual assertion about any Pulumi product version.; source: .claude/commands/address-review/references/dispositions.md)
• L54 in .claude/commands/address-review/references/dispositions.md "- The note must say why, in terms someone reading the PR later can evaluate: 'house voice — we say 'simply' in tutorials deliberately', not 'won't fix'." → ➖ not-a-claim (evidence: This is internal guidance text within Pulumi's own .claude/commands review tooling documentation, describing the author's own prescribed convention for writing disposition notes (an example house-style phrase). It is not a falsifiable…; source: .claude/commands/address-review/references/dispositions.md (self-authored guidance, not a third-party factual claim))
• L55 in .claude/commands/address-review/references/dispositions.md "- For a 🚨 blocker, also post the reason as a PR comment. A blocker accepted silently reads to the scraper as ignored_outstanding, and to a maintainer as an…" → ➖ not-a-claim (evidence: This is internal documentation describing the PR author's own tooling/process design (a Claude Code command reference for handling review dispositions and how their own scraper interprets them) — not a falsifiable external fact.; source: .claude/commands/address-review/references/dispositions.md (self-describing internal tooling doc))
• L62 in .claude/commands/address-review/references/dispositions.md "- The note says what the finding actually points at and why nothing follows from it." → ➖ not-a-claim (evidence: This line is descriptive guidance text within the PR author's own internal tooling documentation (.claude/commands/address-review/references/dispositions.md), describing the author's own design convention for writing disposition…; source: .claude/commands/address-review/references/dispositions.md)
• L72 in .claude/commands/address-review/references/dispositions.md "- 💡 Pre-existing — optional by construction: not introduced by this PR and not the author's debt. Ask once whether to include them, default no, and…" → ➖ not-a-claim (evidence: This line is internal design documentation for the PR's own review-automation tooling (dispositions.md defines the closed set of worklist outcomes), not a falsifiable external claim; the text is self-consistent with the rest of the same…; source: repo:.claude/commands/address-review/references/dispositions.md)
• L88 in .claude/commands/address-review/references/dispositions.md "Write it as each decision is made, not at the end. The file is what makes --resume work after a lost session, and what --require-clean reads to answer…" → ➖ not-a-claim (evidence: (escalated from pass3) This is a faithful description of the PR author's own internal tooling design (the .review-worklist-<PR>.json state file used by --resume and --require-clean), documented in the same file that defines the…; source: repo:.claude/commands/address-review/references/dispositions.md)
• L8 in .claude/commands/address-review/references/watching.md "The pre-merge review is a GitHub Actions job, not something you can block on. claude-code-review.yml gives the job a 40-minute ceiling and the model step…" → ✅ verified (evidence: (escalated from pass3) The workflow source confirms both anchors exactly: job-level timeout-minutes: 40 ("It's sized so the Opus step's budget survives pre-step variance... under the old job-level-only 25-min timeout") and the…; source: gh_query repos/pulumi/docs .github/workflows/claude-code-review.yml (read via read_file pattern match, lines 219-232, 1013))
• L17 in .claude/commands/address-review/references/watching.md "| Trivial short-circuit | review:trivial | No pinned comment is coming. If review:prose-flagged is also set, triage posted an advisory comment — walk…" → ➖ not-a-claim (evidence: (escalated from pass3) This is a line from the PR's own internal automation reference (.claude/commands/address-review), describing the authors' own designed behavior for the review:trivial label short-circuit and its interaction with…; source: repo:.claude/commands/address-review/references/watching.md)
• L18 in .claude/commands/address-review/references/watching.md "| Frontmatter-only | review:frontmatter-only | Same as above. |" → ➖ not-a-claim (evidence: This is a line from an internal Claude Code command reference table documenting the author's own review-label naming scheme (review:frontmatter-only), not a falsifiable external fact.; source: .claude/commands/address-review/references/watching.md)
• L19 in .claude/commands/address-review/references/watching.md "| Oversized | review:oversized | Triage posted a <!-- TRIAGE_OVERSIZED --> advisory suggesting a split. Offer to split the hand-written source into its…" → ➖ not-a-claim (evidence: This is a description of the PR author's own internal automation/tooling convention (a .claude/commands workflow file documenting a label and triage marker comment used by their own review-automation pipeline), not a falsifiable…; source: .claude/commands/address-review/references/watching.md)
• L21 in .claude/commands/address-review/references/watching.md "| review:error | Workflow failed before publishing | Watching won't help. Read the Actions log; @claude #new-review reruns from scratch. |" → ➖ not-a-claim (evidence: This is a line from the PR author's own internal documentation describing their own /address-review tooling and its labeling/retry conventions (e.g. @claude #new-review reruns from scratch). It's a design description of the author's…; source: .claude/commands/address-review/references/watching.md)
• L25 in .claude/commands/address-review/references/watching.md "When the session has PR activity subscription available (Claude Code on the web and other remote sessions expose subscribe_pr_activity), use it:" → ✅ verified (evidence: (escalated from pass3) Multiple independent repos' Claude Code integration docs confirm subscribe_pr_activity/unsubscribe_pr_activity as tools exposed in remote/web Claude Code sessions for subscribing to GitHub PR events, e.g.…; source: gh search code "subscribe_pr_activity" (cross-repo results, e.g. synesenom/ran:.claude/skills/pr/SKILL.md))
• L34 in .claude/commands/address-review/references/watching.md "In a local CLI session there is no event stream. Poll on a bounded loop, and tell the user the shape of it before starting ('checking every 2 minutes for…" → ➖ not-a-claim (evidence: This is internal guidance/documentation for the author's own Claude command workflow describing a suggested polling pattern ("checking every 2 minutes for up to 30") as an illustrative example, not a falsifiable factual assertion about a…; source: .claude/commands/address-review/references/watching.md (self-authored process doc))
• L49 in .claude/commands/address-review/references/watching.md "- Cap it. 30 minutes of polling, then stop and report — never an unbounded loop." → ➖ not-a-claim (evidence: This is a design/policy statement authored by the PR itself describing its own polling cap behavior for a Claude command, not a falsifiable third-party fact.; source: .claude/commands/address-review/references/watching.md L49)
• L59 in .claude/commands/address-review/references/watching.md "- Pre-read the diff for the things the review reliably flags: missing aliases on moved files, internal links to pages that don't exist, frontmatter…" → ➖ not-a-claim (evidence: This is a line from an internal Claude command reference (.claude/commands/address-review/references/watching.md) describing the PR author's own review-checklist heuristics (aliases, broken links, meta_desc length, heading case). It's a…; source: .claude/commands/address-review/references/watching.md)
• L65 in .claude/commands/address-review/references/watching.md "Past the 40-minute ceiling with no pinned comment and no review:error:" → ➖ not-a-claim (evidence: This is a line from an internal Claude Code slash-command reference file (.claude/commands/address-review/references/watching.md) describing the PR author's own internal automation/workflow logic for watching review comments, not a…; source: N/A - internal repo tooling documentation, not externally verifiable)
• L69 in .claude/commands/address-review/references/watching.md "1. Re-trigger with @claude #new-review, which bypasses the skip paths, or transition draft → ready." → ➖ not-a-claim (evidence: This is internal documentation of the PR authors' own automation pipeline. Sibling files in the same repo (CONTRIBUTING.md, claude-code-review.yml, claude-triage.yml) corroborate the same @claude #new-review force-override mechanism…; source: repo:CONTRIBUTING.md, repo:.github/workflows/claude-triage.yml, repo:.claude/commands/address-review/references/watching.md)
• L71 in .claude/commands/address-review/references/watching.md "Report what happened rather than waiting again. Two silent 40-minute waits is worse than one clear 'the review job timed out; want me to retrigger it?'" → ➖ not-a-claim (evidence: This is prescriptive guidance/design rationale authored within the PR's own internal tooling documentation (a Claude Code slash-command reference), not a falsifiable external fact.; source: .claude/commands/address-review/references/watching.md)
• L82 in .claude/commands/docs-review/scripts/review-worklist.py "HEAD_SENTINEL_RE = re.compile(r'')" → ➖ not-a-claim (evidence: This is a regex literal defining the author's own internal sentinel comment format within their own tooling script, not a falsifiable factual assertion about a third party or product behavior.; source: .claude/commands/docs-review/scripts/review-worklist.py (self-authored script))
• L456 in .claude/commands/docs-review/scripts/review-worklist.py "## Pre-merge Review — Last updated 2026-08-20T10:00:00Z" → ➖ not-a-claim (evidence: This is a dynamically generated "last updated" timestamp string embedded in a script's output template (review-worklist.py), not a falsifiable factual assertion about a product or external fact.; source: .claude/commands/docs-review/scripts/review-worklist.py L456)
• L500 in .claude/commands/docs-review/scripts/review-worklist.py "- 2026-08-20T10:00:00Z — initial review (abc1234)" → ➖ not-a-claim (evidence: This is an example/placeholder timestamp and commit hash in a script file (likely a docstring or template example illustrating log format), not a falsifiable factual assertion.; source: .claude/commands/docs-review/scripts/review-worklist.py)
• L341 in .claude/commands/shipit/SKILL.md "1. Watch for the review (Recommended) — invoke /address-review {PR} --watch. Reviews typically post in 5-15 minutes." → ✅ verified (evidence: (escalated from pass3) The referenced sibling skill file .claude/commands/address-review/references/watching.md states: "claude-code-review.yml gives the job a 40-minute ceiling and the model step 18 minutes; in practice a review…; source: repo:.claude/commands/address-review/references/watching.md)
• L127-144 in .claude/commands/docs-review/scripts/review-worklist.py "inline-suggestion fetch yields an empty list on any gh/parse failure, with no confidence signal — against the module's stated 'fail-closed on completeness' contract" → ⚔️ mismatch (evidence: run() returns "" and logs to stderr on CalledProcessError/FileNotFoundError, and the per-line json.loads loop continues past any chunk it can't decode, so both paths reach return []; summarize() derives clean only from parse_confidence, which is computed from the pinned body alone; source: repo:.claude/commands/docs-review/scripts/review-worklist.py L112-119, L127-144, L355-370, L435-437)
• L188 in .claude/commands/docs-review/scripts/review-worklist.py "A one-line summary for the checklist view; the full bullet rides in JSON." → 🌀 framing-drift (evidence: extract_bucket_bullets in the shared parser returns single column-0 lines — "Sub-bullets (indented) and continuation paragraphs (no leading **) are not counted" — so the text field carries the bullet's first line only, for every bucket; source: repo:.claude/commands/docs-review/scripts/validate-pinned.py L381-408)
• L96 in .claude/commands/docs-review/scripts/review-worklist.py "BUCKETS table presented as the four buckets' shared configuration" → 🌀 framing-drift (evidence: only the outstanding and pre-existing rows are read, by the loop at L215; the low and style rows are never indexed, and their heading string is independently hard-coded in _split_low_confidence; source: repo:.claude/commands/docs-review/scripts/review-worklist.py L96-102, L164, L215, L230-241)
• L326 in .claude/commands/shipit/SKILL.md "🐿️ Ship it! Your changes are ready for review." → 🌀 framing-drift (evidence: the same PR makes Step 7's gh pr create unconditionally --draft, and Step 9 immediately offers to mark the PR ready — so at the moment this line prints, the PR is not ready for review; source: repo:.claude/commands/shipit/SKILL.md Step 7 and Step 9, this diff)
• L274 in AGENTS.md "Shipping a PR includes finishing its review. A PR is done when every finding the pre-merge review raised — 🚨 blockers, ⚠️ low-confidence findings, and ✏️…" → ➖ not-a-claim (evidence: This is a description of the PR author's own internal review/process convention documented in AGENTS.md, not a falsifiable third-party or product claim.; source: AGENTS.md L274)

🚨 Outstanding in this PR

No outstanding findings.

⚠️ Low-confidence

No low-confidence findings.

📋 Triaged verifier findings

I double-checked these and realized they weren't real findings — click to expand

No triaged findings.

💡 Pre-existing issues in touched files (optional)

No pre-existing issues in touched files.

✅ Resolved since last review

• [L153-164] .claude/commands/docs-review/scripts/review-worklist.py — decoded_any latches on the first page that parses, so a decode failure on page 2 still returns (page-1-only, True) and --require-clean answers "clean" against a genuinely short suggestion set. (resolved in 7f158321) — took the suggested shape exactly: failed_any is set both by a JSONDecodeError and by a valid-JSON document that isn't a list (a gh error object), and the return is decoded_any and not failed_any. The short list still comes back alongside ok=False, which is the right split — a partial result is worth showing, it just isn't worth calling complete. The comment that over-promised (# Bytes came back but nothing decoded) is gone, replaced by one that names the partial case specifically.

• [L44-48] .claude/commands/docs-review/scripts/review-worklist.py — the module docstring's "exactly one parser" promise no longer held: extract_bucket_bullets was uncalled, and its finding_re was duplicated as a local FINDING_START_RE. (resolved in 7f158321) — the author took the second option and found the problem was wider than the finding said: four copies, not two (extract_bucket_bullets L404 and _finding_paragraphs L2391 in validate-pinned.py, scrape-review-outcomes.py L105, and _bullet_blocks). All four now reference one exported validate_pinned.FINDING_START_RE; git grep at HEAD finds a single re.compile of the pattern, in validate-pinned.py L209, and the self-test asserts identity (is) rather than equality so a future local copy fails the suite instead of drifting quietly. The docstring now claims what is actually true — same recognition rule, wider capture, _bullet_blocks named as the deliberate extension rather than extract_bucket_bullets as a shared helper it had stopped calling.

  On the offer to revert the scrape-review-outcomes.py hunk: keep it. That line was the clearest instance of the problem — its comment already said it was "mirroring extract_bucket_bullets," which is a duplicate annotated as one. Landing a shared constant while leaving a self-documented copy beside it is the drift the constant exists to prevent, so the one-line change is inside the finding's intent, not beyond it.

• [L127-144] .claude/commands/docs-review/scripts/review-worklist.py — the inline-suggestion fetch can return an empty list without anyone noticing, and --require-clean will then answer "clean" for a PR whose ✏️ items were never enumerated. (resolved in 7cb09b9c) — fetch_inline_suggestions returns (suggestions, ok); empty stdout is ok=False, and summarize gates clean on suggestions_ok alongside parse_confidence. Two additions beyond the suggested shape are the right calls: render_markdown emits a warning banner when the fetch failed, and _remaining_label returns "every listed item decided — but the list is incomplete" rather than "0 item(s) still open" — "0 open" was precisely the false signal at issue. Leaving --body-file-with-no---pr at ok=True is correct: that caller opted out of the suggestions lane.

• [L188] _first_sentence's docstring says "the full bullet rides in JSON", but text is set from the same single line the summary is derived from. (resolved in 7cb09b9c) — the author took the first option rather than reworking the docstring: _bullet_blocks groups each column-0 bullet with its continuation lines, stopping at the next bullet or a nested heading, so text now carries the fix prose and any suggested patch. One block per bullet keeps the count in agreement with extract_bucket_bullets, and the style loop folds continuations into the bullet they sit under while still tracking ##### <path> headings.

• [L96] the BUCKETS table reads as the configuration for all four buckets, but only outstanding and pre-existing are ever read. (resolved in 7cb09b9c) — _split_low_confidence reads its heading from BUCKETS["low"][0], both the low and style loops read blocking/optional from the table, and the new PLAIN_BUCKETS tuple names the two buckets that sit directly under their own H3. The table comment now explains why low and style share a heading.

• [L326] .claude/commands/shipit/SKILL.md — Step 8's closing line still says "🐿️ Ship it! Your changes are ready for review." while Step 7 creates the PR as a draft. (resolved in 7cb09b9c) — now "🐿️ Ship it! Your changes are pushed and the PR is open as a draft."

📜 Review history

• 2026-08-21T22:26:33Z — initial review: 1 blocker (the inline-suggestion fetch can silently report an empty worklist, so --require-clean can answer "clean" without having seen the ✏️ items) and 3 low-confidence documentation/behavior mismatches; all cross-file references verified (0b20b1a)
• 2026-08-21T23:06:08Z — re-reviewed after fix push (1 new commit, 7cb09b9) at @CamSoper's request; no disputes raised. All four findings verified resolved and moved to ✅. Two new low-confidence notes on the code the fix commit added: decoded_any doesn't fail closed on a partial pagination decode failure (L153-164), and the module docstring's "exactly one parser" claim no longer holds now that extract_bucket_bullets is uncalled and its finding_re is duplicated as FINDING_START_RE (L44-48). The embedded --self-test could not be executed in this environment (sandbox blocked the interpreter), so the three new checks the commit adds were read rather than run; the author reports ./scripts/test-review-pipeline.sh green.
• 2026-08-21T23:15:33Z — re-reviewed after fix push (1 new commit, 7f15832) at @CamSoper's request; no disputes raised. Both round-2 notes verified resolved and moved to ✅; nothing new raised. The partial-decode fix matches the suggested shape (failed_any on both JSONDecodeError and non-list documents; ok = decoded_any and not failed_any), and the single-definition claim was checked independently of the diff — git grep for the bullet pattern at HEAD returns exactly one re.compile, in validate-pinned.py L209, with review-worklist.py L96 and scrape-review-outcomes.py L105 both binding _vp.FINDING_START_RE after their existing import-by-path block, and validate-pinned.py's own two former locals (L408, L2394) now using the module constant. The scrape-review-outcomes.py hunk the author offered to revert should stay: its old comment described itself as "mirroring extract_bucket_bullets," so it was the copy most likely to drift. The four new --self-test cases (all-pages-decode, silent gh failure, error object, partial failure) were read line by line — stubbing module-global run is the right seam, and fetch_inline_suggestions resolves run from globals so the stub takes effect — but the interpreter is still unavailable in this environment, so nothing was executed; the author reports ./scripts/test-review-pipeline.sh green and 184 pytest cases passing.

---

• Refresh this review — comment @claude #update-review. Say what you fixed, or which finding you dispute and why; both work in the same mention.
• Ask for anything else — comment @claude with no hashtag (questions, one-off fixes). Leaves this review untouched.

Important

Please don't hide, resolve, or delete this comment! It breaks things!

📖 How pre-merge review works — the full lifecycle, short-circuits, and escape hatches.

[CamSoper]

@claude #update-review

All four findings fixed in 7cb09b9 — no disputes.

[L127-144] 🚨 — you were right, and the docstring was writing a check the code didn't cash. fetch_inline_suggestions now returns (suggestions, ok): empty stdout is failure (with --jq '[...]', gh prints at least [] per page on success), and bytes-that-decode-to-nothing is failure too, tracked by a decoded_any flag rather than a silent continue. ok gates clean alongside parse_confidence, so a failed fetch can't present as a finished worklist. Two consequences worth noting beyond the suggested shape: the markdown render grows a warning banner when the fetch failed, and _remaining_label no longer says "0 item(s) still open" in that state — it says "every listed item decided — but the list is incomplete", since "0 open" was the exact false signal the finding is about. --body-file with no --pr stays ok=True: that caller opted out of the suggestions lane deliberately.

[L188] ⚠️ — took the first option rather than rewording the docstring. New _bullet_blocks() groups each column-0 bullet with its continuation lines (stopping at the next bullet or a nested heading), so text now carries the fix prose and any suggested patch. One block per bullet, so counts still agree with extract_bucket_bullets. The style loop folds continuations into the bullet it's under while still tracking the ##### <path> headings. Your own blocker bullet was the worked example — its python block is precisely what was being dropped.

[L96] ⚠️ — dropped the duplication rather than the rows. _split_low_confidence reads its heading from BUCKETS["low"][0], both the low and style loops read their blocking/optional from the table, and a new PLAIN_BUCKETS tuple names the two buckets that sit directly under their own H3. The table comment now explains why low and style share a heading.

[L326] ⚠️ — fixed as suggested: "🐿️ Ship it! Your changes are pushed and the PR is open as a draft."

Self-test extended to cover all three code fixes — multi-line bullet capture, block boundaries, and a failed fetch blocking clean. ./scripts/test-review-pipeline.sh green.

---

Generated by Claude Code

[github-actions]

🤖 Review updated on @CamSoper's request.

[CamSoper]

@claude #update-review

Both round-2 findings fixed in 7f15832 — no disputes.

[L153-164] — right, and the comment I wrote (# Bytes came back but nothing decoded) described a stricter guarantee than the code delivered. decoded_any latched on the first page that parsed, so a decode failure on page 2 returned a short list with ok=True. Took the suggested shape: failed_any is set by both a JSONDecodeError and a valid-JSON-but-not-a-list document, and ok is now decoded_any and not failed_any. The short list is still returned alongside ok=False — a partial result is worth showing, it just isn't worth calling complete.

[L44-48] — took the second option, and it was more crowded than the finding suggested: that regex existed in four places, not two. validate-pinned.py defined it as a local inside both extract_bucket_bullets (L404) and extract_finding_paragraphs (L2391), scrape-review-outcomes.py had its own copy at L105 explicitly commented as "mirroring extract_bucket_bullets", and _bullet_blocks added a fourth. Hoisted it to a module-level validate_pinned.FINDING_START_RE with a comment naming its consumers; all four sites now reference that one object, and the self-test asserts identity (is, not equality) so a future local copy fails the suite rather than drifting quietly. Pointing scrape-review-outcomes.py at it is one line beyond what the finding asked for — I'd rather not create a shared constant and leave a known duplicate sitting next to it, but say the word and I'll revert that hunk.

The docstring now describes what actually happens instead of claiming a reuse that had stopped being true: same recognition rule, wider capture — extract_bucket_bullets returns a bullet's first line, which is right for counting and useless for working, so _bullet_blocks walks the same sections with the same rule and keeps the continuation lines.

Self-test adds four cases for the fetch's completeness signal (all-pages-decode, silent gh failure, error object, partial failure), stubbing run rather than gh. ./scripts/test-review-pipeline.sh green and 184 pytest cases pass — worth noting since the last review couldn't execute the interpreter in its sandbox.

---

Generated by Claude Code

[github-actions]

🤖 Review updated on @CamSoper's request.