Skip to content

ci(ui): only run the UI build when dashboard files change - #32809

Closed
ryan-crabbe-berri wants to merge 1 commit into
litellm_internal_stagingfrom
litellm_ui_build_gate
Closed

ci(ui): only run the UI build when dashboard files change#32809
ryan-crabbe-berri wants to merge 1 commit into
litellm_internal_stagingfrom
litellm_ui_build_gate

Conversation

@ryan-crabbe-berri

Copy link
Copy Markdown
Contributor

Relevant issues

Linear ticket

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have added meaningful tests
  • 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 (Greptile reviews automatically once the PR is opened; only comment @greptileai to re-request a review after pushing changes)

Screenshots / Proof of Fix

The build-ui job ran a full npm ci + npm run build (a ~10-minute next build) on every PR, backend-only ones included, even though nothing it builds had changed. This gates it the same way frontend-lint already gates itself: the job still runs on every PR so the required check reports a result and never strands, but the install and build steps only execute when a file under ui/litellm-dashboard/ actually changed.

The detection step runs from the job's working-directory (ui/litellm-dashboard), so git diff --name-only "$BASE_SHA"...HEAD -- . is already scoped to the dashboard; it needs fetch-depth: 0 on the checkout so the base commit is present for the diff.

Behavior:

PR touches ui/litellm-dashboard/**   -> ui_changed=true  -> Setup + npm ci + npm run build run (unchanged)
PR touches only backend/docs         -> ui_changed=false -> steps skipped, job reports success in seconds

This is a CI-only change; the logic mirrors the existing frontend-lint job in the same repo, and both workflow files parse as valid YAML.

Type

🚄 Infrastructure

Changes

Add a "Detect UI changes" step to build-ui and gate Setup Node, Install dependencies, and Build on its ui_changed output. Add fetch-depth: 0 to the checkout so the PR base commit is available to diff against.

The job intentionally still runs on every PR rather than using a workflow-level paths: filter; a paths: filter would leave the required check pending forever on PRs that don't touch the UI, which is the stranding problem this repo already solved for backend tests in #32532. Keeping the job and gating its steps avoids that.

build-ui ran a full `next build` (~10 min, npm ci + build) on every PR,
including backend-only ones. Mirror the frontend-lint job's pattern: keep the
job always running so the required check never strands, but gate the install
and build steps on whether anything under ui/litellm-dashboard actually
changed. On backend-only PRs the job now reports success immediately without
installing or building.
@ryan-crabbe-berri
ryan-crabbe-berri requested a review from a team July 10, 2026 19:01
@greptile-apps

greptile-apps Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This CI-only change optimises the build-ui job by adding a "Detect UI changes" step that short-circuits the expensive npm ci + next build path when no files under ui/litellm-dashboard/ have changed. The approach deliberately keeps the job running on every PR (avoiding the required-check stranding problem) and gates only the costly steps via steps.changed.outputs.ui_changed.

  • Adds fetch-depth: 0 to the build-ui checkout so the base commit is available for the three-dot git diff, mirroring the pattern already in frontend-lint.
  • The detection shell script pipes git diff --name-only \"$BASE_SHA\"...HEAD -- . through grep -q .; since the working-directory is ui/litellm-dashboard, the -- . pathspec is automatically scoped to the dashboard subtree.

Confidence Score: 4/5

Safe to merge — the change is purely additive to a CI workflow and cannot affect production code.

The implementation correctly mirrors the existing frontend-lint job's change-detection pattern. The one edge case worth noting is that a git diff failure would silently fall through to ui_changed=false and skip the build rather than failing the step, but fetch-depth: 0 makes that scenario very unlikely in practice.

No files require special attention beyond the single workflow file changed.

Important Files Changed

Filename Overview
.github/workflows/test-litellm-ui-build.yml Adds a "Detect UI changes" step to the build-ui job to skip expensive npm ci + next build steps when no dashboard files changed; mirrors the existing frontend-lint job pattern with fetch-depth: 0 and a git diff guard.

Reviews (1): Last reviewed commit: "ci(ui): only run the UI build when dashb..." | Re-trigger Greptile

Comment on lines +32 to +38
run: |
if git diff --name-only "$BASE_SHA"...HEAD -- . | grep -q .; then
echo "ui_changed=true" >> "$GITHUB_OUTPUT"
else
echo "ui_changed=false" >> "$GITHUB_OUTPUT"
echo "No ui/litellm-dashboard changes in this PR; skipping build."
fi

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Silent git failure sets ui_changed=false

GitHub Actions uses bash -eo pipefail by default, but if pipeline; then absorbs the pipeline's exit code rather than propagating it as a script error. If git diff were to fail (e.g., BASE_SHA not in the fetched history despite fetch-depth: 0), the pipeline exits non-zero, the else branch fires, and ui_changed=false is written — silently skipping the build even when dashboard files did change. fetch-depth: 0 makes this scenario unlikely, and the existing frontend-lint job uses the same pattern, so this is a minor robustness note rather than a blocking concern.

@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@codspeed-hq

codspeed-hq Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 31 untouched benchmarks


Comparing litellm_ui_build_gate (6174210) with litellm_internal_staging (11aeeea)1

Open in CodSpeed

Footnotes

  1. No successful run was found on litellm_internal_staging (b9008cc) during the generation of this report, so 11aeeea was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@ryan-crabbe-berri

Copy link
Copy Markdown
Contributor Author

Folding this into #32810; the build-ui gating and the lint-workflow split touch the same file and are one concern, so they belong in a single PR.

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.

1 participant