Conversation
The container image selection logic was inverted - when the base image was built (should-build=true), the container used an empty string, causing failures. Now always uses the python-image output when available. Fixes #772 - Dependabot PRs (768-770) failing with base image issues Co-authored-by: openhands <openhands@all-hands.dev>
- Add root-level pyproject.toml with comprehensive Ruff config - Configure Ruff to check all Python directories (backend, ai-engine, modporter, tests) - Add appropriate ignores for legacy code patterns (unused imports, module-level imports not at top, bare except, etc.) - Update CI workflow to use root config with 'ruff check .' - Exclude UTF-16 encoded temp_init.py file from linting Co-authored-by: openhands <openhands@all-hands.dev>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Adds a root-level Ruff configuration and updates CI to run Ruff across the repository, addressing issue #678 (Python linting for backend/ai-engine and related Python code).
Changes:
- Introduces a root
pyproject.tomlRuff configuration (rules, ignores, per-file ignores, excludes). - Updates GitHub Actions CI to install Python for the Ruff job and run
ruff check .using the root config. - Adjusts CI integration-test container image selection logic and job dependencies.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
pyproject.toml |
Adds centralized Ruff configuration (ruleset, ignores, per-file ignores, excludes). |
.github/workflows/ci.yml |
Updates CI Ruff lint step and modifies integration test container image configuration and job dependencies. |
.github/workflows/ci.yml
Outdated
| # Use Python base image only when it's available and should-build is false | ||
| container: | ||
| image: ${{ needs.prepare-base-images.outputs.should-build == 'false' && needs.prepare-base-images.outputs.python-image || '' }} | ||
| image: ${{ needs.prepare-base-images.outputs.python-image }} | ||
| options: --name test-container-${{ matrix.test-suite }} --user root |
There was a problem hiding this comment.
integration-tests now always sets container.image to needs.prepare-base-images.outputs.python-image, but prepare-base-images only runs when dependencies changes. When that job is skipped, this output will be empty/undefined and the integration job will fail to start (or unintentionally skip) due to an invalid container image. Consider restoring a conditional fallback (e.g., only set the container image when the base image job ran / produced a tag), or split into two jobs (container vs runner) gated by if conditions.
| exclude = [ | ||
| "tests/", | ||
| "src/tests/", | ||
| ".git/", |
There was a problem hiding this comment.
The Ruff config excludes tests/ (and src/tests/), but the same file also defines per-file-ignores for tests/* and the PR description claims tests are covered. Since excluded paths are never linted, either remove tests from exclude (and rely on per-file-ignores), or drop the tests per-file ignore entries / update the PR description to match the actual behavior.
| "backend/src/api/conversions.py" = ["N805"] | ||
| "backend/src/utils/validators.py" = ["N805"] | ||
| "backend/src/models/types.py" = ["N811"] | ||
| "backend/src/models/addon_models.py" = ["N811"] |
There was a problem hiding this comment.
Several per-file-ignores entries reference backend paths that don't exist in this repo (e.g., backend/src/utils/validators.py, backend/src/models/types.py). These patterns will never match and make the root Ruff config harder to maintain. Suggest pruning them or updating the paths to the actual files/directories that need ignores.
| "ai-engine/src/utils/validators.py" = ["N805"] | ||
| "ai-engine/src/models/types.py" = ["N811"] | ||
| "ai-engine/src/services/*.py" = ["N806"] | ||
| "ai-engine/src/engines/*.py" = ["N806"] | ||
| "ai-engine/src/exceptions.py" = ["N818"] |
There was a problem hiding this comment.
The root Ruff config includes per-file-ignores under ai-engine/src/..., but ai-engine/src/ doesn't exist in the current tree. These entries will never apply, so either update them to the real ai-engine layout (e.g., ai-engine/utils/..., ai-engine/models/...) or remove them to avoid stale configuration.
Summary
This PR adds Ruff linter configuration to the project to address GitHub issue #678.
Changes Made
**Created root-level ** with comprehensive Ruff configuration:
Updated CI workflow ():
Why This Works
Testing
Related to: #678