Run Pylint through the PyPy shim - #10
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Two-tier Python linting workflow with Pylint via PyPy shimThis PR implements a two-tier local Python linting architecture: a fast Ruff pass followed by a reproducible, pinned Pylint run executed through a PyPy-backed shim. The decision, rationale and migration plan are recorded in ADR 004 (docs/adr-004-two-tier-python-linting.md) and developer-facing usage is documented in docs/developers-guide.md. MotivationProvide a single documented lint command that is fast for frequent local use (Ruff) while adding focused Pylint checks for logging safety, pattern-matching correctness, selected code-quality heuristics and size/complexity limits — executed reproducibly via a pinned PyPy shim and Makefile-driven tool wiring. Key changes
ValidationAuthor-reported validation:
Notes & follow-ups
WalkthroughCentralise Makefile tool execution behind ChangesTool Execution & Configuration Modernisation
Sequence Diagram(s)sequenceDiagram
participant Makefile
participant UV as uv
participant UV_ENV as UV_ENV
participant Venv as ".venv"
participant Sync as "uv sync --group dev"
participant Ruff as ruff
participant PylintShim as "pylint-pypy-shim"
participant PyTest as pytest
Makefile->>UV: $(UV_ENV) $(UV) venv --clear
UV->>Venv: create venv
Makefile->>UV: $(UV_ENV) $(UV) sync --group dev
UV->>Sync: install dev deps
Makefile->>UV: $(UV_ENV) $(UV) run ruff check
UV_ENV->>Ruff: execute Ruff
Makefile->>UV: $(UV_ENV) $(UV) tool run --python pypy pylint-pypy-shim
UV_ENV->>PylintShim: execute focused Pylint
Makefile->>UV: $(UV_ENV) $(UV) run pytest
UV_ENV->>PyTest: run tests
Possibly related PRs
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Reviewer's GuideIntegrates a PyPy-backed Pylint pass into the existing uv-driven workflow, tightens Ruff configuration (Python 3.14 target, NumPy docstrings, banned typing generics), and standardizes Makefile tooling so all lint/format/test commands run through the project virtualenv. Flow diagram for updated lint workflow with PyPy-backed Pylintflowchart LR
A[make lint] --> B[build]
B --> C[.venv via UV_ENV and UV]
C --> D[UV sync --group dev]
D --> E[UV run ruff check]
E --> F[UV tool run pylint-pypy]
F --> G[PyPy interpreter]
F --> H[pylint-pypy-shim repo@PYLINT_PYPY_SHIM_REF]
G --> I[Pylint checks beatcue and tests]
H --> I
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
leynos
marked this pull request as ready for review
May 15, 2026 11:41
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![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
🤖 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 `@Makefile`:
- Around line 4-5: The Makefile defines UV and a TOOLS variable but doesn't
include UV in TOOLS, so preflight validation misses the uv binary; update the
TOOLS definition to include the UV variable (reference symbols: UV and TOOLS) so
Makefile targets that run `$(UV_ENV) $(UV)` are validated ahead of execution and
fail with a clear message if uv is missing.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 805a3cce-be08-44f8-8853-d85bf23ebbe8
📒 Files selected for processing (2)
Makefilepyproject.toml
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/adr-004-two-tier-python-linting.md`:
- Around line 138-141: The sentence "Managed PyPy may lag the project's target
Python version. The Pylint configuration disables `syntax-error` so the second
tier remains useful on files it can parse while Ruff and the project type
checker continue to cover the Python target." would read more smoothly with a
comma to separate clauses; update the text (the fragment containing "files it
can parse while Ruff") to "files it can parse, while Ruff" so the clause is
clearly delimited.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f660467e-f51f-4da2-85b5-17c3ad8a3703
📒 Files selected for processing (5)
.gitignoreMakefiledocs/adr-004-two-tier-python-linting.mddocs/developers-guide.mdpyproject.toml
Import the Episodic Ruff lint baseline and add the focused Pylint configuration used there as a second lint tier. Run Ruff from the project environment first, then invoke the pinned `pylint-pypy-shim` tool against `beatcue` and `tests` so the slower Pylint checks stay explicit and reproducible.
Add developer guidance for running `make lint`, overriding the lint Makefile variables, and understanding the imported Episodic policy. Record the Ruff-first and PyPy-shimmed Pylint design in ADR 004 so the lint architecture has an explicit rationale and migration path.
Add a `.deps` stamp so routine formatting and linting reuse an up-to-date synced environment instead of invoking `uv sync` unconditionally. Fail early when `uv` is unavailable, keep the generated dependency stamp out of Git, and group the focused Pylint enable list by purpose so future rule changes are easier to review.
Add a comma in ADR 004 so the PyPy parser caveat separates the Pylint limitation from Ruff and type-checker coverage more clearly.
Use truthiness checks for empty captured stdout and stderr assertions so the tests imported from `main` pass the branch's PyPy-backed Pylint tier. Keep exact stderr comparisons where diagnostics are part of the behaviour.
lodyai
Bot
force-pushed
the
feat/ruff-pylint-pypy-shim
branch
from
May 16, 2026 11:22
e1230f1 to
509ef07
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@Makefile`:
- Around line 93-94: The check-architecture Makefile target currently runs the
UV command directly; guard it by invoking the existing ensure_uv preflight so a
direct make check-architecture fails with the explicit UV error path. Modify the
target named check-architecture to depend on or call ensure_uv before executing
$(UV_ENV) $(UV) run python -m beatcue.architecture (or insert a line to run
ensure_uv at the top of the target) so ensure_uv runs first and provides the
explicit error handling.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b6ab1414-86b3-4d1b-b55b-c52d1caf68e6
📒 Files selected for processing (6)
.gitignoreMakefiledocs/adr-004-two-tier-python-linting.mddocs/developers-guide.mdpyproject.tomltests/test_architecture_cli.py
Run the shared `ensure_uv` check before invoking the architecture checker so `make check-architecture` uses the same explicit missing-UV error path as the other UV-backed targets.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@Makefile`:
- Line 93: The Makefile's check-architecture target depends on .venv but needs
the project installed; update the dependency to .deps so running make
check-architecture ensures `uv sync` (project installation) runs before
executing `python -m beatcue.architecture`; change the dependency from `.venv`
to `.deps` in the `check-architecture:` rule to fix the ModuleNotFoundError.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Make `check-architecture` depend on `.deps` so direct invocations run `uv sync` before executing `python -m beatcue.architecture`.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@Makefile`:
- Around line 33-36: The .deps Makefile rule currently lists only pyproject.toml
and .venv as prerequisites, so changes to the uv.lock lockfile won't trigger the
uv sync; update the .deps target prerequisites to include the lockfile (uv.lock)
so that when uv.lock changes the recipe (calling ensure_uv and "$(UV_ENV) $(UV)
sync --group dev") runs and the .deps stamp is retouched; keep the existing
ensure_uv call and recipe intact (refer to the .deps target, ensure_uv, UV_ENV
and UV symbols).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Add `uv.lock` to the `.deps` prerequisites so `uv sync --group dev` runs again when the lockfile changes and refreshes the dependency stamp.
This was referenced May 16, 2026
This was referenced May 25, 2026
Closed
4 tasks
This was referenced Jul 26, 2026
This was referenced Aug 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This branch imports the Episodic lint policy into BeatCue and adds a second lint tier that runs Pylint through the pinned PyPy shim after Ruff. It keeps the fast Ruff pass first while making the extra Pylint checks reproducible from the standard
make linttarget.It also documents the lint architecture for contributors, records the Ruff-first, PyPy-shimmed Pylint decision in ADR 004, and addresses review feedback by adding a stamped dependency-sync target plus clearer
uvfailure handling.Review walkthrough
.depsguardsuv sync --group dev, howuvis checked before use, and howmake lintruns Ruff before the pinnedpylint-pypy-shimcommand.typing.*APIs, NumPy pydocstyle convention, and grouped focused Pylint message selection.make lint, Makefile variables, the.depsstamp, the Episodic policy, and the lint configuration sections.Validation
mbake validate Makefile: passed; Makefile syntax is valid.make check-fmt: passed; Ruff reported 27 files already formatted.make lint: passed; Ruff reported all checks passed and Pylint rated the code 10.00/10.make typecheck: passed;ty checkreported all checks passed.make test: passed; 1 test passed.make markdownlint: passed.make nixie: passed.make check-fmt UV=/definitely/missing: failed as expected with the explicituverror message.Notes
No roadmap task, issue, or execplan is associated with this branch.
make fmtwas run during the documentation update. Ruff formatting and import sorting passed, but themdformat-allwrapper exits non-zero on pre-existing long-table diagnostics in unrelated Markdown files even though the repository's configuredmake markdownlinttarget passes. Formatter side effects outside the requested files were reverted.