Repository navigation
Glow-ups that come out clean: pre-verify edited claims, bounded post-open autofix, receipts - #21949
Conversation
Sentinel — merge gate status (preview)Warning
|
| Gate | What it needs | |
|---|---|---|
| ✅ | G1 review-ran | Nothing to do: review current at 78e1d787c. |
| ✅ | G2 findings-answered | Nothing to do: every finding answered. |
| 🔴 | G3 right-approver | Needs approval from a member of any review team (pulumi/docs-blog-review, pulumi/docs-guild, pulumi/docs-marketing-review, pulumi/docs-tools) or a repository administrator — no qualifying human approval at this head (bot approvals never count). |
| ➖ | G4 infra-evidence | Doesn't apply to this PR: no changed path affects the deploy. |
| ➖ | G5 oversized-ack | Doesn't apply to this PR: not oversized. |
Break glass: a member of a routing team (.github/review-routing.yml teams:) can apply review:waived, which is logged.
Evaluated at head 78e1d787c.
|
Your site preview for commit 78e1d78 is ready! 🎉 http://www-testing-pulumi-docs-origin-pr-21949-78e1d787.s3-website.us-west-2.amazonaws.com |
Reviewer's guide v7 — not for the authorTip Check the PR author: nothing here is yours. Your to-do list is the "Author action guide" comment. Approval needed from: @pulumi/docs-tools — any member's approval satisfies the merge gate. Note What this PR changes:
A regression here would ship glow-up PRs with unverified or wrongly rewritten claims, or let the autofix loop push past its bounds with the bot token. The infra pass read the new workflows for permission scope and input injection (inputs pass through Review confidence:
|
Author action guide v7 — nothing blocks mergeNote Nothing here needs an answer from you. A human reviewer still approves the merge. This PR adds a pre-verify round for glow-up edits, a bounded post-open autofix workflow, and receipts in the PR body. The review read the new workflows and scripts against the infra criteria. Full evidence: verification trail, investigation log, review history. Review v7 · updated 2026-09-29T01:43:31Z · head commit 78e1d78 Please don't edit or delete this comment; it's the review's record. |
|
Independent adversarial review of head Blocking
Should fix
Minor
No Generated by Claude Code |
|
Thanks. All of these were real. All fixed in c91cec9; here's each one. Blocking
Should fix
Minor
Validated: Generated by Claude Code |
|
@claude pushed c91cec9, which fixes everything in the adversarial review above (pass-marker lifecycle, card author filter, no Bash/web in the autofix model, fail-closed export, settle before the backstop, trusted checkouts, opt-in autofix, row moves). Please re-review the new commit. #update-review Generated by Claude Code |
|
I verified Partial
New
Generated by Claude Code |
pulumi-bot opens content-review PRs and never reads the author card, so a card telling 'the PR author' to answer addressed nobody. Cards for content-review/* branches carry CLAUDE_REVIEW_AUTOMATED_AUTHOR and address the requested reviewer; build-evidence keeps that audience when it re-stamps the callout. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KAxPaBAhPHjsfgkAXuuXvq
When the pre-merge review still blocks a glow-up after it opens, one pass per head (at most two per PR) fixes, refutes, or reverts each open finding, re-verifies its own change with preverify-glowup.py, pushes as pulumi-bot, answers the card with one #update-review mention, and records the dispositions in the PR body. Whatever it can't settle goes to the routed team by name, once. Kill switch: GLOWUP_AUTOFIX=0. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KAxPaBAhPHjsfgkAXuuXvq
Whole-page extraction on dense pages truncated at max_tokens=8192 on both passes (the Pulumi YAML reference, every run in September 2026), so the content-review lanes verified those pages on the regex floor alone. A truncated call now retries as two halves split at the nearest H2, two levels deep, and the spent tokens stay on the books. Also: GLOWUP_PREVERIFY=0 kill switch for the pre-verification rounds; replay-mode (--verdicts-file) for measuring pre-verify on past PRs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KAxPaBAhPHjsfgkAXuuXvq
… checks - Autofix pass markers record the answered head separately from the pushed head, so a refreshed card gets pass 2 and escalation; a refute-only pass escalates once the card refreshes with findings open. - Review cards come from github-actions[bot] only; the fix job's model has no Bash or web tools. - The autofix post-check runs from a fresh master checkout on inputs rebuilt from git, and the change is withheld unless the model and a non-degraded post-check both succeeded. - Pre-verify rounds record the article hash; settle restores the pre-repair snapshot (or the pristine page) before the backstop reverts. Rounds run from a trusted checkout of the workflow's own commit. - Reverted blocks move matched Backlog executed rows to declined. - The autofix stands down on human commits or answers and is opt-in (GLOWUP_AUTOFIX == '1'); the card promises the autofix only on glow-up branches. - split_user_text finds H2s in standard-scope hunks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KAxPaBAhPHjsfgkAXuuXvq
…lation - The autofix post-check and patch build move to their own job on a fresh runner; nothing runs after the model on its runner except a two-file upload. The model also gets --disallowed-tools for Bash and web, which overrides project allow rules. - The trusted pre-verify checkout is deleted before every checkout, so a planted hook or core.fsmonitor can't run. - A pass escalates only what it left unresolved; a budget-spent PR escalates through select with what is actually open. - A bare human '@claude #update-review' refresh no longer stands the autofix down; only answers to findings (or human commits) do. - settle's restored-pristine outcome also withholds the verdict, so a bundle-asset change can't publish under a stale body. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KAxPaBAhPHjsfgkAXuuXvq
- settle's restored-pristine case now writes .content-review-incomplete.json, carried in the review handoff; record-review.py --incomplete-marker records the no-sentinel run incomplete (retried) instead of derived clean. - glowup-autofix: neutralize HTML comments in model-supplied note/source (and rendered summaries), and read the pass/escalation markers only as the comment's first line, so a note can't forge or suppress either marker. - human_activity: a commit author with no linked login is a person unless its email is one of our automation identities (pulumi-bot, the Claude co-author trailer, *[bot] noreply). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FHDG7o9KCZvdnEdBZtW6qq
78d194b to
bc86ed7
Compare
|
@claude rebased onto master (now Generated by Claude Code |
…oke hooks Never PR upstream. Fork sandbox for smoke-testing pulumi#21949.
The three pre-verify round gates compared `vars.GLOWUP_PREVERIFY != '0'`
bare. An unset variable is null, and GitHub coerces null-vs-'0' to numbers
(0 != 0 is false), so unset meant OFF: every round was skipped and the
receipts said "did not run". A fork smoke run caught it. Wrap the variable
in format('{0}', ...) like every other kill switch in the repo, and add a
test that fails on any bare `vars.X ==/!= '0'` in a workflow.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FHDG7o9KCZvdnEdBZtW6qq
…oke hooks Never PR upstream. Fork sandbox for smoke-testing pulumi#21949.
|
Fork smoke test found a real bug, fixed in 78e1d78. The three pre-verify round gates used a bare Re-running the fork smoke on the fixed head to check the rounds and the restored-pristine → @claude pushed 78e1d78: pre-verify gate null-coercion fix plus a workflow guard test #update-review Generated by Claude Code |
|
Fork smoke test on the fixed head (78e1d78, plus a fork-only ESC bypass on CamSoper/pulumi.docs) passed end to end:
Not covered: the autofix workflow (it's still off behind Minor, not fixed: on the restored-pristine path the receipts keep the generic "Every claim on an edited line is verified…" line under the CAUTION. It only shows in the The fork's master is back to where it was, and the smoke PRs are closed. Generated by Claude Code |
Proposed changes
Glow-up PRs keep shipping errors the pre-merge review then catches, and those findings deadlock because the author card asks pulumi-bot to answer. This makes a glow-up PR either open already clean or get fixed after it opens, with receipts, and gives whatever's left a human owner.
1. Pre-verification (primary path) --
scripts/content-review/preverify-glowup.pyAfter the glow-up model step, the workflow runs the pre-merge review's own claim pipeline (URL fetch, regex + atomic + holistic extraction at
standardscrutiny, merge,verify-claims.py) over the pristine→edited patch. Atstandardscrutiny the extractor reads only+lines, so this checks the edited spans and nothing else: the same claims the review will check, at a cost proportional to the edit.contradicted/mismatch/flagged(🚨),unverifiable(❓), or aframing-driftthe edit introduced. Each gets a provenance against the pristine page's verdicts (introducedvscarried).### Pre-verification repairsection in the skill: correct the claim to what the cited evidence states outright, or revert the edit to the pristine text. Later rounds reuse verdicts for unchanged claim text, so a repair round pays only for what it changed.revert). The diff only shrinks, so nothing new needs verifying.verify-glowup-scope.pystill bounds the result at publish. A Backlog executed row whose line was reverted moves to Backlog declined, and its id moves in the verdict. Rows with no known line are listed in the receipts for a human to check..preverify-code, deleted before every checkout so no planted hook orcore.fsmonitorsurvives) with only the edited article copied in, and the composite action (.github/actions/glowup-preverify-round) is loaded from that checkout. Round files and pre-repair snapshots are uploaded as artifacts as soon as they are written, then read back from fresh downloads into cleared directories.GLOWUP_PREVERIFY=0.2. Receipts
A new
## Pre-verificationsection in every glow-up PR body (composer placeholder, filled by the workflow and never by the model) lists every claim on an edited line with its verdict, source, and outcome (kept / repaired in round N / reverted), plus the reverted blocks and token cost. A machine marker (<!-- PREVERIFY {...} -->) carriesclean/degraded/counts. The full JSON goes to the run'spreverifyartifact (30 days).3. Bounded post-open autofix (fallback, opt-in) --
content-review-glowup-autofix.yml+scripts/content-review/glowup-autofix.pyThis is off until the repo variable
GLOWUP_AUTOFIXis'1', so merging turns on no bot pushes. When enabled, it runs when any pre-merge review lane completes (plus a 6-hourly backstop and manual dispatch) and sweeps open pulumi-botcontent-review/glowup-*PRs:select(deterministic):github-actions[bot]comments only.@claude F3: … #update-review). A bare@claude #update-reviewrefresh doesn't count, since the Sentinel and the brief tell people to post exactly that.fix(model, unprivileged): the tools areRead,Write,Edit,Glob,Grepplus the brand MCP, with no Bash and no web (--disallowed-toolsoverrides any allow rule in the PR head's settings), because it reads review text that commenters can influence while holding an API key. For each finding it recordsfixed/reverted/refuted(with a source from the checkout) /unresolved. A bot can'tacceptordefer. Nothing runs on that runner after the model except the upload of two files by path.postcheck(deterministic, fresh runner): re-verifies the change with the same pre-verify code, from a clean master checkout with only the article overlaid, builds the patch, and fails closed. Nothing the model could have planted (git config, hooks, Python startup files) exists on this runner. The change leaves the job only when the model step and a non-degraded post-check both succeeded. Otherwise the patch is empty, and everyfixed/revertedis recordedunresolved.publish(deterministic, credentialed):verify-glowup-scope.pyagainst master, and runsmake lint.@claude F1: … F2: … #update-reviewcomment as pulumi-bot, and accumulates dispositions in a## Post-open review findingstable in the PR body.unresolved. A PR whose refreshed card is still open after the budget is spent escalates throughselect, which lists what's actually open. Either way it happens once. The routed team (fromroute-pr.py/review-routing.yml) is requested, a comment names the team and the open ids, and the PR getscontent-review/needs-human.4. The author card no longer addresses pulumi-bot
Cards for
content-review/*branches carry<!-- CLAUDE_REVIEW_AUTOMATED_AUTHOR -->, and the callout addresses the requested reviewer. Onlycontent-review/glowup-*cards (marker… autofix) mention the autofix, and only "where enabled"; fix- and retire-lane cards don't.build-evidence._fix_headerkeeps that audience when it re-stamps the callout.5. Correction: the survey's "no requested reviewer" was wrong, but the PR body's claim was also wrong
All three open glow-ups do have a team review request (#21939 and #21938 →
docs-guild; #21897 →docs-marketing-reviewvia the get-started override). Triage made them. The survey readrequested_reviewers, which lists users only; teams are inrequested_teams. The glow-up notice's "the PR-review sweep assigns the reviewers" was still false, because no sweep does that. The notice, the skill, and the worker prompt now say what happens: triage requests the routed team, and the autofix escalates to it by name.6. Found along the way: whole-page claim extraction silently truncated on dense pages
Both LLM extraction passes over the 770-line YAML reference failed with
response truncated at max_tokens=8192 (twice)on every glow-up/fix run in September. The page was verified on the 34 regex claims alone, which is part of why the glow-up's number edits there went unchecked.extract-claims-llm.pynow retries a truncated call as two halves split at the nearest H2 (two levels deep), and keeps the spent tokens in the usage totals. This touches a shared review script, but the change is minimal, and without it pre-verify would degrade the same way on a large glow-up. Cost change for every lane that uses the extractor: a split happens only on a call that already truncated twice, which previously meant the pass returned nothing. The worst case is 1+2+4 = 7 calls per truncated body per pass, so 14 per file across both passes. H2 detection works in both the whole-file and the standard-scope hunk formats.Measurement: replay on #21939 and #21897
I had no API key in this session, so instead of re-running the (non-deterministic) glow-ups I replayed pre-verify's real
verify/revert/receiptscode against the actual verdicts. The pre-merge review ran the same scripts over the same pristine→head diff; itsreview-claimsartifact, plus the glow-up runs'review-snapshotpristine copy and handoff patch, is exactly what round 1 would have produced (preverify-glowup.py verify --verdicts-file).4 changes. 1 unchanged).Cost (Opus 5.5 $4/$20/MTok, cache-read $0.20; Sonnet 5 $2/$10):
So round 1 costs about what the pre-merge review already spends verifying the same diff: +40% on a typical glow-up and roughly 2.7× on a dense reference page. I prototyped reusing this run's pristine verdicts for claims whose assertion didn't change. The replay showed zero matches on both PRs, because the extractor restates claims differently each run, so I removed it rather than ship dead complexity. A cheaper design would let the pre-merge review reuse pre-verify's verdicts for a PR whose diff is unchanged since pre-verify ran. That trades away the review's independence and belongs to the review pipeline, so I left it out; say if you want it.
Validation
make test-review-pipeline: every suite passes, including new self-tests (preverify-glowup.py,glowup-autofix.py), new pytest suites (test_extract_claims_split.py, automated-author tests intest_compose_v3.py), and the existing workflow-sanity / MCP-config checks on the new model steps.make lint: passes. Prettier is clean on the new YAML.preverify-glowup.pyend-to-end in--dry-runagainst the Content glow-up: content/docs/iac/get-started/aws/modify-program.md #21897 article (every pipeline stage invoked, receipts rendered), and in replay mode as above.glowup-autofix.py selectagainst live Content glow-up: content/docs/iac/languages-sdks/yaml/yaml-language-reference.md #21939:fix — 7 open blocking finding(s); pass 1/2.b7f6561, found four blockers and four should-fixes, fixed inc91cec9f. The follow-up onc91cec9ffound two partial fixes and four new items, fixed inedf9fda3. Both rounds added lifecycle self-tests (select → answer → select at the new head, refute-only escalation, human stand-down, card author filter, settle's three outcomes, row moves).GLOWUP_PREVERIFY=0turns it off). The autofix stays off untilGLOWUP_AUTOFIX=1.Out of lane, noted
author_staleness(review-routing.yml) will close a bot PR at 21 days even though it has no author to go stale. That's the other review-loop session's area; flagging it here only.Related issues (optional)
#21939, #21897, #21938
🤖 Generated with Claude Code
https://claude.ai/code/session_01KAxPaBAhPHjsfgkAXuuXvq
Generated by Claude Code