Skip to content

chore(pre-commit): add deep import check hook - #401

Closed
CatherineSue wants to merge 2 commits into
mainfrom
chang/pre-commit-hooks
Closed

CatherineSue wants to merge 2 commits into
mainfrom
chang/pre-commit-hooks

Conversation

@CatherineSue

@CatherineSue CatherineSue commented Feb 10, 2026 •

Copy link
Copy Markdown
Member

Description

Problem

Deeply nested internal imports (e.g. use crate::a::b::c::d::e) hurt readability and often indicate missing re-exports or overly coupled module structure.

Solution

Adds a pre-commit hook and CI check that flags crate:: and super:: import paths with more than 4 path segments. External crate imports are ignored since those are out of our control.

Changes

  • Add scripts/check-deep-imports.sh — bash script that greps .rs files for deeply nested internal imports
  • Update .pre-commit-config.yaml — register check-deep-imports as a local pre-commit hook for Rust files
  • Update .github/workflows/pr-test-rust.yml — add lint-imports CI job that runs the same check on every PR, gated in the finish job

Test Plan

Run against the codebase to verify it catches known violations:

bash scripts/check-deep-imports.sh $(find . -name '*.rs' -not -path './target/*')

Currently flags 2 internal violations:

  • model_gateway/src/routers/grpc/common/stages/mod.rs:12
  • model_gateway/src/routers/grpc/pipeline.rs:12
Checklist
  • cargo +nightly fmt passes
  • cargo clippy --all-targets --all-features -- -D warnings passes
  • (Optional) Documentation updated

Summary by CodeRabbit

  • Chores
    • Enhanced CI/CD pipeline with automated checks to enforce import depth standards in the codebase.
    • Added pre-commit hook to catch deeply nested imports during development.
    • Updated project metadata author information.

@github-actions github-actions Bot added dependencies Dependency updates grpc gRPC client and router changes labels Feb 10, 2026
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello @CatherineSue, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a new pre-commit hook to improve code readability and maintainability by preventing deeply nested internal Rust imports. The hook identifies and flags crate:: and super:: import paths that exceed four segments, encouraging developers to use shorter paths or re-exports. This helps maintain a cleaner module structure and reduces coupling within the codebase.

Highlights

  • New Pre-commit Hook: Introduced a check-deep-imports pre-commit hook to enforce import path depth limits in Rust files.
  • Deep Import Detection Script: Added scripts/check-deep-imports.sh, a bash script that identifies crate:: and super:: imports with more than 4 path segments.
Changelog
  • .pre-commit-config.yaml
    • Registered the new check-deep-imports script as a local pre-commit hook for Rust files.
  • grpc_client/python/pyproject.toml
    • Reordered the authors list entries.
  • scripts/check-deep-imports.sh
    • Added a new bash script that uses grep to find deeply nested crate:: or super:: imports in Rust files and exits with an error if violations are found.
Activity
  • No specific activity (comments, reviews, progress updates) has been recorded for this pull request yet.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

@coderabbitai

coderabbitai Bot commented Feb 10, 2026 •

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This PR introduces a new linting check for deeply nested Rust imports across CI/CD pipelines and local development. It adds a Bash script to detect imports exceeding four path segments, integrates it into GitHub Actions and pre-commit hooks, and reorders authors in the Python gRPC client configuration.

Changes

Cohort / File(s) Summary
CI/CD Workflow Integration
.github/workflows/pr-test-rust.yml
Added lint-imports job that runs after check-ci and integrated it into the finish job's dependency chain and failure conditions to enforce import depth validation in CI.
Pre-commit Configuration
.pre-commit-config.yaml
Added new check-deep-imports hook that runs scripts/check-deep-imports.sh against Rust files to enforce import nesting depth limits locally before commits.
Import Validation Script
scripts/check-deep-imports.sh
New Bash script that scans Rust files for internal imports with more than 4 path segments (5+ total segments), reports violations, and exits with status 1 if violations found.
Project Configuration
grpc_client/python/pyproject.toml
Reordered authors list: moved Simo Lin to follow Chang Su (new order: Chang Su, Simo Lin, Keyang Ru).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Suggested labels

workflow, model-gateway

Suggested reviewers

  • key4ng
  • slin1237
  • XinyueZhang369

Poem

🐰 A hop, a check, imports run deep,
We guard the depths, no more to keep,
Four segments max, our code stays clean,
The prettiest Rust paths you've ever seen! 🌿

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding a pre-commit hook for checking deep imports, which is reflected in the new hook configuration, script, and CI workflow updates.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chang/pre-commit-hooks

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
scripts/check-deep-imports.sh (2)

6-6: MAX_SEGMENTS is not used in the regex pattern.

The variable MAX_SEGMENTS=4 is defined but the regex hardcodes {4,}. If the threshold changes, both need updating separately. Consider constructing the pattern dynamically or adding a comment noting this coupling.

♻️ Optional: Use variable in regex
 MAX_SEGMENTS=4
 violations=0
+# Build regex quantifier from MAX_SEGMENTS
+quantifier="{$MAX_SEGMENTS,}"
 
 for file in "$@"; do
     [[ -f "$file" ]] || continue
 
     # Match: use crate::a::b::c::d  (5+ segments starting with crate/super)
-    # {4,} means 4+ additional ::word segments after crate/super = 5+ total
+    # Quantifier means MAX_SEGMENTS+ additional ::word segments after crate/super
     while IFS= read -r line_info; do
         line_num="${line_info%%:*}"
         line_content="${line_info#*:}"
         echo "  $file:$line_num:$line_content"
         ((violations++))
-    done < <(grep -nE '^\s*use\s+(crate|super)(::\w+){4,}' "$file" 2>/dev/null || true)
+    done < <(grep -nE "^\s*use\s+(crate|super)(::\w+)$quantifier" "$file" 2>/dev/null || true)
 done

Also applies to: 19-19


12-19: Regex pattern does not match pub use re-exports.

The pattern ^\s*use\s+ won't match pub use crate::a::b::c::d::e or pub(crate) use .... While the codebase currently contains no such cases, extending the pattern would make the check more comprehensive if re-exports with deep nesting are added in the future.

.pre-commit-config.yaml (1)

55-61: Missing blank line before next repo section.

For consistency with other repo sections in this file, add a blank line after the check-deep-imports hook before the ruff-pre-commit repo.

♻️ Suggested fix
       - id: check-deep-imports
         name: check-deep-imports
         description: Reject internal imports with more than 4 path segments
         entry: scripts/check-deep-imports.sh
         language: script
         types: [rust]
+
   - repo: https://github.com/astral-sh/ruff-pre-commit
.github/workflows/pr-test-rust.yml (1)

53-55: Command substitution may fail with whitespace in paths.

Using $(find ...) directly in the command line can break if any file path contains spaces. While unlikely for .rs files, consider using xargs or find -exec for robustness.

♻️ Safer alternative using find -print0 and xargs
       - name: Check for deeply nested imports
         run: |
-          bash scripts/check-deep-imports.sh $(find . -name '*.rs' -not -path './target/*' -not -path './TensorRT-LLM/*')
+          find . -name '*.rs' -not -path './target/*' -not -path './TensorRT-LLM/*' -print0 | xargs -0 bash scripts/check-deep-imports.sh

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

Adds a pre-commit hook that rejects internal Rust imports (crate::, super::)
with more than 4 path segments to keep import paths shallow and readable.
@CatherineSue
CatherineSue force-pushed the chang/pre-commit-hooks branch from 91b6771 to dd67bd8 Compare February 10, 2026 21:51
@github-actions github-actions Bot added the ci CI/CD configuration changes label Feb 10, 2026

@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 introduces a new pre-commit hook to detect and flag deeply nested internal imports in Rust code, aiming to improve code readability and module structure. A potential argument injection vulnerability exists in the grep command within scripts/check-deep-imports.sh, which could lead to denial-of-service or bypass scenarios if filenames start with a hyphen. Additionally, the current detection logic could be made more robust by enhancing the regular expression to cover pub use statements and glob imports (*) for more comprehensive coverage.

Comment thread scripts/check-deep-imports.sh
@CatherineSue
CatherineSue deleted the chang/pre-commit-hooks branch February 10, 2026 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci CI/CD configuration changes dependencies Dependency updates grpc gRPC client and router changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant