From 8f0b29b015a962cb91a2b1ce334b303e30c23c9f Mon Sep 17 00:00:00 2001 From: Yassin Kortam Date: Sat, 23 May 2026 10:36:46 -0700 Subject: [PATCH] fix(hooks): address Greptile review on PR #28703 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves two findings from the automated code review: 1. CONTRIBUTING.md: shrink the new Conventional Commits / Branches section to a 2-line pointer at docs.litellm.ai. Per the team convention, the full documentation lives in the litellm-docs repo — see BerriAI/litellm-docs#208 for the companion change that adds the section to docs/extras/contributing_code.md. 2. .githooks/commit-msg: tighten the subject regex to also reject an uppercase first letter in the description. CI's subjectPattern is ^(?![A-Z]).+$ so the previous local hook would accept 'feat: Add thing' which would then fail the PR-title check. The local hook is now the strictly tighter of the two gates. Test cases extended to cover both the new rejection and the digit/symbol-start cases that remain allowed. Resolves LIT-3306 Co-Authored-By: Claude Opus 4.7 (1M context) --- .githooks/commit-msg | 7 +++- CONTRIBUTING.md | 60 +--------------------------- tests/test_litellm/test_git_hooks.py | 26 ++++++++++++ 3 files changed, 33 insertions(+), 60 deletions(-) diff --git a/.githooks/commit-msg b/.githooks/commit-msg index 8f927df492d..b64e38a2286 100755 --- a/.githooks/commit-msg +++ b/.githooks/commit-msg @@ -43,7 +43,11 @@ case "$subject" in esac ALLOWED_TYPES="feat|fix|docs|style|refactor|perf|test|build|ci|chore|revert" -PATTERN="^(${ALLOWED_TYPES})(\([^)]+\))?!?: .+" +# Description must not start with an uppercase letter — kept in sync with the +# subjectPattern in .github/workflows/conventional-commits.yml so the local +# hook is the strictly tighter of the two gates. (Without this guard, a commit +# like "feat: Add thing" passes locally but fails the PR-title CI check.) +PATTERN="^(${ALLOWED_TYPES})(\([^)]+\))?!?: [^A-Z].*" if printf '%s' "$subject" | grep -Eq "$PATTERN"; then exit 0 @@ -55,6 +59,7 @@ cat >&2 <()!: + (description must start with a lowercase letter) Allowed types: feat, fix, docs, style, refactor, perf, test, build, ci, chore, revert Examples: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4d651a4a0cf..cfa05e3e00f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -55,65 +55,7 @@ That's it! Your local development environment is ready. ## Commit and Branch Conventions -LiteLLM enforces two community specs: - -- **Commits** follow [Conventional Commits 1.0.0](https://www.conventionalcommits.org/en/v1.0.0/) — `()!: ` -- **Branches** follow [Conventional Branches](https://conventional-branch.github.io/) — `/` - -### Commit message format - -``` -()!: - - - - -``` - -Allowed `` values: `feat`, `fix`, `docs`, `style`, `refactor`, `perf`, `test`, `build`, `ci`, `chore`, `revert`. Add `!` before `:` for breaking changes. - -Examples: -``` -feat(router): add weighted round-robin strategy -fix(bedrock): decouple STS region from aws_region_name -chore(deps): bump black to 26.3.1 -refactor!: drop Python 3.8 support -``` - -PR titles must follow the same format — squash-merge uses the PR title as the commit subject, and a CI check validates it. - -### Branch naming - -Format: `/` where `` is one of `feature`, `bugfix`, `hotfix`, `release`, `chore`. - -Examples: -``` -feature/weighted-round-robin -bugfix/streaming-empty-chunks -chore/bump-black -hotfix/auth-bypass -``` - -Branches always allowed (bypass the check): `main`, `litellm_internal_staging`, `dependabot/*`, `gh-readonly-queue/*`. - -### Installing the hooks - -The hooks live in `.githooks/` and are opt-in. Run once per clone: - -```bash -make install-hooks -``` - -This sets `core.hooksPath=.githooks` for the local repository. The hooks run on `git commit` (subject validation) and `git push` (branch validation). - -In a rare emergency you can bypass them per-command: - -```bash -git commit --no-verify -m "..." -git push --no-verify -``` - -To uninstall: `git config --unset core.hooksPath`. +Commits follow [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) and branches follow [Conventional Branches](https://conventional-branch.github.io/). Run `make install-hooks` once per clone to enable the local git hooks that enforce these — see the [contributor docs](https://docs.litellm.ai/docs/extras/contributing_code#commit-and-branch-conventions) for the full type list, examples, the protected-branch bypass list, and how to opt out. ### 2. Development Workflow diff --git a/tests/test_litellm/test_git_hooks.py b/tests/test_litellm/test_git_hooks.py index 53a1224b43e..c6980d1f44a 100644 --- a/tests/test_litellm/test_git_hooks.py +++ b/tests/test_litellm/test_git_hooks.py @@ -104,6 +104,13 @@ def test_commit_msg_accepts_conventional_subjects(tmp_path, subject): "ux: thing", # unknown type "Feat(router): capital type", # types are lowercase "feat(router):", # empty description + # Description must start with a lowercase letter — kept in sync with + # the CI workflow's subjectPattern so the local hook never accepts a + # subject that CI will later reject. + "feat: Add thing", + "fix(router): Decouple something", + "chore: BUMP deps", + "feat: A", ], ) def test_commit_msg_rejects_invalid_subjects(tmp_path, subject): @@ -115,6 +122,25 @@ def test_commit_msg_rejects_invalid_subjects(tmp_path, subject): assert "Conventional Commits" in result.stderr +@pytest.mark.parametrize( + "subject", + [ + # Lowercase letter — the common case. + "feat: lowercase start is fine", + # The CI's `^(?![A-Z]).+$` rejects only uppercase A-Z, so digits and + # symbols are still allowed; mirror that behavior here. + "feat: 1-based indexing now works", + "fix(deps): @types/node bump", + ], +) +def test_commit_msg_accepts_non_uppercase_starts(tmp_path, subject): + result = _run_commit_msg(subject, tmp_path) + assert result.returncode == 0, ( + f"hook rejected a valid non-uppercase-start subject:\n" + f" subject: {subject!r}\n stderr: {result.stderr}" + ) + + @pytest.mark.parametrize( "subject", [