Skip to content

[Chore] satisfy pre-commit on rollout.py + vllm_engine.py (pre-existing lint debt) - #140

Merged
CalvinXKY merged 1 commit into
mainfrom
sync/lint-cleanup
Jun 3, 2026
Merged

[Chore] satisfy pre-commit on rollout.py + vllm_engine.py (pre-existing lint debt)#140
CalvinXKY merged 1 commit into
mainfrom
sync/lint-cleanup

Conversation

@aoshen02

@aoshen02 aoshen02 commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

What

Mechanical pre-commit cleanup of two files that carry pre-existing lint debt on main. Decoupled into its own PR (per maintainer request) so the functional sync PRs (#137 PR-F, #138 PR-C) stay free of unrelated reformatting noise.

Applies exactly what the repo's own pre-commit hooks (ruff --fix, autoflake, isort --profile=black, black -l119) produce — no hand edits:

  • vime/ray/rollout.py
    • drop unused from argparse import Namespace (ruff F401 / autoflake; zero remaining refs)
    • isort group separation (blank line after the ray.util third-party import)
    • reorder two function-local lazy imports in _start_router so third-party (vllm_router.router_args.RouterArgs) precedes first-party (vime.utils.http_utils.run_router) — behavior-neutral
  • vime/backends/vllm_utils/vllm_engine.py
    • +2 blank lines before class VLLMEngine (black E302)

Why

main is already red on pre-commit run --all-files in these two files. Any PR touching them inherits the red gate. Fixing it here keeps the gate green for #137/#138 without bundling formatting churn into review.

Verification

  • All 4 pre-commit hooks pass after the change.
  • python -m py_compile clean on both files.
  • +6/−3, no functional change.

🤖 Generated with Claude Code

…ng debt)

These are the only two files in the repo that fail the pre-commit gate
(ruff/autoflake/isort/black) on origin/main — confirmed repo-wide:
  - ruff:     only vime/ray/rollout.py:8 (F401 unused `argparse.Namespace`)
  - autoflake:only vime/ray/rollout.py
  - black:    vime/ray/rollout.py + vime/backends/vllm_utils/vllm_engine.py

Apply the exact hook auto-fixes (ruff --fix, autoflake --remove-all-unused-imports,
isort --profile=black, black -l119), no functional change:
  - rollout.py: drop unused `from argparse import Namespace` (zero refs); isort
    blank-line separation between third-party and first-party; order two
    deferred function-local imports (vllm_router before vime.*).
  - vllm_engine.py: two blank lines before `class VLLMEngine` (E302/black).

Decoupled from the slime-sync PRs (#137/#138) on purpose: those two PRs each
touch one of these files, and the auto-fixing gate would reformat them on any
PR that does. Landing the debt once here keeps the sync PRs scoped to behavior.

Verified in vime-vllm cpu image: all four hooks pass on both files afterward;
py_compile clean; `Namespace` has no remaining references.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: aoshen02 <aoshen@inferact.ai>

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

Copy link
Copy Markdown
Contributor

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 performs minor code cleanup and formatting adjustments. It removes an unused Namespace import from argparse in vime/ray/rollout.py, reorders local imports within the _start_router function, and adds blank lines for improved code readability in both vime/ray/rollout.py and vime/backends/vllm_utils/vllm_engine.py. There are no review comments, and no further feedback is required.

@CalvinXKY
CalvinXKY merged commit 2719a8f into main Jun 3, 2026
1 of 2 checks passed
@aoshen02
aoshen02 deleted the sync/lint-cleanup branch June 8, 2026 14:17
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.

2 participants