Skip to content

fix: self-heal Flue incremental review diff on rebase and upstream merges#32070

Open
mvvmm wants to merge 2 commits into
productionfrom
fix/flue-incremental-diff-rebase-contamination
Open

fix: self-heal Flue incremental review diff on rebase and upstream merges#32070
mvvmm wants to merge 2 commits into
productionfrom
fix/flue-incremental-diff-rebase-contamination

Conversation

@mvvmm

@mvvmm mvvmm commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes the Flue docs-bot occasionally posting a review whose findings are about files the PR never touched (observed on #31957, whose comment flagged catalog-models/*.json, chunk-citations.mdx, and client-api.mdx — none of which are in that PR).

Root cause: the code-review and style-guide specialists computed their incremental diff as a three-dot compare of the last-reviewed head SHA against the current head. That is only correct when the branch is a clean forward-extension of the last-reviewed SHA (a normal push). After a rebase/force-push the old SHA is orphaned, so GitHub's compare merge-base regresses to the old fork point and the delta sweeps in every upstream production commit the branch absorbed. A production → PR merge ("Update branch") has the same effect while still reporting status: ahead. The prior self-heal only fired on a 404 (base SHA garbage-collected), so neither case was covered.

Fix: a shared fetchFilesForDiffMode helper (.flue/lib/diff-fetch.ts) that trusts the incremental delta only when all three hold, else self-heals to the full PR diff:

Guard Catches
compare succeeded (not 404) base SHA garbage-collected
status is ahead/identical rebase / force-push (diverged)
every file in the delta is in the PR's net diff production → PR merge that drags in upstream files

comparePullRequestHeads now returns the compare status/ahead_by/behind_by to support this, and both specialists log a diff_self_healed event with the reason when incremental degrades to full. The normal fast-forward push path is unchanged, so line-level incremental review is preserved.

No test infra exists in .flue; verified via tsc --noEmit -p .flue/tsconfig.json (passes) and Prettier (format:core scope). conventions-specialist and dependabot-review use the full diff only and are unaffected.

…rges

The code-review and style-guide specialists computed their incremental
diff as a three-dot compare of the last-reviewed head SHA against the
current head. That is only correct when the branch is a clean forward
extension of the last-reviewed SHA. After a rebase/force-push the old SHA
is orphaned, so the compare's merge-base regresses to the old fork point
and the delta sweeps in every upstream commit the branch absorbed — the
review then flags findings in files the PR never touched. A
production->PR merge ("Update branch") has the same effect while still
reporting status: ahead.

The prior self-heal only fired on a 404 (base SHA garbage-collected), so
neither the rebase nor the merge case was covered.

Add fetchFilesForDiffMode (lib/diff-fetch.ts), shared by both
specialists, which trusts the incremental delta only when the compare
succeeds, its status is ahead/identical, and every file in the delta
belongs to the PR's net diff. Any violation self-heals to the full PR
diff. comparePullRequestHeads now returns the compare status/ahead_by/
behind_by to support this.
@github-actions

Copy link
Copy Markdown
Contributor

This pull request requires reviews from CODEOWNERS as it changes files that match the following patterns:

Pattern Owners
* @cloudflare/product-owners
*.ts @cloudflare/content-engineering, @kodster28

@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

@mvvmm mvvmm marked this pull request as ready for review July 14, 2026 16:21
@mvvmm mvvmm requested review from a team and kodster28 as code owners July 14, 2026 16:21
@cloudflare-docs-bot

cloudflare-docs-bot Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Review

✅ No issues found in full PR diff.

Code Review

This code review is in beta and may not always be helpful — use your judgment.

No code review issues found.

Conventions

No convention issues found.

Style Guide Review

No style-guide issues found.

Commands

Only codeowners can run commands. Post a comment with the command to trigger it.

Command Description
/review Runs a review now. Incremental if a prior review exists, full if not.
/full-review Re-reviews the entire PR diff from scratch, ignoring incremental history. Useful after a rebase, when you want a fresh review, or if the bot gets out of sync and reports issues that no longer exist.
/ignore-review-limit Permanently lifts the 2-review automatic limit for this PR. Future pushes will trigger reviews as normal.
/disable-auto-review Stops automatic reviews from triggering on future pushes to this PR. Codeowners can still run /review or /full-review manually.

Address review findings on the diff self-heal:

- getPullRequestFiles now follows Link-header pagination instead of
  fetching a single per_page=100 page, so the PR net-diff set used by
  fetchFilesForDiffMode's containment check is complete for PRs with more
  than 100 changed files.
- comparePullRequestHeads paginates the compare file list (deduping by
  filename, since the compare endpoint paginates over commits and can
  repeat files across pages) and captures status/ahead_by/behind_by from
  the first page. This keeps the incremental delta and the upstream-file
  self-heal decision from being based on a truncated first page.
- The compare base/head refs are URL-encoded (preserving '/') so refs
  with special characters produce a well-formed request.
@mvvmm

mvvmm commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

/full-review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants