Skip to content

Import episodic lint gates - #67

Merged
leynos merged 7 commits into
mainfrom
feat/pylint-pypy-lint-import
May 16, 2026
Merged

Import episodic lint gates#67
leynos merged 7 commits into
mainfrom
feat/pylint-pypy-lint-import

Conversation

@leynos

@leynos leynos commented May 15, 2026

Copy link
Copy Markdown
Owner

Summary

This branch imports the lint policy used by leynos/episodic so Lading runs Ruff first and then a focused Pylint pass through the PyPy shim. It raises the project lint baseline while keeping existing large-module and test-scaffolding debt explicit instead of folding broad refactors into the tooling change.

No linked issue or roadmap task was identified for this branch.

Review walkthrough

  • Start with Makefile to see the new pylint-pypy-shim command wiring and the second lint tier after Ruff.
  • Then review pyproject.toml for the imported Ruff selections, Pylint message allow-list, local thresholds, and formatter exclusions for documentation examples.
  • Finish with representative annotation migrations such as lading/commands/publish_plan.py, plus the corresponding test updates under tests, to verify the typing banned-api rule is satisfied without behaviour changes.

Validation

  • make lint: passed, including Ruff and PyPy-backed Pylint.
  • make check-fmt: passed.
  • make typecheck: passed.
  • make test: passed, 432 tests.
  • git diff --check: passed.

Notes

  • The imported Pylint pass keeps module size, argument-count, and resource-lifecycle thresholds explicit, but avoids forcing unrelated module splits in this branch.
  • Ruff excludes docs and .rules from formatter traversal so existing prose examples are not rewritten as part of this tooling import.

References

Import the stricter Ruff rule set and add the PyPy-backed Pylint shim as
a second lint tier after Ruff. Keep local thresholds explicit where the
existing codebase would otherwise require unrelated module splits.

Migrate typing-only collection annotations to `collections.abc` so the
new banned typing aliases rule can run cleanly.
@coderabbitai

coderabbitai Bot commented May 15, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: dc301e78-b896-4330-b265-bdff10c232af

📥 Commits

Reviewing files that changed from the base of the PR and between b323604 and 59cb0be.

📒 Files selected for processing (6)
  • .rules/python-exception-design-raising-handling-and-logging.md
  • docs/cmd-mox-usage-guide.md
  • docs/developers-guide.md
  • docs/lading-design.md
  • docs/scripting-standards.md
  • docs/users-guide.md

Summary

This pull request imports episodic lint gates, adding a two-tier lint workflow where Ruff runs first and, if it passes, a focused Pylint pass executes via a PyPy-backed pylint-pypy-shim. The change migrates many type-hints from typing.* to collections.abc to satisfy the new banned-typing rules and updates Makefile/tooling wiring so uv is resolved via a UV variable.

Key Changes

  • Makefile

    • Adds UV resolution: UV ?= $(shell command -v uv 2>/dev/null || printf '%s/.local/bin/uv' "$$HOME") and replaces literal uv uses with $(UV).
    • Adds Pylint variables: PYLINT_PYTHON, PYLINT_TARGETS, PYLINT_PYPY_SHIM_REF, PYLINT_PYPY_SHIM and defines PYLINT to run the pinned pylint-pypy shim via $(UV) tool run.
    • Updates targets (.venv, build, build-release, lint, test) to depend on/use $(UV) and to run the two-tier lint (ruff then $(PYLINT) $(PYLINT_TARGETS)).
  • pyproject.toml

    • Expands Ruff configuration (preview enabled, py313 target, extended excludes including .rules and docs, formatter excludes).
    • Broadens Ruff lint select/ignore lists, tightens McCabe/embedded pylint limits, and adds flake8-tidy-imports.banned-api entries banning many typing.* APIs (migration guidance messages).
    • Adds Pylint configuration with explicit sizing thresholds (max-module-lines, max-args, max-locals, max-statements) and a curated enable-list (pattern: disable-all, then enable selected checks).
  • Code changes (non-behavioural)

    • Systematic migration from typing.* to collections.abc (imported as cabc) across many modules and tests (core code, command modules, tests and fixtures). Examples: lading/cli.py, lading/commands/*, lading/utils/process.py, lading/workspace/{metadata,models}, and numerous tests under tests/, matching the Ruff/Pylint banned-typing policy.
    • Minor formatting/docstring and test whitespace adjustments; one refactor in tests/e2e/helpers/git_helpers.py replaces a try/except with contextlib.suppress(FileNotFoundError).
  • Documentation

    • docs/developers-guide.md: new "Build environment" and "Linting workflow" sections documenting UV resolution, the two-tier lint gate, the pylint-pypy-shim pin, and the pyproject.toml policy.
    • Various rule documents and guides updated (.rules and docs/*) to align with policy and examples.

Validation

  • make lint (Ruff + PyPy-backed Pylint), make check-fmt, make typecheck, make test (432 tests) and git diff --check all passed locally per PR description.

Files of note (samples)

  • Makefile: PYLINT_PYPY_SHIM_REF and PYLINT definitions present.
  • pyproject.toml: [tool.ruff] and [tool.pylint.*] configuration blocks added/expanded.
  • Many modules now import collections.abc as cabc (see lading/cli.py, lading/commands/, lading/utils/process.py, lading/workspace/ and many test files).

Notes

  • No new execplan document was added.
  • Imported Pylint pass preserves explicit thresholds (module size, argument count, locals/statements) and uses a focused allow/enable list to avoid broad refactors while enforcing hygiene and deprecation migrations (notably for typing APIs).

Walkthrough

Replace typing-based annotations with collections.abc across core modules and tests; add UV autodetection and a PyPy-backed Pylint invocation in the Makefile; expand Ruff and Pylint configuration in pyproject.toml.

Changes

Type annotation migration and linting infrastructure

Layer / File(s) Summary
Makefile: UV detection and Pylint tooling
Makefile
Add `UV ?= $(shell command -v uv 2>/dev/null
pyproject.toml: Ruff and Pylint configuration expansion
pyproject.toml
Enable Ruff preview, set target-version to py313, expand excludes and select/ignore lists; move per-file-ignores to a table; add flake8-tidy-imports banned-api for deprecated typing.*; tighten complexity thresholds; expand Pylint sizing limits and enabled messages.
Code & tests: typing → collections.abc and small refactors
lading/*, tests/*, lading/commands/publish_execution.py
Replace typing-based generics with collections.abc aliases (imported as cabc) across library and tests for Sequence, Mapping, Iterator, Callable, Collection, Set; update related annotations and a few docstring/headings; minor test refactors (blank-line separators, use contextlib.suppress, small reformatting).

Possibly related PRs

Sail the hints from typing’s shore,
collections.abc opens the door.
UV wakes the linting tune,
Ruff first, then PyPy Pylint soon.
Tests and code now speak as four.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/pylint-pypy-lint-import

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @leynos, you have reached your weekly rate limit of 2500000 diff characters.

Please try again later or upgrade to continue using Sourcery

codescene-delta-analysis[bot]

This comment was marked as outdated.

@leynos
leynos marked this pull request as ready for review May 15, 2026 11:40

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 71-73: The lint target currently lists only ruff as a prerequisite
but invokes PYLINT via the UV tool, so add uv as an explicit prerequisite to the
lint target declaration (update the target header for lint to include uv
alongside ruff) so the Makefile will fail fast if UV is missing; modify the
"lint" target line (the one defining lint: ruff ...) to include uv.
🪄 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: dbc295db-a640-4a64-aa77-109502dbc743

📥 Commits

Reviewing files that changed from the base of the PR and between 28c6cf4 and 1c44627.

📒 Files selected for processing (41)
  • Makefile
  • lading/cli.py
  • lading/commands/bump.py
  • lading/commands/bump_docs.py
  • lading/commands/bump_toml.py
  • lading/commands/publish.py
  • lading/commands/publish_execution.py
  • lading/commands/publish_plan.py
  • lading/config.py
  • lading/utils/process.py
  • lading/workspace/metadata.py
  • lading/workspace/models.py
  • pyproject.toml
  • tests/bdd/steps/metadata_fixtures.py
  • tests/bdd/steps/test_bump_steps.py
  • tests/bdd/steps/test_commands_catalogue_steps.py
  • tests/bdd/steps/test_publish_given_steps.py
  • tests/bdd/steps/test_publish_helpers.py
  • tests/bdd/steps/test_publish_infrastructure.py
  • tests/bdd/steps/test_publish_when_steps.py
  • tests/bdd/steps/test_workspace_metadata_steps.py
  • tests/conftest.py
  • tests/e2e/conftest.py
  • tests/e2e/helpers/e2e_steps_helpers.py
  • tests/e2e/helpers/git_helpers.py
  • tests/e2e/helpers/workspace_builder.py
  • tests/e2e/steps/test_e2e_steps.py
  • tests/unit/conftest.py
  • tests/unit/publish/conftest.py
  • tests/unit/publish/preflight_test_utils.py
  • tests/unit/publish/test_packaging.py
  • tests/unit/publish/test_preflight_checks.py
  • tests/unit/publish/test_publish_execution_helpers.py
  • tests/unit/publish/test_run_integration.py
  • tests/unit/test_bump_command_internals.py
  • tests/unit/test_cli.py
  • tests/unit/test_publish_formatting.py
  • tests/unit/test_publish_patch_strategy.py
  • tests/unit/test_publish_planning.py
  • tests/unit/test_publish_staging.py
  • tests/unit/test_workspace_models_validation.py

Comment thread Makefile Outdated
leynos added 2 commits May 15, 2026 13:52
Add `uv` as an explicit prerequisite for the lint target because the
Pylint tier is executed through `uv tool run`. This makes the target fail
at the tool-check stage when `uv` is unavailable.
Add developer guidance for the two-tier Ruff and PyPy-backed Pylint gate,
including the Makefile variables and `pyproject.toml` policy locations.
codescene-delta-analysis[bot]

This comment was marked as outdated.

codescene-delta-analysis[bot]

This comment was marked as outdated.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses a literal "uv" in the lint prerequisite and
in the TOOLS list, which breaks the override/fallback in the UV variable; update
references to use the resolved variable $(UV) instead of the literal "uv" so
overrides like `make lint UV=/custom/uv` work correctly—specifically replace the
uv prerequisite on the lint target and the uv entry in the TOOLS variable with
$(UV), keeping existing whitespace and ordering.
🪄 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: 1426ba0e-41a8-46de-91a8-4306016fb7d3

📥 Commits

Reviewing files that changed from the base of the PR and between 1c44627 and fbb3bfd.

📒 Files selected for processing (1)
  • Makefile

Comment thread Makefile Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/developers-guide.md`:
- Around line 42-45: The sentence describing the `lint` target (mentioning
`ruff`, `uv`, and Makefile prerequisites) is missing two commas; add a comma
before "so" in "so it fails during tool checks" and add a comma after
"invocations" in "as well as command invocations" to correctly separate the
clauses and improve readability.
🪄 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: cdbb28f0-078d-4ac1-9461-b0bb674a3501

📥 Commits

Reviewing files that changed from the base of the PR and between fbb3bfd and a4754a9.

📒 Files selected for processing (1)
  • docs/developers-guide.md

Comment thread docs/developers-guide.md Outdated
Use the resolved `UV` variable in the lint prerequisites and tool checks so
Makefile overrides apply consistently. Tidy the developer guide sentence that
explains the lint prerequisites.
codescene-delta-analysis[bot]

This comment was marked as outdated.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
Makefile (1)

5-5: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Restore a resolvable uv prerequisite target.

Fix the dependency graph broken at Line 5. Replacing uv with $(UV) in TOOLS
removes the uv target, while build and test still require uv, which
causes the reported CI failure: “No rule to make target 'uv'”.

Patch
-TOOLS = $(MDFORMAT_ALL) ruff ty $(MDLINT) $(NIXIE) $(UV)
+TOOLS = $(MDFORMAT_ALL) ruff ty $(MDLINT) $(NIXIE) uv
@@
+uv: ## Verify required CLI tools
+	$(call ensure_tool,$(UV))
+
 ifneq ($(strip $(TOOLS)),)
 $(TOOLS): ## Verify required CLI tools
 	$(call ensure_tool,$@)
 endif

Based on learnings: “Future lint additions must be wired through Makefile
prerequisites as well as command invocations to ensure local failures remain
early and clear”.

🤖 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 `@Makefile` at line 5, The Makefile's TOOLS list removed the literal uv
prerequisite by replacing uv with $(UV), breaking targets that still depend on a
literal uv; restore a resolvable prerequisite by adding back the literal "uv"
into the TOOLS assignment (or alternatively ensure the uv target is defined as
$(UV): ... and update dependent targets), and make the dependencies consistent
by updating the build and test targets to depend on the same symbol (either the
literal uv or the variable $(UV)) so the Make dependency graph resolves
correctly; specifically check and adjust the TOOLS variable, the uv/$(UV) target
definition, and the build/test prerequisites (targets named build and test) to
use the same identifier.
🤖 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.

Duplicate comments:
In `@Makefile`:
- Line 5: The Makefile's TOOLS list removed the literal uv prerequisite by
replacing uv with $(UV), breaking targets that still depend on a literal uv;
restore a resolvable prerequisite by adding back the literal "uv" into the TOOLS
assignment (or alternatively ensure the uv target is defined as $(UV): ... and
update dependent targets), and make the dependencies consistent by updating the
build and test targets to depend on the same symbol (either the literal uv or
the variable $(UV)) so the Make dependency graph resolves correctly;
specifically check and adjust the TOOLS variable, the uv/$(UV) target
definition, and the build/test prerequisites (targets named build and test) to
use the same identifier.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: f14a7a78-af61-4830-9c62-d1a66cd24e6a

📥 Commits

Reviewing files that changed from the base of the PR and between a4754a9 and 829a5af.

📒 Files selected for processing (2)
  • Makefile
  • docs/developers-guide.md

Organize the Pylint enable list by rule family so reviewers can scan the
second-tier lint policy more easily. Keep `consider-using-with` enabled in
the runtime hazards group and suppress the one direct `Popen` ownership case
locally where streaming relay threads require explicit lifecycle handling.
codescene-delta-analysis[bot]

This comment was marked as outdated.

Resolve build and test target dependencies through `$(UV)` so
`make` and Makefile validators do not look for a literal `uv` target.
Keep command execution aligned with the same configured path.
codescene-delta-analysis[bot]

This comment was marked as outdated.

Add developer guidance for the Makefile `UV` variable so the
`command -v uv` lookup and `$HOME/.local/bin/uv` fallback are clear.
Wrap existing Markdown lines that blocked the documented commit gate.

@codescene-delta-analysis codescene-delta-analysis Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gates Passed
6 Quality Gates Passed

See analysis details in CodeScene

Absence of Expected Change Pattern

  • lading/lading/commands/publish.py is usually changed with: lading/tests/bdd/steps/test_publish_steps.py

Quality Gate Profile: Pay Down Tech Debt
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant