Skip to content

fix(scripts): end make check with a ran/skipped summary and verdict - #36864

Merged
mateo-berri merged 2 commits into
litellm_internal_stagingfrom
litellm_make_check_run_summary
Aug 14, 2026
Merged

fix(scripts): end make check with a ran/skipped summary and verdict#36864
mateo-berri merged 2 commits into
litellm_internal_stagingfrom
litellm_make_check_run_summary

Conversation

@mateo-berri

@mateo-berri mateo-berri commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

  • make check could exit 0 printing nothing, leaving a zero-byte log
  • staged files matching no check looked identical to a crashed run
  • devs re-ran it, then read the script, to learn nothing ran

How it solves it:

  • every run now ends naming each check as ran or skipped
  • when nothing matches, a NOTE lists the in-scope files and says the pass is a no-op
  • a final check: PASS or check: FAIL line mirrors the exit code
  • early informational exits (nothing to check, no merge base) print the verdict too

User Flow

Before: a contributor whose staged files match no lint check gets a silent exit 0 and a zero-byte log, indistinguishable from the script dying early

  1. They stage a cost-map JSON and a tests/test_litellm test file with git add, then run make check
  2. The terminal prints only "check: logging full output to .git/pre_commit_lint.log" and "check: full log: ..." with nothing in between, and exits 0
  3. They open the log file and find it empty, so a clean pass looks exactly like a run that silently broke
  4. They re-run make check a few times, then have to read scripts/pre_commit_lint.sh itself to learn that nothing was ever supposed to run

After: the same run ends with a per-check ran/skipped summary, the reason nothing ran, and an explicit verdict

  1. They stage a cost-map JSON and a tests/test_litellm test file with git add, then run make check
  2. The terminal prints the same first log line, then a "check: summary" block naming all four checks as skipped with the reason, like "no litellm/ Python files in scope"
  3. A NOTE lists the staged files, says no gating lint check matches them, and that the pass is a no-op, not a lint verdict
  4. The run ends with "check: PASS" ("check: FAIL" on a red run), and the log file holds that same summary instead of being empty

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)

Delays in PR merge?

If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).

Screenshots / Proof of Fix

Scenario from the bug report: a file no gating check covers (a tests/test_litellm Python file) is the only thing staged, in a bootstrapped worktree

Before, running the script as of the merge base 9d069f2:

$ git show origin/litellm_internal_staging:scripts/pre_commit_lint.sh > /tmp/base_pre_commit_lint.sh && chmod +x /tmp/base_pre_commit_lint.sh
$ git add tests/test_litellm/tmp_proof_scratch.py
$ /tmp/base_pre_commit_lint.sh; echo "exit=$?"
check: logging full output to /Users/mateo/Development/litellm/.git/worktrees/litellm-wt-check-summary/pre_commit_lint.log
check: full log: /Users/mateo/Development/litellm/.git/worktrees/litellm-wt-check-summary/pre_commit_lint.log
exit=0
$ wc -c /Users/mateo/Development/litellm/.git/worktrees/litellm-wt-check-summary/pre_commit_lint.log
       0 /Users/mateo/Development/litellm/.git/worktrees/litellm-wt-check-summary/pre_commit_lint.log

After, same staged file at cfbd431:

$ scripts/pre_commit_lint.sh; echo "exit=$?"
check: logging full output to /Users/mateo/Development/litellm/.git/worktrees/litellm-wt-check-summary/pre_commit_lint.log
check: summary
    skipped: Python lint (make lint) (no litellm/ Python files in scope)
    skipped: tests/e2e checks (basedpyright + raw HTTP client ban) (no tests/e2e Python files in scope)
    skipped: dashboard lint (prettier + eslint + lint budgets) (no dashboard files in scope)
    skipped: dashboard API-type sync (npm run gen:api) (no litellm/proxy, litellm/types, or generator files in scope)
check: NOTE - no gating lint check matches the files in scope, so nothing ran:
    tests/test_litellm/tmp_proof_scratch.py
  A pass here is a no-op, not a lint verdict.
check: PASS
check: full log: /Users/mateo/Development/litellm/.git/worktrees/litellm-wt-check-summary/pre_commit_lint.log
exit=0
$ wc -c /Users/mateo/Development/litellm/.git/worktrees/litellm-wt-check-summary/pre_commit_lint.log
     584 /Users/mateo/Development/litellm/.git/worktrees/litellm-wt-check-summary/pre_commit_lint.log

Early-exit verdict at 3fbe40c, in a clean detached worktree at the staging tip with nothing staged and no changes:

$ scripts/pre_commit_lint.sh; echo "exit=$?"
check: logging full output to /Users/mateo/Development/litellm/.git/worktrees/litellm-wt-clean-tip/pre_commit_lint.log
check: nothing to check (no staged files, no working-tree changes, no branch changes vs origin/litellm_internal_staging)
check: PASS
check: full log: /Users/mateo/Development/litellm/.git/worktrees/litellm-wt-clean-tip/pre_commit_lint.log
exit=0

And a run at cfbd431 where checks are in scope (this PR's own files staged plus the scripts change exercised via make check before committing) ended the same way with the ran/skipped split and a verdict, per the new tests which also cover the FAIL path

Type

🐛 Bug Fix

Caveats (if any)

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

Note

Low Risk
Changes are limited to the local pre-commit lint script and its unit tests; no production runtime, auth, or data paths are touched.

Overview
make check (scripts/pre_commit_lint.sh) no longer finishes with a silent exit 0 and an empty log when nothing runs. Every run now ends with a check: summary block that labels each of the four gating checks as ran or skipped (with a short reason).

When staged or scoped files match no lint gate, the script prints a NOTE listing those paths and stating the pass is a no-op, not a lint verdict. Early exits (nothing to check vs. cannot resolve merge base) also print check: PASS or check: FAIL so the outcome matches the exit code.

Tests in test_pre_commit_lint.py assert the new verdict lines and add coverage for the summary, the no-match no-op path (including a non-empty log), and failing runs.

Reviewed by Cursor Bugbot for commit 3fbe40c. Bugbot is set up for automated code reviews on this repo. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR adds explicit verdicts to informational exits and appends a ran/skipped summary and final status to normal lint runs.

  • Reports which Python, e2e, dashboard, and API-type synchronization checks ran or were skipped.
  • Explains no-op scopes and records the summary in the existing lint log.
  • Adds tests for successful, failing, empty-scope, and unmatched-file runs.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
scripts/pre_commit_lint.sh Adds per-check summary output, no-op diagnostics, and PASS/FAIL verdicts while preserving the existing exit status.
tests/test_litellm/test_pre_commit_lint.py Extends the sandboxed script tests to verify summaries, no-op logging, and success/failure verdict output.

Reviews (2): Last reviewed commit: "fix(scripts): print the make check verdi..." | Re-trigger Greptile

Comment thread scripts/pre_commit_lint.sh
Comment thread scripts/pre_commit_lint.sh
@codecov

codecov Bot commented Aug 14, 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

Copy link
Copy Markdown
Contributor Author

@greptileai

@mateo-berri

Copy link
Copy Markdown
Contributor Author

bugbot run

@cursor cursor Bot left a comment

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.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 3fbe40c. Configure here.

@mateo-berri
mateo-berri merged commit 3c57e9f into litellm_internal_staging Aug 14, 2026
82 of 85 checks passed
@mateo-berri
mateo-berri deleted the litellm_make_check_run_summary branch August 14, 2026 03:45
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.

2 participants