Repository navigation
Add Swift file length budget CI guard - #3223
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR adds Swift file-length budget enforcement to the CI pipeline by introducing a Python script that validates Swift source files against a line-count budget, integrating it into the CI workflow with guard checks, and providing comprehensive shell tests for validation. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b5a3bef89
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Greptile SummaryThis PR introduces a Swift file-length budget guard: a Python script ( Confidence Score: 4/5Safe to merge; CI pass/fail logic is correct and all findings are P2 display/coverage issues. All three findings are P2: an off-by-one in the delta label for untracked files at exactly the threshold, a misleading scripts/swift_file_length_budget.py (delta display logic) and tests/test_ci_swift_file_length_budget.sh (missing untracked-new-file coverage) Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[CI: workflow-guard-tests] --> B[test_ci_swift_file_length_budget.sh]
B --> C[swift_file_length_budget.py]
C --> D[collect_file_lengths\nscans Sources/ CLI/ Packages/\ncmuxTests/ cmuxUITests/]
D --> E[tracked_file_lengths\nfilter >= threshold 500]
E --> F{--write-budget?}
F -- yes --> G[write_budget\n.github/swift-file-length-budget.tsv]
F -- no --> H[load_budget\n.github/swift-file-length-budget.tsv]
H --> I[compare_budget\nactual vs allowed]
I --> J{violations?}
J -- new file >= threshold\nnot in budget --> K[exit 1\nBudget exceeded]
J -- tracked file\ngrew past allowed --> K
J -- none --> L[exit 0\nBudget respected]
L --> M[print reduction hints\nif budget can shrink]
Reviews (1): Last reviewed commit: "ci: add Swift file length budget" | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/swift_file_length_budget.py`:
- Around line 190-211: The code currently uses args.budget as-is so relative
budget paths are resolved from the current CWD; update the main flow so
args.budget is resolved against repo_root (the Path stored in repo_root) before
any exists/read/write operations: after repo_root = args.repo_root.resolve(...)
compute a resolved_budget Path by joining repo_root with args.budget when
args.budget is relative (or always replace args.budget with repo_root /
args.budget and .resolve(strict=False)), then use that resolved args.budget for
write_budget, exists(), load_budget and the printed messages so all budget
reads/writes are rooted at the selected repo_root.
- Around line 66-79: The parser currently overwrites duplicate relative-path
rows silently; modify the reader so after parsing rel_path but before assigning
budget[rel_path] it checks if rel_path already exists in the budget dict and if
so raises a ValueError like f"{path}:{line_number}: duplicate budget entry for
{rel_path!r}" to fail early; locate the code that uses variables budget, path,
line_number, rel_path, count (the block that currently does budget[rel_path] =
count) and add this duplicate check there.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 75e55190-b7e1-4ff8-b386-a70922cbf713
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (3)
.github/workflows/ci.ymlscripts/swift_file_length_budget.pytests/test_ci_swift_file_length_budget.sh
Summary:\n- Add a Swift file length budget for cmux-owned Swift files at or above 500 lines.\n- Add a guard script that fails when tracked files grow or new files cross the threshold without an explicit budget update.\n- Wire the guard into workflow-guard-tests.\n\nTesting:\n- python3 -m py_compile scripts/swift_file_length_budget.py\n- python3 scripts/swift_file_length_budget.py --budget .github/swift-file-length-budget.tsv\n- ./tests/test_ci_swift_file_length_budget.sh\n- ./tests/test_ci_swift_warning_budget.sh\n- ./tests/test_ci_self_hosted_guard.sh\n- ./tests/test_ci_unit_test_spm_retry.sh\n- ./tests/test_ci_scheme_testaction_debug.sh\n- ./tests/test_ci_create_dmg_pinned.sh\n- node scripts/release_asset_guard.test.js\n- ./tests/test_ci_ghosttykit_checksum_present.sh\n\nNote:\n- ./tests/test_ci_ghosttykit_checksum_verification.sh fails locally on macOS because the fixture tar includes ._GhosttyKit.xcframework, but this existing guard passed in CI on #3220.
Summary by cubic
Adds and enforces a Swift file length budget for cmux-owned files ≥500 lines. CI fails if a tracked file grows or a new file crosses the threshold without updating the budget.
New Features
Migration
python3 scripts/swift_file_length_budget.py --budget .github/swift-file-length-budget.tsv --write-budget
Written for commit 9f07bb7. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
Chores
Tests