Skip to content

fix(ci): scope UI lint to the files a PR actually changed#34600

Merged
mateo-berri merged 1 commit into
litellm_internal_stagingfrom
litellm_fix_ui_lint_stale_base_diff
Jul 25, 2026
Merged

fix(ci): scope UI lint to the files a PR actually changed#34600
mateo-berri merged 1 commit into
litellm_internal_stagingfrom
litellm_fix_ui_lint_stale_base_diff

Conversation

@mateo-berri

@mateo-berri mateo-berri commented Jul 25, 2026

Copy link
Copy Markdown
Collaborator

TLDR

Problem this solves:

  • UI Lint / frontend-lint fails PRs on other people's files
  • Its diff spans base-branch commits landed after PR open
  • A two-Python-file PR got 283 dashboard files linted

How it solves it:

  • Diff the PR head against its own merge base
  • Correct whether HEAD is a merge commit or the head

Relevant issues

Blocks #34192

Linear ticket

Pre-Submission checklist

  • I have added meaningful tests (no harness runs GitHub Actions workflow files; the selection logic is reproduced by hand on the failing job's exact commits under Proof of Fix)
  • My PR passes all CI/CD checks (e.g., lint, format, unit tests)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review

Screenshots / Proof of Fix

There is nothing to curl here; the change is to how a CI job selects files, so the proof is the job's own selection reproduced by hand against the exact commits it ran on.

The failing job is frontend-lint on #34192. That PR changes two Python files and no UI file. Its checkout step reports what the job is actually looking at:

HEAD is now at 4dbc09f727 Merge 85ad6971e997f2507d6fe0be51423f22d6bf782d into 7b019cf152d0001c14fe18ffc5bd57212b3ed47b

so HEAD is the PR merged into the current base tip, while github.event.pull_request.base.sha is still 28e93e42e5, the base tip from when the PR was opened. git diff "$BASE_SHA"...HEAD takes the merge base of those two, which is 28e93e42e5 itself, so the file list becomes every base-branch commit since. Reproduced locally on the same commits:

$ git diff --name-only --diff-filter=ACMR 28e93e42e5 7b019cf152 -- ui/litellm-dashboard | wc -l
283

283, matching the job log exactly:

##[group]Prettier (283 files)
##[group]ESLint (278 files)
✖ 709 problems (3 errors, 706 warnings)
##[error]  3:1  error  'antd' import is restricted from being used by a pattern ...

All three errors are no-restricted-imports on antd in models-and-endpoints/add/page.tsx, models-and-endpoints/llm-credentials/page.tsx, and models-and-endpoints/vertexCredentialsUpload.ts, none of which #34192 touches. They are on the base branch today:

$ git show origin/litellm_internal_staging:'ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/add/page.tsx' | head -3
"use client";

import { Form } from "antd";

With this change, the same job on the same commits selects nothing and short-circuits:

$ merge_base=$(git merge-base 28e93e42e5 85ad6971e9)
$ echo $merge_base
e17f3b6e1a30bbceca25b0c34eb5345bc97804e9
$ git diff --name-only --diff-filter=ACMR "$merge_base" 85ad6971e9 -- ui/litellm-dashboard | wc -l
0
$ git diff --name-only --diff-filter=ACMR "$merge_base" 85ad6971e9
enterprise/litellm_enterprise/proxy/hooks/managed_files.py
tests/enterprise/litellm_enterprise/proxy/hooks/test_managed_files.py

has_files=false, so the job logs "No lintable UI files changed in this PR; nothing to check" and passes, which is what it was written to do.

Type

🐛 Bug Fix
🚄 Infrastructure

Changes

Collect changed files used git diff "$BASE_SHA"...HEAD. The three-dot form resolves to the merge base of its two endpoints, and for a pull_request event those endpoints are the base tip from PR-open time and a merge commit that already contains the current base tip. The merge base of those is the older base tip, so the diff is "everything on the base branch since this PR was opened, plus the PR", not "the PR".

It now resolves the merge base from the PR's own head sha and diffs from there, which is the PR's changes and nothing else, and does not depend on whether the checkout leaves HEAD on a merge commit or on the head commit.

The blast radius of the old behaviour grows with PR age: a PR is red the moment any eslint error lands on the base branch after it was opened, in a file its author never touched, and the only way out was to reopen the PR. Prettier had the same exposure, and the wasted work is real too, since 283 files were being formatted and linted for a PR that changed none of them.

The same "$BASE_SHA"...HEAD idiom appears in test-linting.yml and, as vitest --changed "$BASE_SHA", in test-litellm-ui-unit.yml. Those are over-inclusive in the same way but currently fail open rather than closed (they run extra checks rather than reporting foreign violations as yours), so they are left alone here to keep this PR to the one job that is failing.

Final Attestation

  • The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR

`UI Lint / frontend-lint` collected its file list from
`"$BASE_SHA"...HEAD`, where `BASE_SHA` is the base branch tip captured when
the PR was opened and `HEAD` is the merge of the PR into the *current* base
tip that actions/checkout leaves behind. The three-dot merge base of those two
is `BASE_SHA` itself, so the diff spans every base-branch commit landed since
the PR was opened.

Any PR opened before an eslint violation landed on the base branch therefore
fails on files it never touched. PR #34192 changes two Python files and no UI
file at all, and the job still linted 283 dashboard files and failed on three
`no-restricted-imports` antd errors from unrelated commits.

Diffing the PR head against its own merge base gives exactly the files the PR
changed, whether the checkout leaves HEAD on a merge commit or on the head
commit.
@mateo-berri
mateo-berri requested a review from a team July 25, 2026 03:31
@greptile-apps

greptile-apps Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Updates the UI lint workflow to select files by diffing the pull request head against its merge base with the pull request’s recorded base SHA.

  • Adds the pull request head SHA to the changed-file collection step.
  • Computes an explicit merge base independent of the checkout’s current HEAD.
  • Uses the resulting commit range for Prettier and ESLint file selection.

Confidence Score: 5/5

The pull request appears safe to merge, with no actionable defects identified in the changed workflow logic.

The workflow now derives the comparison from the pull request’s explicit base and head commits rather than the synthetic checkout HEAD, preventing unrelated base-branch changes from entering the lint file list.

Important Files Changed

Filename Overview
.github/workflows/test-litellm-ui-lint.yml Corrects changed-file selection so UI lint no longer includes unrelated commits added to the base branch after a pull request opened.

Reviews (1): Last reviewed commit: "fix(ci): scope UI lint to the files a PR..." | Re-trigger Greptile

@codecov

codecov Bot commented Jul 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@mateo-berri
mateo-berri merged commit d816478 into litellm_internal_staging Jul 25, 2026
76 of 77 checks passed
@mateo-berri
mateo-berri deleted the litellm_fix_ui_lint_stale_base_diff branch July 25, 2026 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants