Repository navigation
feat(lint): enforce cyclomatic complexity in python ruff config - #1000
Merged
Merged
Conversation
Add C901 (mccabe) cyclomatic complexity rule with max-complexity = 15 to Ruff configuration, paralleling Go's gocyclo (min-complexity: 15) enforcement. Decompose parse_transcript in tools/benchmark/compare_trajectories.py and verify_review_comments_posted in tools/pr_review/review_pr_verify_posted.py into single-responsibility helper functions to satisfy the complexity limit. Update Python language rules documentation accordingly. TAG=agy CONV=22009166-566e-4d4f-ace4-c924d7d32241
There was a problem hiding this comment.
Code Review
This pull request introduces a cyclomatic complexity limit of 15 for Python code, updating the style guide and configuring Ruff's mccabe linter (C901) in pyproject.toml. To comply with this new limit, complex functions in tools/benchmark/compare_trajectories.py and tools/pr_review/review_pr_verify_posted.py are refactored into smaller, single-responsibility helper functions. Feedback on the changes suggests explicitly handling unknown actions in _is_re_review_thread_posted by returning an error instead of falling back to True, preventing potential silent verification failures.
Explicitly return an error instead of silently succeeding when an unknown re-review thread action is encountered in verify_review_comments_posted. Add unit test for unknown action handling. Addresses review comment on PR #1000. TAG=agy CONV=22009166-566e-4d4f-ace4-c924d7d32241
…f linter Linter enforcement in pyproject.toml is sufficient; omit duplicate documentation in rules. TAG=agy CONV=22009166-566e-4d4f-ace4-c924d7d32241
VLukiantsev
marked this pull request as ready for review
September 28, 2026 10:49
VLukiantsev
enabled auto-merge
September 28, 2026 10:49
janzak-dev
approved these changes
Sep 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Enforce Python cyclomatic complexity checks via Ruff (
C901/mccabe) withmax-complexity = 15, paralleling Go'sgocycloenforcement. Refactor existing violating functions intools/into focused, single-responsibility helper routines to satisfy the complexity limit.Motivation & Context
Python scripts under
tools/and throughout the repo have been growing in complexity without static lint guards to prevent sprawling conditional paths. Go already enforces both nested complexity (nestif: 4) and cyclomatic complexity (gocyclo: 15). Enabling Ruff'sC901rule withmax-complexity = 15introduces an equivalent guardrail in Python, preventing runaway complexity in scripts while keeping functions clean and maintainable.Key Changes
C901in[tool.ruff.lint.select]and configured[tool.ruff.lint.mccabe] max-complexity = 15inpyproject.toml.tools/benchmark/compare_trajectories.py: Decomposedparse_transcriptinto_parse_step_lineand_record_tool_callhelper routines.tools/pr_review/review_pr_verify_posted.py: Decomposedverify_review_comments_postedinto_is_initial_inline_posted,_is_re_review_thread_posted,_verify_all_checklist_items, and_mark_items_as_postedhelpers, with explicit error handling on unknown re-review thread actions.tools/pr_review/review_pr_verify_posted_test.pyfor unknown re-review thread action handling.Assumptions
None — requirements and threshold were fully specified and confirmed.
Verification & Testing
ruff check .passed with 0 violations across the repository.ruff format --check .passed cleanly.bazel test //tools/benchmark/... //tools/pr_review/...passed (16/16 test targets passed)../.githooks/pre-commitpassed with all checks green (Ruff, Mypy, Buildifier, Go determinism).Related Issues / Bugs
N/A
TAG=agy
CONV=22009166-566e-4d4f-ace4-c924d7d32241