Skip to content

fix(docs-pr-review): stop follow-up reviews from churning on style - #2163

Merged
soheimam merged 4 commits into
masterfrom
fix/docs-pr-review-rereview-churn
Oct 19, 2026
Merged

soheimam merged 4 commits into
masterfrom
fix/docs-pr-review-rereview-churn

Conversation

@soheimam

Copy link
Copy Markdown
Contributor

What changed? Why?

Follow-up runs of the docs PR reviewer kept turning into style churn. One author on #2159: "It looks recursive actually. Every time I push an update it flags new issues." The first review was useful. Every review after that reviewed the whole PR again, including text the author had just taken from the reviewer's own suggestions, and it sometimes reversed its own advice:

#2159 text Round 1 Round 2 Round 3
Deposit sentence (L32) suggested it word for word — "unsupported claim, vague link text"
Fast bridge paragraph (L65) added "typically… usually" "remove the hedge" "overgeneralizes, soften it"
Token issuer closing (L101) wrote "Choose the protocol and its security settings before launch" — "vague"

The causes in the code:

  1. The model never saw its previous reviews. The prompt had no history.
  2. Every run sent the full PR diff, so earlier lines, many of them the reviewer's own rewrites, were reviewed again.
  3. Nothing told the model that zero findings was acceptable, and "writing quality" has no stopping point.
  4. Suggested rewrites added hedges that the next round then flagged.

This PR adds structure to follow-up rounds:

  • History. The last reviewed SHA, the round number, and earlier findings and suggested text are stored as a hidden, validated marker in the summary comment. PRs reviewed before this change fall back to the bot's earlier inline reviews.
  • Scope. A follow-up round gets the previous findings and only the diff since the last reviewed commit (compare API), plus follow-up rules: don't repeat findings, don't flag text taken from an earlier suggestion, don't contradict earlier advice, report writing findings only when they're blocking, and "fits" with no findings is the expected result.
  • Code guards (applyFollowUpScope), so the scope holds even if the model ignores the prompt. In follow-up rounds, non-blocking findings are dropped when they're about wording, point at text taken from an earlier suggestion, or don't touch a line changed since the last review. Blocking findings always stay.
  • No review when nothing changed. No model call when the head was already reviewed, or when no reviewed page changed since the last review (for example, a merge from master).
  • Less noise. Outdated inline suggestions from earlier rounds are minimized as OUTDATED.
  • Index fix. The reviewer parsed docs/llms-full.txt for the page index, but that file holds full page bodies and has no index. Live pages were reported as "not in llms-full.txt" with 0 neighbors read, so placement and consistency checks ran without context. It now reads docs/llms.txt (233 entries; docs(base-chain): add fast bridges, aggregators, and messaging protocols to Bridge to Base #2159's page is found under Get Started > Quickstart with 4 neighbors).
  • The general prompt now says zero findings is a valid result and that a suggested rewrite must itself meet the guidelines.
  • npm test now runs the reviewer tests. They weren't run in CI before.
  • docs/llms-full.txt: the post-commit hook regenerated it and picked up the OpenServ row from docs(get-started): add OpenServ to Builder Stack #2158, which was missing on master.

Notes to reviewers

  • No workflow, token permission, or egress changes. Compare uses contents: read; minimizing comments uses pull-requests: write through api.github.com/graphql, a host already allowed.
  • The state marker is treated as untrusted input: fields are validated and length-capped, it's read only from github-actions[bot] comments, < and > are escaped so it can't end the HTML comment early, and it's placed only inside <untrusted_*> tags in the prompt.
  • If the compare can't be computed reliably (force-push, more than 300 files, or an API error), the round reviews the full diff under the follow-up rules, and only the writing and applied-suggestion guards apply.
  • Known limit: the model's summary paragraph could still mention a finding that the code guard later dropped. The summary says how many findings were left out.

How has it been tested?

Screenshots

N/A (no user-facing changes)

Generated with Toshi

Follow-up reviews were stateless: each push got a fresh review of the whole
PR, including text the author had just taken from the previous round, so
rounds after the first drifted into contradictory style suggestions.

- Store review state (last reviewed SHA, round, findings) in the summary
  comment; fall back to earlier inline reviews for PRs reviewed before this.
- Give follow-up rounds the previous findings and only the diff since the
  last reviewed commit, with explicit follow-up rules in the prompt.
- Enforce the scope in code: in follow-up rounds drop non-blocking writing
  findings, findings on text taken from an earlier suggestion, and findings
  outside the lines changed since the previous review.
- Skip the model call when no reviewed page changed since the last review
  (for example, a merge from master) or the head was already reviewed.
- Minimize outdated inline suggestions from earlier rounds.
- Read the page index from docs/llms.txt; llms-full.txt has no index, so
  pages were reported as not indexed and no neighbors were read.
- Run the reviewer tests in npm test.
@mintlify

mintlify Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated
base 🟢 Ready View Preview Oct 19, 2026, 10:24 PM

💡 Tip: Enable Automations to automatically generate PRs for you.

@cb-heimdall

cb-heimdall commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

✅ Heimdall Review Status

Requirement Status More Info
Reviews ✅ 1/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 0
Sum 1

@github-actions

github-actions Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Docs review

Risk: High (one human reviewer (the tab owner), with an AI pre-review) · risk: high · shadow mode, not enforced

  • changes package.json
Lower-level signals
  • Medium: changes code in scripts/docs-pr-review/__tests__/docs-pr-review.test.mjs
  • Medium: changes code in scripts/docs-pr-review/__tests__/history.test.mjs
  • Medium: changes code in scripts/docs-pr-review/context.mjs
  • Medium: changes code in scripts/docs-pr-review/github.mjs
  • Medium: changes code in scripts/docs-pr-review/history.mjs
  • Medium: changes code in scripts/docs-pr-review/index.mjs
  • Medium: changes code in scripts/docs-pr-review/prompts.mjs
  • Medium: changes code in scripts/docs-pr-review/render.mjs
  • and 1 more

No documentation pages changed in c30f97a, so there is no content review.

Advisory review from the Docs PR review workflow (run). It does not approve, block, or edit this PR. Add the skip-ai-review label to turn it off for this PR.

Comment thread scripts/docs-pr-review/__tests__/history.test.mjs Fixed
ericbrown99
ericbrown99 previously approved these changes Oct 19, 2026
@cb-heimdall
cb-heimdall dismissed ericbrown99’s stale review October 19, 2026 22:23

Approved review 5578429622 from ericbrown99 is now dismissed due to new commit. Re-request for approval.

@cb-heimdall

Copy link
Copy Markdown
Collaborator

Review Error for ericbrown99 @ 2026-10-19 23:09:34 UTC
User failed mfa authentication, see go/mfa-help

@soheimam
soheimam merged commit efe7cf8 into master Oct 19, 2026
14 checks passed
@soheimam
soheimam deleted the fix/docs-pr-review-rereview-churn branch October 19, 2026 23:49

This branch was successfully deployed

1 active deployment
staging - docs — c30f97a2 Deployed Oct 19, 2026 by mintlify[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants