Skip to content

feat: add autoflake pre-commit hook to remove unused imports#1528

Open
cristiam86 wants to merge 4 commits intomainfrom
dxp-645-add-to-pre-commit-to-remove-unused-imports
Open

feat: add autoflake pre-commit hook to remove unused imports#1528
cristiam86 wants to merge 4 commits intomainfrom
dxp-645-add-to-pre-commit-to-remove-unused-imports

Conversation

@cristiam86
Copy link
Member

@cristiam86 cristiam86 commented Mar 12, 2026

Fixes #DXP-645

What

  • Added autoflake v2.3.1 to .pre-commit-config.yaml to automatically remove unused imports during commits
  • Hook runs before black formatter to ensure proper formatting
  • Cleaned up 100+ unused imports across 75 Python files

Why

Unused imports create code smell and increase maintenance burden. Automating their removal keeps the codebase clean without manual effort.

Testing done

  • All DB/SQLAlchemy tests pass (60/60)
  • All modified Python files pass syntax checks (75/75)
  • Pre-commit hooks execute successfully

Decisions made

  • Used autoflake for focused import removal (simpler than ruff for this specific task)
  • Placed autoflake before black so formatting is applied after import cleanup
  • Applied on existing codebase to remove accumulated unused imports

Checks

  • I have tested this code
  • I have reviewed my own PR
  • I have created an issue for this PR (DXP-645)
  • I have set a descriptive PR title compliant with conventional commits

Summary by CodeRabbit

  • Chores
    • Added an autoflake pre-commit hook to automatically remove unused imports and keep code tidy.
    • Performed widespread import and placeholder cleanup across the codebase to reduce clutter and improve maintainability.
    • Minor test and internal housekeeping adjustments to align with the cleaned codebase.

- Added autoflake v2.3.1 to .pre-commit-config.yaml to automatically remove
  unused imports on commit, running before black for proper formatting
- Autoflake cleaned up 100+ unused imports across 75 Python files
- All DB/SQLAlchemy tests pass (60/60)
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 12, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8b31da44-309b-4d10-b5cf-d09ccb26eeae

📥 Commits

Reviewing files that changed from the base of the PR and between 4e61c4e and 4457aa9.

📒 Files selected for processing (2)
  • backend/consensus/base.py
  • backend/protocol_rpc/app_lifespan.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • backend/protocol_rpc/app_lifespan.py

📝 Walkthrough

Walkthrough

Adds an autoflake pre-commit hook and removes numerous unused imports and a few trivial pass statements across many modules and tests; no substantive control-flow, API, or migration logic changes are introduced.

Changes

Cohort / File(s) Summary
Pre-commit Configuration
/.pre-commit-config.yaml
Adds autoflake v2.3.1 hook to remove unused imports and perform in-place edits.
Consensus & Worker
backend/consensus/base.py, backend/consensus/monitoring.py, backend/consensus/worker.py, backend/consensus/worker_service.py
Removed unused imports and a few trivial pass statements; no control-flow changes.
Database Handler & Migrations
backend/database_handler/..., backend/database_handler/migration/versions/*
Removed multiple unused imports (ORM/session/dialects/model imports) across chain_snapshot, contract_processor, transactions_processor and migration scripts.
Protocol RPC
backend/protocol_rpc/app_lifespan.py, backend/protocol_rpc/broadcast.py, backend/protocol_rpc/calls_intercept/__init__.py, backend/protocol_rpc/contract_linter.py, backend/protocol_rpc/endpoints.py, backend/protocol_rpc/explorer/queries.py, backend/protocol_rpc/fastapi_*, backend/protocol_rpc/message_handler/*, backend/protocol_rpc/rpc_endpoint_manager.py, backend/protocol_rpc/transactions_parser.py, backend/protocol_rpc/validators_init.py
Removed many unused imports; notable risky edits: removed method bodies (pass) from abstract methods in calls_intercept/__init__.py (may cause syntax errors) and removed Severity import in contract_linter.py while severity usage remains.
Node, GenVM & Domain
backend/node/..., backend/domain/types.py
Pruned unused imports (datetime, logging, Path, decimal, asdict, etc.) and reduced top-level import surface in genvm/origin modules.
Rollup & Validators
backend/rollup/consensus_service.py, backend/validators/__init__.py
Removed unused imports (os, Web3, logging, Path).
Scripts & Config
scripts/update_error_transactions_metrics.py, uvicorn_config.py
Minor import cleanup (removed unused datetime.timedelta, multiprocessing).
Tests — many modules
tests/... (unit, integration, load, db-sqlalchemy, direct)
Removed numerous unused imports across tests; one test file dropped ~10 effect classes from its surface (tests/unit/consensus/test_effects.py), and several tests had narrowed/removed imports.
Miscellaneous helpers & archived scripts
tests/common/*, tests/load/script_archive_unused/*
Removed unused json/os/datetime/Web3/eth_utils imports in helper and archived scripts.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

run-tests

Suggested reviewers

  • dohernandez
  • cgmello
  • MuncleUscles

Poem

🐰
I hopped through imports, neat and spry,
Autoflake trimmed what piled too high.
Passes vanished, json waved goodbye,
Tests still hum beneath the sky —
A rabbit tidies, then hops on by.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the primary change: adding autoflake pre-commit hook to remove unused imports, which is the main focus of this PR.
Description check ✅ Passed The description is comprehensive and follows the template structure with clear sections for What, Why, Testing done, Decisions made, and all required checks completed.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch dxp-645-add-to-pre-commit-to-remove-unused-imports
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
tests/db-sqlalchemy/transactions_processor_test.py (1)

202-206: Remove redundant pass statement.

The pass at line 203 is a no-op leftover from replacing the unused import time statement. Since the function body contains actual code, this pass serves no purpose and can be removed.

🧹 Proposed fix
 def test_get_highest_timestamp(transactions_processor: TransactionsProcessor):
-    pass
-
     # Initially should return 0 when no transactions exist
     assert transactions_processor.get_highest_timestamp() == 0
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/db-sqlalchemy/transactions_processor_test.py` around lines 202 - 206,
Remove the redundant no-op "pass" from the test function
test_get_highest_timestamp so the function body only contains the actual
assertion; locate the test_get_highest_timestamp definition in the tests and
delete the solitary pass line while leaving the assertion that calls
TransactionsProcessor.get_highest_timestamp() intact.
backend/node/genvm/base.py (1)

198-198: Remove orphaned pass statement.

The pass statement is a no-op that appears to be leftover from removing the logging setup. It serves no purpose since the code continues immediately to line 200. Consider removing it for cleaner code.

🧹 Suggested cleanup
         elif res.result_kind == ResultCode.INTERNAL_ERROR:
-            pass
-
             error_ctx = None
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/node/genvm/base.py` at line 198, Remove the orphaned no-op "pass"
statement that was left behind in the function/method body (the stray pass at
the top of the block) since it serves no purpose and the subsequent code
executes immediately; simply delete that pass, verify indentation and
surrounding logic remain correct (no placeholder needed), and run tests/lint to
ensure no behavioral or formatting regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@backend/node/genvm/base.py`:
- Line 198: Remove the orphaned no-op "pass" statement that was left behind in
the function/method body (the stray pass at the top of the block) since it
serves no purpose and the subsequent code executes immediately; simply delete
that pass, verify indentation and surrounding logic remain correct (no
placeholder needed), and run tests/lint to ensure no behavioral or formatting
regressions.

In `@tests/db-sqlalchemy/transactions_processor_test.py`:
- Around line 202-206: Remove the redundant no-op "pass" from the test function
test_get_highest_timestamp so the function body only contains the actual
assertion; locate the test_get_highest_timestamp definition in the tests and
delete the solitary pass line while leaving the assertion that calls
TransactionsProcessor.get_highest_timestamp() intact.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f75e4773-fa06-4360-afe1-819d82bf934e

📥 Commits

Reviewing files that changed from the base of the PR and between 6dad06d and a65525d.

📒 Files selected for processing (76)
  • .pre-commit-config.yaml
  • asgi.py
  • backend/consensus/base.py
  • backend/consensus/monitoring.py
  • backend/consensus/worker.py
  • backend/consensus/worker_service.py
  • backend/database_handler/chain_snapshot.py
  • backend/database_handler/contract_processor.py
  • backend/database_handler/migration/versions/3bc34e44eb72_add_providers_unique_constraint.py
  • backend/database_handler/migration/versions/67943badcbe9_add_activated_status.py
  • backend/database_handler/migration/versions/6fd3e2cea05b_add_default_to_llm_providers.py
  • backend/database_handler/migration/versions/99015c5b5b78_add_snapshot_table.py
  • backend/database_handler/migration/versions/ab256b41602a_remove_invalid_models.py
  • backend/database_handler/migration/versions/db38e78684a8_add_providers_table.py
  • backend/database_handler/transactions_processor.py
  • backend/domain/types.py
  • backend/node/base.py
  • backend/node/genvm/base.py
  • backend/node/genvm/origin/base_host.py
  • backend/node/genvm/origin/host_fns.py
  • backend/protocol_rpc/app_lifespan.py
  • backend/protocol_rpc/broadcast.py
  • backend/protocol_rpc/calls_intercept/__init__.py
  • backend/protocol_rpc/contract_linter.py
  • backend/protocol_rpc/endpoints.py
  • backend/protocol_rpc/explorer/queries.py
  • backend/protocol_rpc/fastapi_endpoint_generator.py
  • backend/protocol_rpc/fastapi_rpc_handler.py
  • backend/protocol_rpc/fastapi_server.py
  • backend/protocol_rpc/message_handler/fastapi_handler.py
  • backend/protocol_rpc/message_handler/worker_handler.py
  • backend/protocol_rpc/rpc_endpoint_manager.py
  • backend/protocol_rpc/transactions_parser.py
  • backend/protocol_rpc/validators_init.py
  • backend/rollup/consensus_service.py
  • backend/validators/__init__.py
  • scripts/update_error_transactions_metrics.py
  • tests/common/request.py
  • tests/common/transactions.py
  • tests/db-sqlalchemy/accounts_manager_test.py
  • tests/db-sqlalchemy/snapshot_manager_test.py
  • tests/db-sqlalchemy/transactions_processor_test.py
  • tests/direct/test_genvm_smoke_direct.py
  • tests/integration/icontracts/contracts/intelligent_oracle.py
  • tests/integration/icontracts/tests/test_multi_file_contract.py
  • tests/load/deploy_contract/test_wizard_of_coin.py
  • tests/load/deploy_contract/wizard_deploy.py
  • tests/load/deploy_contract/wizard_deploy_and_read.py
  • tests/load/script_archive_unused/analyze_transactions.py
  • tests/load/script_archive_unused/deploy_contract_example.py
  • tests/load/script_archive_unused/deploy_wizard_of_coin.py
  • tests/load/script_archive_unused/generate_correct_deployment_tx.py
  • tests/load/script_archive_unused/generate_deployment_tx_ui_format.py
  • tests/load/script_archive_unused/generate_raw_transaction.py
  • tests/load/script_archive_unused/generate_raw_transaction_fixed.py
  • tests/load/script_archive_unused/generate_ui_compatible_tx.py
  • tests/test_linter_endpoint.py
  • tests/unit/consensus/test_decisions_accepted.py
  • tests/unit/consensus/test_decisions_caller_helpers.py
  • tests/unit/consensus/test_decisions_pending_proposing_committing.py
  • tests/unit/consensus/test_decisions_terminal.py
  • tests/unit/consensus/test_effect_executor.py
  • tests/unit/consensus/test_effects.py
  • tests/unit/consensus/test_helpers.py
  • tests/unit/test_contract_not_found_handling.py
  • tests/unit/test_create_nodes.py
  • tests/unit/test_error_codes.py
  • tests/unit/test_execution_mode.py
  • tests/unit/test_fallback_validator_model_host_data.py
  • tests/unit/test_genvm_retry.py
  • tests/unit/test_genvm_retry_integration.py
  • tests/unit/test_node_execution_time.py
  • tests/unit/test_rate_limiter.py
  • tests/unit/test_types.py
  • tests/unit/test_worker_health_degradation.py
  • uvicorn_config.py
💤 Files with no reviewable changes (57)
  • tests/unit/consensus/test_decisions_terminal.py
  • backend/protocol_rpc/message_handler/worker_handler.py
  • tests/unit/test_create_nodes.py
  • tests/db-sqlalchemy/accounts_manager_test.py
  • backend/node/base.py
  • backend/database_handler/transactions_processor.py
  • tests/unit/test_types.py
  • backend/consensus/worker.py
  • tests/load/deploy_contract/test_wizard_of_coin.py
  • backend/node/genvm/origin/base_host.py
  • tests/integration/icontracts/tests/test_multi_file_contract.py
  • tests/load/script_archive_unused/deploy_wizard_of_coin.py
  • backend/protocol_rpc/message_handler/fastapi_handler.py
  • backend/validators/init.py
  • tests/load/script_archive_unused/deploy_contract_example.py
  • tests/test_linter_endpoint.py
  • tests/load/script_archive_unused/generate_deployment_tx_ui_format.py
  • backend/protocol_rpc/broadcast.py
  • tests/unit/test_fallback_validator_model_host_data.py
  • tests/load/script_archive_unused/generate_ui_compatible_tx.py
  • tests/common/transactions.py
  • tests/load/script_archive_unused/generate_raw_transaction_fixed.py
  • backend/database_handler/contract_processor.py
  • tests/unit/test_node_execution_time.py
  • backend/protocol_rpc/endpoints.py
  • backend/protocol_rpc/fastapi_server.py
  • tests/unit/consensus/test_decisions_pending_proposing_committing.py
  • backend/database_handler/migration/versions/3bc34e44eb72_add_providers_unique_constraint.py
  • tests/unit/consensus/test_effects.py
  • tests/unit/test_error_codes.py
  • tests/unit/consensus/test_helpers.py
  • tests/load/script_archive_unused/generate_correct_deployment_tx.py
  • tests/unit/consensus/test_decisions_accepted.py
  • tests/unit/test_worker_health_degradation.py
  • tests/load/script_archive_unused/analyze_transactions.py
  • backend/database_handler/migration/versions/ab256b41602a_remove_invalid_models.py
  • backend/database_handler/migration/versions/6fd3e2cea05b_add_default_to_llm_providers.py
  • tests/unit/consensus/test_decisions_caller_helpers.py
  • backend/domain/types.py
  • backend/protocol_rpc/fastapi_rpc_handler.py
  • tests/db-sqlalchemy/snapshot_manager_test.py
  • backend/rollup/consensus_service.py
  • backend/database_handler/migration/versions/67943badcbe9_add_activated_status.py
  • tests/direct/test_genvm_smoke_direct.py
  • backend/consensus/base.py
  • backend/protocol_rpc/app_lifespan.py
  • tests/unit/test_genvm_retry.py
  • tests/load/deploy_contract/wizard_deploy_and_read.py
  • tests/load/script_archive_unused/generate_raw_transaction.py
  • backend/protocol_rpc/rpc_endpoint_manager.py
  • backend/database_handler/migration/versions/99015c5b5b78_add_snapshot_table.py
  • backend/database_handler/migration/versions/db38e78684a8_add_providers_table.py
  • tests/load/deploy_contract/wizard_deploy.py
  • backend/consensus/worker_service.py
  • uvicorn_config.py
  • asgi.py
  • backend/protocol_rpc/calls_intercept/init.py

- Restored `from backend.protocol_rpc import rpc_methods` in app_lifespan.py
  with noqa comment — this is a side-effect import that registers all RPC
  methods via decorators
- Removed remaining unused imports from backend/consensus/base.py that were
  missed in the initial autoflake run
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