docs: audit linting config; pin Ruff version and rule selection - #38
canstralian wants to merge 2 commits into
Conversation
Captures the current Python and shell lint surface, identifies determinism gaps (no Ruff project config, unpinned tool versions, no .editorconfig), and applies the lowest-risk fixes that preserve today's zero-issue Ruff baseline: - pyproject.toml pins target-version, line-length, an explicit rule selection matching today's stock defaults (E4/E7/E9/F), excludes for the dashboard subtree and the future distro/ build dir, and a formatter style block. - requirements-dev.txt pins ruff==0.15.8, pytest>=8,<9, pyyaml>=6,<7. - lint.yml and ci.yml install via requirements-dev.txt instead of ad-hoc pip install lines. - .editorconfig declares LF line endings, final newline, indent rules. - docs/LINTING_AUDIT.md documents findings (incl. CI workflow triplication and deferred stricter rule sets) and a phased follow-up. No Python source files modified. No CI job added or removed. ruff check . still reports zero issues after this commit. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RoWXAy79v3vxQ1CWjkGA9h
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds repository-wide editor settings, pinned dev dependencies, Ruff configuration, CI/lint workflow install updates, and a linting audit document with findings and follow-up items. ChangesLinting configuration and CI updates
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request establishes a baseline linting configuration by adding an .editorconfig, a pyproject.toml for Ruff, a requirements-dev.txt file, and a comprehensive linting audit document. The review feedback correctly identifies two issues: first, the pinned Ruff version 0.15.8 does not exist on PyPI and will cause CI failures; second, the selected Ruff rules restrict PEP 8 checks much more than Ruff's actual stock defaults of ["E", "F"].
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| @@ -0,0 +1,3 @@ | |||
| ruff==0.15.8 | |||
There was a problem hiding this comment.
The Ruff version 0.15.8 does not exist on PyPI. This will cause the CI installation step (pip install -r requirements-dev.txt) to fail with a No matching distribution found error. Please pin to a valid, existing version of Ruff, such as 0.5.8.
ruff==0.5.8
There was a problem hiding this comment.
This claim is incorrect — ruff==0.15.8 does exist on PyPI. Verified locally against the live PyPI index:
$ pip index versions ruff | head -2
ruff (0.15.20)
Available versions: 0.15.20, 0.15.19, 0.15.18, ..., 0.15.9, 0.15.8, 0.15.7, ...
$ pip install --dry-run --ignore-installed 'ruff==0.15.8'
Collecting ruff==0.15.8
Downloading ruff-0.15.8-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl.metadata (26 kB)
Would install ruff-0.15.8
Also verifiable at https://pypi.org/project/ruff/0.15.8/. 0.15.20 is the current latest; 0.5.8 is from mid-2024 and predates most of the rule stabilizations we'd want. Keeping the pin as ruff==0.15.8.
Generated by Claude Code
| ] | ||
|
|
||
| [tool.ruff.lint] | ||
| select = ["E4", "E7", "E9", "F"] |
There was a problem hiding this comment.
Ruff's actual stock default rule selection is ["E", "F"] (which enables all E and F rules). Specifying ["E4", "E7", "E9", "F"] restricts the linter to a much smaller subset of PEP 8 rules (disabling E1xx, E2xx, E3xx, E5xx, etc.). If the goal is to match Ruff's stock defaults, please use ["E", "F"].
| select = ["E4", "E7", "E9", "F"] | |
| select = ["E", "F"] |
There was a problem hiding this comment.
Ruff's stock default select is ["E4", "E7", "E9", "F"], not ["E", "F"]. Verified empirically with ruff 0.15.8 against a file that exercises all E sub-categories:
$ ruff check --isolated --no-cache x.py # no config, no --select
E711 Comparison to `None` should be `cond is None`
--> x.py:5:6
Found 1 error.
$ ruff check --isolated --select E,F --no-cache x.py
E501 Line too long (95 > 88)
--> x.py:3:89
E711 Comparison to `None` should be `cond is None`
--> x.py:5:6
Found 2 errors.
The E501 line-too-long finding only appears when the selection is explicitly widened to ["E", "F"]. Under Ruff's stock defaults, the E5xx category is disabled, which is what the audit doc claims and what this PR's pyproject.toml codifies. See also https://docs.astral.sh/ruff/rules/#pycodestyle-e-w and https://docs.astral.sh/ruff/settings/#lint_select — the "Default value" listed for lint.select is ["E4", "E7", "E9", "F"].
Switching to ["E", "F"] would surface 16 new E501 findings in this repo (per the --select E,F,W,I,B,UP probe captured in docs/LINTING_AUDIT.md §3), so it's a policy change, not a determinism fix. Keeping the selection as-is.
Generated by Claude Code
|
Note on two failing checks (not caused by this PR):
Both are pre-existing infrastructure issues unrelated to the lint config changes in this PR and should be tracked as their own follow-ups. Generated by Claude Code |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
docs/LINTING_AUDIT.md (2)
97-97: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse fully-qualified paths for all workflow file references.
F-3 and F-7 quote
ci.ymlandtests.ymlwithout the.github/workflows/prefix, whilelint.ymland every other reference in the document (e.g., line 85, lines 181–182) use the full path. Standardize to.github/workflows/ci.ymland.github/workflows/tests.ymlfor consistency.Also applies to: 159-159
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/LINTING_AUDIT.md` at line 97, The workflow file references in the audit doc are inconsistent because ci.yml and tests.yml are not fully qualified like lint.yml and the other entries. Update the references in the affected section to use the same .github/workflows/ prefix, matching the naming used elsewhere in docs/LINTING_AUDIT.md and the workflow file list.
26-26: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd language specifiers to fenced code blocks.
Two code blocks are missing language identifiers, which triggers
MD040and degrades syntax highlighting.
- Line 26: use
textfor the file list.- Line 43: use
shellfor the command transcript.📝 Proposed fixes
-``` +```text adapters/airtable/scope_mapper.py ...-``` +```shell $ ruff check . ...Also applies to: 43-43
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/LINTING_AUDIT.md` at line 26, Add language identifiers to the fenced code blocks in the LINTING_AUDIT content so Markdown linting passes and syntax highlighting works. Update the code fence showing the file list to use the text specifier, and update the command transcript fence to use shell; locate the affected fences in the document around the existing markdown examples and apply the same fix wherever those unlabeled blocks appear.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/LINTING_AUDIT.md`:
- Line 24: The Python source inventory in the linting audit is inconsistent
because the “16 files” count does not match the 13 files listed below it. Update
the summary in the audit document so the parenthetical count matches the actual
enumerated files, or add the missing three Python filenames to the inventory if
the count is correct.
---
Nitpick comments:
In `@docs/LINTING_AUDIT.md`:
- Line 97: The workflow file references in the audit doc are inconsistent
because ci.yml and tests.yml are not fully qualified like lint.yml and the other
entries. Update the references in the affected section to use the same
.github/workflows/ prefix, matching the naming used elsewhere in
docs/LINTING_AUDIT.md and the workflow file list.
- Line 26: Add language identifiers to the fenced code blocks in the
LINTING_AUDIT content so Markdown linting passes and syntax highlighting works.
Update the code fence showing the file list to use the text specifier, and
update the command transcript fence to use shell; locate the affected fences in
the document around the existing markdown examples and apply the same fix
wherever those unlabeled blocks appear.
🪄 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: 7d03729f-a379-44a2-a280-49899aebb8b8
📒 Files selected for processing (6)
.editorconfig.github/workflows/ci.yml.github/workflows/lint.ymldocs/LINTING_AUDIT.mdpyproject.tomlrequirements-dev.txt
… workflow paths Fixes on docs/LINTING_AUDIT.md raised in PR #38 review: - Inventory count vs list mismatch: parenthetical said 16 files but the list enumerated 13. The 16 count is correct (ruff sees three __init__.py files too). Extended the list to include them and clarified as "13 modules + 3 __init__.py". - Added language identifiers (text, console) to two fenced code blocks (MD040). - Normalized workflow file references to their full .github/workflows/<name>.yml path in F-3, F-7, and the follow-up section for consistency with the rest of the doc. No content change. ruff check . still reports zero issues. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RoWXAy79v3vxQ1CWjkGA9h
2 participants
Summary
docs/LINTING_AUDIT.mdwith 8 findings ranked by operational impact.ruff formatadoption, stricter rule sets, yamllint) and documents each as a recommended follow-up.What changed
pyproject.tomltarget-version = "py312", explicitselect = ["E4","E7","E9","F"](matches stock defaults),line-length = 100,extend-excludeforvectors/dashboard+distro.requirements-dev.txtruff==0.15.8,pytest>=8,<9,pyyaml>=6,<7..editorconfig.github/workflows/lint.ymlpip install ruff→pip install -r requirements-dev.txt..github/workflows/ci.ymlpip install ruff pytest pyyaml→pip install -r requirements-dev.txt.docs/LINTING_AUDIT.mdHeadline findings (full details in
docs/LINTING_AUDIT.md)lint.yml,ci.yml,tests.ymltriplicateruff/shellcheck/pytest. Not fixed — collapsing may break branch protection. Owner decision.ruff format --check .says 13 of 16 Python files would reformat. Not fixed — wants its own one-shot reformat commit.E,F,W,I,B,UP) would surface 60 findings, 37 auto-fixable. Not fixed — proposed 4-step phased adoption.yamllint..editorconfigmissing. Fixed.Test plan
ruff check .reportsAll checks passed!locally with the newpyproject.toml.git diffshows no Python source changes.Lintjob green on this PR.CIjob green on this PR.Testsjob green on this PR.🤖 Generated with Claude Code
https://claude.ai/code/session_01RoWXAy79v3vxQ1CWjkGA9h
Generated by Claude Code
Summary by CodeRabbit
Documentation
Chores