Skip to content

Conversation

@franciscojavierarceo
Copy link
Member

@franciscojavierarceo franciscojavierarceo commented Feb 10, 2026

Summary

  • Make precommit hooks file-aware: format and lint only changed Python files (50-70% faster for typical commits)
  • Add conditional template hook that only runs when template files change
  • Update test-python-integration-local to use -n auto and --dist loadscope (20-40% faster on machines with >8 cores)
  • Add new pytest markers (slow, cloud, local_only) for better test selection during development
  • Add format-python-files and lint-python-files Makefile targets for file-aware operations

Test plan

  • Verify test collection still works: pytest --collect-only sdk/python/tests/unit
  • Verify pre-commit config is valid (commit succeeded with new hooks)
  • Run time pre-commit run --all-files to compare timing
  • Run make test-python-unit-fast to verify parallelization works

🤖 Generated with Claude Code


Open with Devin

…allelization

- Make precommit hooks file-aware: format and lint only changed Python files
- Add conditional template hook that only runs when template files change
- Update test-python-integration-local to use -n auto and --dist loadscope
- Add new pytest markers: slow, cloud, local_only for better test selection
- Add format-python-files and lint-python-files Makefile targets

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@franciscojavierarceo franciscojavierarceo requested a review from a team as a code owner February 10, 2026 16:22
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 2 potential issues.

Open in Devin Review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 423979c832

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

name: Build Templates
stages: [commit]
language: system
files: ^infra/templates/|\.jinja2$

Choose a reason for hiding this comment

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

P2 Badge Include roadmap inputs in template-hook file filter

Restricting the template hook to ^infra/templates/|\.jinja2$ means make build-templates will no longer run when docs/roadmap.md changes, even though infra/scripts/compile-templates.py reads that file to regenerate README.md. In that scenario, commits can update the roadmap without updating the generated README, leaving checked-in docs inconsistent until someone runs the build manually.

Useful? React with 👍 / 👎.

- id: lint
name: Lint
types: [python]
entry: uv run ruff check --fix

Choose a reason for hiding this comment

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

P2 Badge Run Ruff formatter in the file-aware format hook

The new format-files hook only runs ruff check --fix, which does not apply ruff format styling. Since the paired lint hook also skips ruff format --check, commits can pass local pre-commit while still failing the repository's standard lint path (make lint-python) on formatting checks. This is a regression from the previous hook behavior that ran full formatting before commit.

Useful? React with 👍 / 👎.

franciscojavierarceo and others added 3 commits February 10, 2026 12:07
- Format hook now runs both ruff check --fix AND ruff format
- Lint hook now runs both ruff check AND ruff format --check
- Template hook file filter now includes docs/roadmap.md

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

View 5 additional findings in Devin Review.

Open in Devin Review

The codebase uses xdist_group markers extensively for test isolation
(Ray OOM prevention, database registry tests, etc). The loadscope
distribution mode ignores these markers, so restore loadgroup.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

View 8 additional findings in Devin Review.

Open in Devin Review

- id: lint
name: Lint
types: [python]
entry: bash -c 'uv run ruff check --fix "$@" && uv run ruff format "$@"' --
Copy link
Contributor

Choose a reason for hiding this comment

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

🟡 && in format-files pre-commit hook skips ruff format when unfixable lint violations exist

The format-files pre-commit hook entry uses && to chain ruff check --fix and ruff format. When ruff check --fix encounters lint violations that cannot be auto-fixed, it returns exit code 1, and the && operator prevents ruff format from executing. This means files with unfixable lint issues will never be auto-formatted.

Root Cause and Impact

The entry at .pre-commit-config.yaml:13 is:

entry: bash -c 'uv run ruff check --fix "$@" && uv run ruff format "$@"' --

ruff check --fix returns exit code 1 when violations remain after applying all available auto-fixes (e.g., rules that have no auto-fix available). The && operator short-circuits, so ruff format is never invoked.

Notably, the corresponding format-python-files Makefile target at Makefile:64-65 correctly uses ; (semicolons) to separate the commands, ensuring ruff format always runs regardless of ruff check --fix's exit code:

		uv run ruff check --fix $(FILES); \
		uv run ruff format $(FILES); \

Impact: When a developer commits Python files that have unfixable lint violations (which is common — many ruff rules are not auto-fixable), the formatting step is silently skipped. The developer's files won't be auto-formatted, defeating the purpose of the format hook.

Suggested change
entry: bash -c 'uv run ruff check --fix "$@" && uv run ruff format "$@"' --
entry: bash -c 'uv run ruff check --fix "$@"; uv run ruff format "$@"' --
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

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