Skip to content

Add ruff + pre-commit (format + E,F,I) — fixes #28#27

Merged
bordeauxred merged 1 commit into
aganthos:mainfrom
kiranannadatha8:18-ruff-pre-commit
Apr 19, 2026
Merged

Add ruff + pre-commit (format + E,F,I) — fixes #28#27
bordeauxred merged 1 commit into
aganthos:mainfrom
kiranannadatha8:18-ruff-pre-commit

Conversation

@kiranannadatha8
Copy link
Copy Markdown
Contributor

@kiranannadatha8 kiranannadatha8 commented Apr 18, 2026

Summary

  • Adds [tool.ruff] config to pyproject.toml (line-length 99, py311, select E/F/I, excludes benchmarks + skyrl submodule)
  • Pins ruff>=0.4,<0.5 and pre-commit>=3.6 in the dev extra
  • New .pre-commit-config.yaml with ruff + ruff-format hooks pinned to v0.4.4
  • Rewrites ## Style section in CONTRIBUTING.md with pre-commit install instructions
  • Separate commit applies one-shot ruff format + --fix so the config change stays reviewable in isolation

Closes #28.

Test plan

  • ruff check --select E,F,I clawloop tests examples exits 0
  • ruff format --check clawloop tests examples exits 0
  • Reviewer spot-checks commit 2 diff (genuinely-unused symbols removed: EvalResult, field, Future, OptimResult)
  • pip install -e ".[dev]" && pre-commit install — hooks run on commit

Out of scope (deliberately, per issue)

  • UP / B / SIM rules — semantic rewrites, deferred
  • CI lint job — can be added in a follow-up
  • Broader repository-wide pre-commit rollout — deferred

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request standardizes the codebase by adopting ruff for linting and formatting, adding pre-commit hooks, and updating contribution guidelines. The changes consist of extensive formatting adjustments across all modules and tests to ensure consistency. Feedback focuses on improving code quality by removing an empty TYPE_CHECKING block and several unused variables that were unnecessarily preserved with linter suppression tags.

Comment thread clawloop/environments/openclaw.py
Comment thread tests/test_curator_lightweight.py Outdated
Comment thread tests/test_otel_exporter.py Outdated
Comment thread tests/test_local_evolver.py Outdated
Comment thread tests/test_harbor_env.py Outdated
Comment thread examples/openclaw_demo_remote.py Outdated
kiranannadatha8 added a commit to kiranannadatha8/clawloop that referenced this pull request Apr 18, 2026
…mpty TYPE_CHECKING

Per gemini-code-assist review on aganthos#27:
- clawloop/environments/openclaw.py: drop empty `if TYPE_CHECKING: pass`
  block and unused TYPE_CHECKING import
- tests/test_curator_lightweight.py: invoke `curator.consolidate(pb)`
  directly; drop unused `report =` binding
- tests/test_otel_exporter.py: delete `original_span_kind` /
  `original_kind_agent` assignments (never read)
- tests/test_local_evolver.py: remove unused `playbook = Playbook()` and
  its now-dead import
- tests/test_harbor_env.py: call `_make_env()` bare; drop unused `env`
- examples/openclaw_demo_remote.py: drop unused `episodes: list[Episode]
  = []` (captured list is built a few lines below)

Net: 12 lines deleted, no F841 noqa suppressions remain.
@bordeauxred bordeauxred changed the title Add ruff + pre-commit (format + E,F,I) — fixes #18 Add ruff + pre-commit (format + E,F,I) — fixes #28 Apr 18, 2026
@bordeauxred
Copy link
Copy Markdown
Contributor

Thanks for picking this up. I refreshed the tracking issue as #28 and updated the PR metadata so this closes the live issue. The commits and contribution stay yours.

@kiranannadatha8
Copy link
Copy Markdown
Contributor Author

Thanks for picking this up. I refreshed the tracking issue as #28 and updated the PR metadata so this closes the live issue. The commits and contribution stay yours.

Thanks for the swap to #28 — noted. Happy to follow up if anything else surfaces in review.

@bordeauxred
Copy link
Copy Markdown
Contributor

Hi @kiranannadatha8,
thanks a lot for your contribution. Please adress the issues raised by gemini.

@tianyibigdata pleae also take a look

@tianyibigdata
Copy link
Copy Markdown
Collaborator

Thanks for picking this up. I refreshed the tracking issue as #28 and updated the PR metadata so this closes the live issue. The commits and contribution stay yours.

Thanks for the swap to #28 — noted. Happy to follow up if anything else surfaces in review.

Hey @kiranannadatha8, thank you for your contribution. Could you plz address the following issues:

  1. Issues raised by Gemini Reivew
  2. This integration into Github Workflow
  3. Squash merge all commits into one

kiranannadatha8 added a commit to kiranannadatha8/clawloop that referenced this pull request Apr 19, 2026
Adds a parallel `lint` job that runs:
- ruff check --select E,F,I clawloop tests examples
- ruff format --check clawloop tests examples

Version is pinned via the `[dev]` extra (ruff>=0.4,<0.5), which
matches the pre-commit v0.4.4 hook pin so local and CI agree.

Closes the "CI lint job" follow-up called out as out-of-scope in
the PR body; addresses reviewer request on aganthos#27.
@kiranannadatha8
Copy link
Copy Markdown
Contributor Author

Thanks for the review @bordeauxred @tianyibigdata. Status on the three items:

1. Gemini review comments — resolved in f080002:

  • clawloop/environments/openclaw.py — dropped empty TYPE_CHECKING block + its import
  • tests/test_curator_lightweight.py — removed unused report = binding; calling curator.consolidate(pb) directly
  • tests/test_otel_exporter.py — removed unused original_span_kind / original_kind_agent assignments
  • tests/test_local_evolver.py — removed unused playbook + dead import
  • tests/test_harbor_env.py — removed unused env
  • examples/openclaw_demo_remote.py — removed unused episodes: list[Episode] = []

No F841 noqa suppressions remain.

2. GitHub Workflow integration — added in 60f7e26. New parallel lint job in .github/workflows/ci.yml runs:

  • ruff check --select E,F,I clawloop tests examples
  • ruff format --check clawloop tests examples

Version lock chain: pre-commit pins v0.4.4pyproject.toml pins ruff>=0.4,<0.5 in [dev] → CI installs via pip install -e ".[dev]". Verified locally.

3. Squash merge — happy to either (a) leave the 4 commits and let you use GitHub's "Squash and merge" button, or (b) force-push a single squashed commit if you'd prefer the branch history clean before merge. LMK which you'd like.

@tianyibigdata
Copy link
Copy Markdown
Collaborator

tianyibigdata commented Apr 19, 2026

Thanks for the review @bordeauxred @tianyibigdata. Status on the three items:

1. Gemini review comments — resolved in f080002:

  • clawloop/environments/openclaw.py — dropped empty TYPE_CHECKING block + its import
  • tests/test_curator_lightweight.py — removed unused report = binding; calling curator.consolidate(pb) directly
  • tests/test_otel_exporter.py — removed unused original_span_kind / original_kind_agent assignments
  • tests/test_local_evolver.py — removed unused playbook + dead import
  • tests/test_harbor_env.py — removed unused env
  • examples/openclaw_demo_remote.py — removed unused episodes: list[Episode] = []

No F841 noqa suppressions remain.

2. GitHub Workflow integration — added in 60f7e26. New parallel lint job in .github/workflows/ci.yml runs:

  • ruff check --select E,F,I clawloop tests examples
  • ruff format --check clawloop tests examples

Version lock chain: pre-commit pins v0.4.4pyproject.toml pins ruff>=0.4,<0.5 in [dev] → CI installs via pip install -e ".[dev]". Verified locally.

3. Squash merge — happy to either (a) leave the 4 commits and let you use GitHub's "Squash and merge" button, or (b) force-push a single squashed commit if you'd prefer the branch history clean before merge. LMK which you'd like.

thx, saw the gemini fix. one last thing, could you plz add the ruff and its flags to the precommit and call the following in the CI?

      - name: Run pre-commit hooks
        run: pre-commit run --all-files

plz force push the squahsed commit.

@kiranannadatha8
Copy link
Copy Markdown
Contributor Author

kiranannadatha8 commented Apr 19, 2026

Done, @tianyibigdata:

Pre-commit is now the source of truth. .pre-commit-config.yaml carries the flags:

- id: ruff
  args: [--fix, --select, "E,F,I"]
- id: ruff-format

CI lint job calls pre-commit directly:

- run: pip install -e ".[dev]"
- name: Run pre-commit hooks
  run: pre-commit run --all-files

No more duplicate flag config between ci.yml and the hook. Verified locally: pre-commit run --all-filesruff Passed, ruff-format Passed.

Squashed — the 4 PR commits + the two follow-up fixes collapsed into a single commit 87005b8. Force-pushed with lease; PR branch is now one commit on top of main.

@kiranannadatha8
Copy link
Copy Markdown
Contributor Author

Rebased onto latest main and resolved conflicts. Force-pushed as 3871ff9.

What moved in upstream main since fork:

How I resolved:

  1. pyproject.toml — kept both sides: dev extra now includes starlette/uvicorn/httpx (upstream) + ruff/pre-commit (mine); kept [tool.uv.sources] + [tool.ruff] side-by-side. Bumped target-version py311 → py312 to match the new >=3.12 baseline.
  2. clawloop/core/loop.py, environments/__init__.py, train.py, examples/demo_math.py — took upstream content (newer refactors), re-applied ruff format + ruff check --fix --select E,F,I on top.
  3. Added scripts/ to [tool.ruff].extend-exclude — matches the PR's scope (clawloop tests examples) and keeps pre-commit run --all-files green.

Lint violations surfaced by the new config (12 total, all fixed minimally):

  • clawloop/core/loop.py: E501 (long comment moved above the field), E741 (renamed llayer in a comprehension)
  • clawloop/environments/openspiel.py: 2× E501 (split string literals)
  • tests/unit/environments/test_openspiel.py: E501 (wrapped comment)
  • tests/unit/weight_backends/test_tinker_{backend,exporter,sdk}.py: 7× E402 — added # noqa: E402 to imports that follow pytest.importorskip() (idiomatic; they literally can't be at top of file)

Verified locally:

  • uvx ruff@0.4.4 check --select E,F,I clawloop tests examplesAll checks passed!
  • uvx ruff@0.4.4 format --check clawloop tests examples197 files already formatted
  • pre-commit run --all-filesruff Passed, ruff-format Passed

PR now shows 1 commit, MERGEABLE. Ready for re-review.

Comment thread .github/workflows/ci.yml Outdated
submodules: false
- uses: actions/setup-python@v5
with:
python-version: "3.11"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@kiranannadatha8 could you plz upgrade it to 3.12 cuz we switched the python to 312 and 313? We ran the CI but it failed for 3.11.

- pyproject: ruff config (line 99, py312, E/F/I), excludes for
  benchmarks/skyrl/scripts, per-file ignores for tests/examples
- .pre-commit-config.yaml: ruff (--fix --select E,F,I) + ruff-format
  pinned to v0.4.4 — single source of truth for hook flags
- dev extra: ruff>=0.4,<0.5, pre-commit>=3.6
- CI: new lint job runs `uv run pre-commit run --all-files` on py3.12
- CONTRIBUTING.md: pre-commit install + manual run instructions
- one-shot ruff format + E,F,I fixes across clawloop/, tests/, examples/
@kiranannadatha8
Copy link
Copy Markdown
Contributor Author

@tianyibigdata bumped the lint job to Python 3.12 (was 3.11 — that was the CI fail). Also rebased onto latest main and re-ran the format + E,F,I fixes; pre-commit now passes clean.

Verified locally:

  • uv run pre-commit run --all-files → both hooks pass
  • uv run ruff check --select E,F,I clawloop tests examples → All checks passed
  • uv run ruff format --check clawloop tests examples → 197 files already formatted

Single squashed commit, force-pushed with lease.

Copy link
Copy Markdown
Collaborator

@tianyibigdata tianyibigdata left a comment

Choose a reason for hiding this comment

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

LGTM!

@bordeauxred bordeauxred merged commit 3216c45 into aganthos:main Apr 19, 2026
5 checks passed
bordeauxred pushed a commit that referenced this pull request May 22, 2026
- pyproject: ruff config (line 99, py312, E/F/I), excludes for
  benchmarks/skyrl/scripts, per-file ignores for tests/examples
- .pre-commit-config.yaml: ruff (--fix --select E,F,I) + ruff-format
  pinned to v0.4.4 — single source of truth for hook flags
- dev extra: ruff>=0.4,<0.5, pre-commit>=3.6
- CI: new lint job runs `uv run pre-commit run --all-files` on py3.12
- CONTRIBUTING.md: pre-commit install + manual run instructions
- one-shot ruff format + E,F,I fixes across clawloop/, tests/, examples/
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.

Add ruff + pre-commit (format + E,F,I)

3 participants