Repository navigation
fix(docs-pr-review): stop follow-up reviews from churning on style - #2163
Merged
Merged
Conversation
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.
Contributor
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
Collaborator
✅ Heimdall Review Status
|
Contributor
Docs reviewRisk: High (one human reviewer (the tab owner), with an AI pre-review) ·
Lower-level signals
No documentation pages changed in Advisory review from the Docs PR review workflow (run). It does not approve, block, or edit this PR. Add the |
ericbrown99
previously approved these changes
Oct 19, 2026
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.
ericbrown99
approved these changes
Oct 19, 2026
Collaborator
|
Review Error for ericbrown99 @ 2026-10-19 23:09:34 UTC |
ericbrown99
approved these changes
Oct 19, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
The causes in the code:
This PR adds structure to follow-up rounds:
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.docs/llms-full.txtfor 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 readsdocs/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).npm testnow 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
contents: read; minimizing comments usespull-requests: writethroughapi.github.com/graphql, a host already allowed.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.How has it been tested?
npm test: 189/189 pass, including 12 new tests inscripts/docs-pr-review/__tests__/history.test.mjs: state round-trip and tampering, the legacy fallback, compare handling, scope guards, prompt escaping, the summary passing the posting gate, and the realllms.txtindex.index.mjs --context-onlyagainst docs(base-chain): add fast bridges, aggregators, and messaging protocols to Bridge to Base #2159 and docs(base-verify): add base mainnet identity attestation options #2141 shows that history loads and already-reviewed heads exit without a model call.Screenshots
N/A (no user-facing changes)
Generated with Toshi