feat(ci): assert .github/workflows holds only workflows, correctly named - #37616
Conversation
Greptile SummaryThe PR adds a CI hygiene check that ensures top-level workflow files use the expected extension and that reusable-workflow names agree with their triggers.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| .github/scripts/assert_workflow_dir_hygiene.py | Adds validation for stray files, noncanonical workflow extensions, and reusable-workflow naming consistency. |
| .github/workflows/ci-coverage.yml | Adds the workflow-directory hygiene check to the existing CI coverage job. |
| .github/workflows/test-model-map.yml | Renames the existing workflow to the repository’s canonical .yml extension without changing its contents. |
| tests/test_litellm/test_assert_workflow_dir_hygiene.py | Adds focused coverage for trigger parsing, naming rules, directory traversal, and current repository state; the previously reported annotation issue is fixed. |
Reviews (4): Last reviewed commit: "fix(ci): report a .yaml workflow as a na..." | Re-trigger Greptile
|
Added the Final annotations. Worth noting the type-discipline gate targets litellm/ only, so tests are convention-enforced, not gated. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
174838c to
7a0d3b9
Compare
tin-berri
left a comment
There was a problem hiding this comment.
New WF001/WF004 checks catch non-workflow files or misnamed workflows landing in .github/workflows/ (the exact class of issue #37586 found manually). Confirmed the rename is safe — check contexts come from workflow/job names not filenames, so test-model-map.yaml -> .yml doesn't break the required check. Red-first repro (putting the stray back, showing the specific test fails) is solid proof. Scope stays to its stated four files now that #37586 merged. Deliberately doesn't scope-creep into enforcing the kebab-case/prefix convention (29 more files) — correctly left for its own PR. CI green.
GitHub reads .yml and .yaml alike, so WF001 telling you to move a valid .yaml workflow to .github/scripts/ was wrong advice. WF001 now covers only files that are not workflows at all, and the .yml spelling this directory keeps moves to WF004, which says to rename rather than relocate. WF001 also never looked into subdirectories, since GitHub does not read them either; the message now says so. The directory is injected rather than read off a module constant, so the cases are testable without monkeypatching.
843f253 to
c6d0e72
Compare
TLDR
Problem this solves:
.github/workflows/, so nothing else belongs theretest-model-map.yamlis a real workflow wearing the extension this directory does not use_prefix means "reusable", but only by habitHow it solves it:
WF001fails a top-level file that is not a workflow at all, in either extensionWF004fails a real.yamlworkflow separately, and says to rename rather than relocate_prefix, or a_one is uncallableUser Flow
Before: a helper script goes into the workflows directory and nobody notices
.github/workflows/backfill_prices.pynext to the workflow that runs it, because that is where the workflow isAfter: the same PR fails, and says where the file goes
.github/workflows/assert-ci-coveragefails withWF001naming the file and pointing at.github/scripts/or a subdirectoryrun:line keeps working unchangedRelevant issues
Linear ticket
Pre-Submission checklist
uv run pytest tests/test_litellm/<your_test_file>.py -vScreenshots / Proof of Fix
Before (03b7438)
.yaml:After (3e825a7)
Green once the stray is renamed
The census is unaffected
Red-first on the repo-state tests
Unit tests
Type
🆕 New Feature
🧹 Refactoring
✅ Test
Caveats
test-model-map.ymlis not a required check, and its job id does not changecreate-release-branch.ymlstays as it istest-/lint-/scan-prefix parts of the convention are not enforced here; 29 of 52 files would need renaming and that wants its own PRFinal Attestation