chore: ignore the mechanical lint and typing sweeps in git blame - #36076
Conversation
Seven wide-reaching but semantically neutral commits landed since the last entry, together rewriting roughly 162k lines across ~4,700 file touches. Blame on any line they reflowed points at the sweep instead of the commit that wrote the logic. They cover the safe ruff autofix pass, the collections.abc import move, the f-string !s cleanup, lazy log message construction, the LIT010 and LIT011 Final and frozen-parameter rollout, ruff coverage for litellm/types, and the inert type: ignore strip. Smaller ratchet commits are left out on purpose: each touches a few hundred lines at most, so listing them would grow the file faster than it buys back blame accuracy
Greptile SummaryThis PR extends
Confidence Score: 4/5The PR appears safe to merge, with only a non-blocking repository-guideline issue concerning the newly added descriptive comments. The revision hashes resolve and follow the existing blame-ignore format; the only accepted concern is that their seven new description lines violate the repository’s comment policy. Files Needing Attention: .git-blame-ignore-revs
|
| Filename | Overview |
|---|---|
| .git-blame-ignore-revs | Adds seven valid ignored revisions in the established format, but the new descriptive comment lines conflict with the repository-wide prohibition on adding comments without an explicit request. |
Reviews (1): Last reviewed commit: "chore: ignore the mechanical lint and ty..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| # style: unify ruff format width on 120 (#31518) | ||
| 48b5a5a0cc5a694a11219416ee0b6eb6e620e74e | ||
|
|
||
| # refactor(imports): move collections.abc names out of typing (#35495) |
There was a problem hiding this comment.
The seven newly added revision descriptions conflict with the repository-wide instruction prohibiting new comments without an explicit user request, creating a non-blocking contribution-guideline violation across this entry and the descriptions on lines 24, 27, 30, 33, 36, and 39.
Context Used: CLAUDE.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
This file's format is comment-labeled SHAs; all four existing entries do it. The no-comments rule covers code, and this is a plaintext manifest
There was a problem hiding this comment.
You're right — the no-comments instruction targets code, and .git-blame-ignore-revs is a plaintext manifest whose format is comment-labeled SHAs. The existing four entries all follow the same pattern, and git itself documents this format. My comment was a misapplication of that rule.
Shall I remember this as a rule: "The no-comments policy applies to source code only; plaintext config/manifest files like .git-blame-ignore-revs that use comments as their native format are exempt"?
There was a problem hiding this comment.
Yes, please remember it. Config and manifest files where comments are the native labeling format are exempt; the rule targets source code
There was a problem hiding this comment.
Got it — rule created. I won't flag comments in plaintext manifest files like .git-blame-ignore-revs again.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
2 participants
TLDR
Problem this solves:
git blamenow points at those sweeps, not real authorslitellm/How it solves it:
.git-blame-ignore-revsRelevant issues
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)There is no code here to test, so the tests box stays unchecked rather than get a checkmark it did not earn. What the file claims is checkable though, and the proof section below verifies it: every listed SHA resolves, and blame attribution actually moves off the sweep and back onto the commit that wrote the logic
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
Captured on this branch at
2c91166d3, with the parentb66d4e696standing in for the before state (the ignore file is what changes, so before is simply blame with the entries absent)The end user here is anyone opening GitHub Blame or running
git blamelocally, so the proof is that command, before and afterBefore,
litellm/router.py:273is credited to the LIT010: Finalsweep even though the line's logic predates it:After wiring the file in, the line goes back to the commit that wrote it:
Same story for the lazy log message sweep in
litellm/_redis.py, before:and after:
Every new SHA also resolves to a real commit that is an ancestor of
litellm_internal_staging, so blame will not choke on a bad entry:(the three pre-existing entries older than this shallow clone's history are skipped by the loop, not failures)
Type
🚄 Infrastructure
Changes
Seven commits landed since the last entry (
#31518) that rewrote a large fraction of the tree without changing what it does. Together they touch roughly 4,700 files and 162k lines, which is enough to make blame onlitellm/mostly useless: annotations, imports, f-strings, and log call shapes all moved, so the sweep author sits on top of lines they never wrote397e8e49collections.abcnames out oftyping(#35495)b604e2b27b2d3440!sconversion flags from f-strings (#35546)c9887a1f2708620dFinalon locals, freeze function parameters (#35807)4e32a8bflitellm/typesfrom the ruff lint exclusion (#35926)338e4111type: ignorecomments, zero LIT009/010/011 headroom (#35928)c9887a1fis the one entry not from the lint budget series, and it earns its place the same way: aperf:change applied uniformly across 419 files, rewriting f-string log calls into%-args without touching a line of logicLeft out on purpose are the smaller budget-ratchet commits (
075babd0,cdfefd7f,6c76f5f9and friends), each a few hundred lines at most; thechore(typing)commits that replacedAnywith TypedDicts and Protocols, since those are design decisions worth blaming; the recurring Next.js build artifact commits, because blame on generated output means nothing either way; and72bcb748, which is a pure deletion and so leaves no surviving lines to misattribute. Listing that long tail would grow the file faster than it buys back accuracyOrdering and comment style follow what was already in the file: chronological, one blank-line-separated block per entry, commit subject and PR number above the full 40-character SHA
QA runbook
Final Attestation