agent-lightning icon indicating copy to clipboard operation
agent-lightning copied to clipboard

🚀 Refactoring Proposal: Adopt Ruff for Integrated Linting and Formatting

Open Borda opened this issue 1 month ago • 8 comments

Motivation (Current Limitations)

Our current setup relies on black (formatting) and isort (import sorting). This leaves two key gaps:

  1. Missing Linting: No automated checks for common bugs, anti-patterns, or adherence to best practices (e.g., Flake8/Pylint rules).

  2. Toolchain Overhead: Managing separate dependencies and execution steps for black and isort adds unnecessary complexity to our CI/CD pipeline and local development loop.

Proposal (Solution)

Integrate Ruff as the single, high-performance tool for linting, import sorting, and formatting. Ruff is an extremely fast, Rust-based utility designed to consolidate the Python code quality pipeline.

Justification: Progressive Improvement

Ruff offers a compelling improvement over the current setup, delivering speed and functionality beyond simple formatting:

Feature Benefit Progressive Change from Current
Extreme Speed Drastically reduces CI/CD runtimes and local pre-commit check times. Improves developer feedback cycles significantly.
Tool Consolidation Replaces isort entirely and is a drop-in formatter for black. Simplifies dependencies and reduces toolchain maintenance.
Code Quality Assurance Instantly enforces 600+ linting rules (Flake8, Pylint subsets) to catch bugs and anti-patterns. Introduces vital quality checks that black and isort do not provide.

Recommended Action Plan

  1. Dependency Update: Add ruff and remove isort.

  2. Configuration: Define [tool.ruff] in pyproject.toml, setting line-length = 88 and enabling a core set of linting rules (e.g., ["E", "F", "W", "I", "UP"]).

  3. Initial Fix: Run ruff check --fix . across the codebase.

  4. CI/CD: Update CI scripts to use ruff check and ruff format --check (or keep black initially during a transition phase).

Next Steps

If approved, I will submit a PR implementing this transition (Phase 1: Ruff for linting/sorting, replacing isort).

Borda avatar Nov 26 '25 08:11 Borda

I haven't discussed with anyone yet and using black/isort is purely my personal choice.

Personally I don't like ruff's one-click experience. I've seen other projects using it, and I've contributed to some of them. Ruff sometimes format the code in a way that I don't like. It could be that they've configured ruff too strictly; but it leaves me with a bad impression on ruff.

Also, do we still need pyright with ruff? If we do, maybe ruff only replaces black and isort and the motivation to switch is not that strong. -- we don't use flake8 or pylint in our project.

ultmaster avatar Nov 27 '25 08:11 ultmaster

I haven't discussed with anyone yet and using black/isort is purely my personal choice.

I think that setting a standard is a good approach

Personally I don't like ruff's one-click experience. I've seen other projects using it, and I've contributed to some of them. Ruff sometimes format the code in a way that I don't like. It could be that they've configured ruff too strictly; but it leaves me with a bad impression on ruff.

I see and understand your point of view. It is like English and dialects; they are almost the same, following the same standard, but a few particular words could differ, and if you insist on one specific dialect, it is as it is :)

Also, do we still need pyright with ruff? If we do, maybe ruff only replaces black and isort and the motivation to switch is not that strong. -- we don't use flake8 or pylint in our project.

Yes, Ruff also has some typing capabilities or Bandit checks, and many more optional linting

Borda avatar Nov 27 '25 08:11 Borda

How about existing pyright: ignore? I think migrating them might be a huge effort. Just don't know whether it's worth it.

ultmaster avatar Nov 27 '25 10:11 ultmaster

How about existing pyright: ignore? I think migrating them might be a huge effort. Just don't know whether it's worth it.

It could be done in smaller pieces/steps and I am happy to carry it if we agree :)

Borda avatar Nov 27 '25 11:11 Borda

That's one of the decisions we need to be careful about. Let's pause a bit and I'll discuss with the team.

ultmaster avatar Nov 27 '25 13:11 ultmaster

Let me first share my thoughts on the two problems you mentioned.

Missing Linting: No automated checks for common bugs, anti-patterns, or adherence to best practices (e.g., Flake8/Pylint rules).

This is done by pyright, which is actually maintained by microsoft.

Toolchain Overhead: Managing separate dependencies and execution steps for black and isort adds unnecessary complexity to our CI/CD pipeline and local development loop.

Actually it doesn't matter since the CI is very complex. The complexity introduced by black and isort is not worth mentioning.

Migrating a new static type checking tool is a very tough in my experience. You need to carefully resolve all the warnings, while maintaining and not breaking the functionalities of any code. So unless you can convince me otherwise, I'll personally vote "no" on this matter.

Let's still wait for anyone from maintenance team willing to supervise this.

ultmaster avatar Nov 27 '25 13:11 ultmaster

Migrating a new static type checking tool is a very tough in my experience.

Not sure what you mean... if typing is correct, so just switch a tool to check it, should be a very small change... :)

This is done by pyright, which is actually maintained by microsoft.

We do not need ot replace it, we may add the Ruff rule, which could do some simple fixes if needed before the pyright check if it is correct, and in case there is collision we can simply revert the Ruff rule :)

Borda avatar Nov 30 '25 23:11 Borda

In that case, you can run the ruff and open a draft PR; and let's review the diff before deciding whether to keep it.

ultmaster avatar Dec 01 '25 05:12 ultmaster