fix(vale): scope changed-files diff to PR branch additions only#6211
Merged
Conversation
Switch the two git diff invocations in changed_lines_to_json.py from endpoint comparison (base head) to three-dot syntax (base...head), which scopes the diff to what the PR branch added since it diverged from main. The previous form folded main's drift since the branch was created into the "PR-changed" file set, causing Vale to run on files the PR never touched. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
3 Skipped Deployments
|
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.
Summary
main. Reproduced on clickpipes: support 32 vcpus #6210 (a one-file PR; Vale reported 30 errors across 17 unrelated files).scripts/vale/changed_lines_to_json.pyused endpoint diff (git diff <base> <head>), which surfaces both the PR's edits andmain's drift since the branch was created.<base>...<head>), which scopes the diff to what the branch added since it diverged frommain.Problem (reproduced on #6210)
PR #6210 ("support 32 vcpus") changes a single file:
docs/integrations/data-ingestion/clickpipes/postgres/scaling.md, +5/−5. Vale CI failed with 30 annotations across 17 different files, none of themscaling.md. Examples of flagged files:docs/whats-new/changelog/index.mddocs/best-practices/json_type.mddocs/cloud/_snippets/_clickpipes_faq.mddocs/chdb/index.mdThese are pre-existing style issues on files modified on
mainafter the PR's branch was cut.Cause
scripts/vale/changed_lines_to_json.pybuilds the "PR-changed" file list withgit diff --name-only <base_sha> <head_sha>(endpoint comparison). Whenbase.shais today'smainHEAD andhead.shais a PR commit whose parent is an oldermain, this diff returns two kinds of differences lumped together:mainhas gained since the branch diverged but that the PR doesn't have (incorrect — that'smain's drift, not the PR's work).Vale then runs on the combined file set and reports pre-existing issues in files the PR never modified.
Fix
<base>...<head>is git shorthand formerge-base(base, head)..head— only what the branch added since it diverged. No behavior change for up-to-date PRs (when the branch's parent equalsbase.sha, three-dot and endpoint forms are identical).Verification
Running the patched script against #6210's actual SHAs:
1 file, 5 lines — matches the PR's actual
+5diff exactly. With the same SHAs on the unpatched script, the file list contained 17 files ofmain-drift.Test plan
scaling.md) when run against clickpipes: support 32 vcpus #6210's SHAs locallymainPR (e.g., clickpipes: support 32 vcpus #6210) confirms Vale annotations are scoped to PR-touched fileslogs/changed_lines.jsonRelated
Sibling fix to #6188, which fixed within-hunk line-range tracking in
vale_annotations.py. That PR corrected how the annotation parser maps Vale output to PR lines; this PR corrects which files and lines are considered "PR-changed" in the first place. Both scripts now agree on the PR scope.🤖 Generated with Claude Code