Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR centralizes CI linting into a single pre-commit step, adds a pre-commit config and dev dependency, updates CONTRIBUTING.md with pre-commit instructions, and applies numerous whitespace/EOF formatting fixes across docs and scripts. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
podkidyshev
requested review from
amaslenn,
jeffnvidia and
srivatsankrishnan
as code owners
Contributor
Greptile SummaryThis PR introduces a Key changes:
Issues found:
Confidence Score: 3/5
Important Files Changed
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.pre-commit-config.yaml:
- Around line 34-56: Hooks pyright, vulture, lint-imports, and taplo-fmt are
configured with pass_filenames: false and no files/types filters so they always
run against the whole repo or hardcoded paths; change their configs to scope to
relevant changed files by either setting pass_filenames: true (so the hook
receives the changed filenames) or adding precise files/types globs for each
hook (e.g. Python src/** tests/** for pyright/vulture/lint-imports and toml/glob
for taplo-fmt) and remove hardcoded repo-wide paths; update the entries for the
id values pyright, vulture, lint-imports, and taplo-fmt accordingly so the hooks
only run on matching file changes.
In `@CONTRIBUTING.md`:
- Around line 27-33: Update the CONTRIBUTING.md text that currently enumerates
specific linters to instead point contributors to use the pre-commit
configuration as the canonical source: replace the sentence that lists `ruff
check`, `pyright`, etc. with wording that says "Ensure your code passes the
pre-commit hooks defined in .pre-commit-config.yaml" (and keep the existing
pre-commit install/run commands). Reference .pre-commit-config.yaml as the
source of truth and remove the hard-coded tool list so future hook changes in
ci.yml or .pre-commit-config.yaml stay in sync.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 47e9e874-eeb8-45dc-bbd9-e14af3468341
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
.github/workflows/ci.yml.github/workflows/docs.yml.gitignore.pre-commit-config.yamlCONTRIBUTING.mdconf/common/test_scenario/ucc_generator_file.txtdoc/DEV.rstdoc/index.rstdoc/reporting.rstdoc/workloads/aiconfigurator.rstdoc/workloads/ddlb.rstdoc/workloads/deepep.rstdoc/workloads/megatron_bridge.rstdoc/workloads/nemo_launcher.rstgreptile.jsonpyproject.tomlsrc/cloudai/systems/slurm/slurm-metadata.shsrc/cloudai/util/base-report.jinja2src/cloudai/util/nixl_report_template.jinja2src/cloudai/workloads/ai_dynamo/ai_dynamo.sh
💤 Files with no reviewable changes (6)
- doc/workloads/aiconfigurator.rst
- doc/workloads/ddlb.rst
- doc/index.rst
- doc/reporting.rst
- doc/DEV.rst
- doc/workloads/deepep.rst
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.pre-commit-config.yaml (1)
52-55: 🧹 Nitpick | 🔵 TrivialAdd
filesfilter and ensure trailing newline.The
taplo-fmthook should include afilesfilter to only trigger on TOML file changes, avoiding unnecessary runs on unrelated commits. Also, the file appears to be missing a trailing newline.♻️ Proposed fix
- id: taplo-fmt name: taplo entry: taplo fmt --check --diff language: system + files: \.toml$ +🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.pre-commit-config.yaml around lines 52 - 55, Update the taplo-fmt hook block (id: taplo-fmt, name: taplo, entry: taplo fmt --check --diff) to include a files filter that restricts the hook to TOML files (e.g., add files: '\\.toml$'), and ensure the .pre-commit-config.yaml file ends with a trailing newline so the file terminates with a newline character.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.pre-commit-config.yaml:
- Line 33: The YAML linter is failing due to extra spaces inside the
square-bracket list for the pre-commit hook args; locate the args entry shown as
args: [ --fix ] and remove the inner spacing so the list is compact (args:
[--fix]) to conform to YAMLlint/bracket-spacing rules.
In `@CONTRIBUTING.md`:
- Around line 28-34: The fenced bash code block containing the three commands
starting with "uv sync --extra dev" needs a blank line inserted immediately
before the opening ```bash and another blank line immediately after the closing
``` so the block is surrounded by empty lines per MD031; update the section in
CONTRIBUTING.md where that block appears (the block with "uv sync --extra dev",
"uv run pre-commit install", "uv run pre-commit run --all-files") to include one
blank line above the opening fence and one blank line below the closing fence.
---
Duplicate comments:
In @.pre-commit-config.yaml:
- Around line 52-55: Update the taplo-fmt hook block (id: taplo-fmt, name:
taplo, entry: taplo fmt --check --diff) to include a files filter that restricts
the hook to TOML files (e.g., add files: '\\.toml$'), and ensure the
.pre-commit-config.yaml file ends with a trailing newline so the file terminates
with a newline character.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c046f3bb-6f17-4e26-9cf2-b81eca1853cb
📒 Files selected for processing (3)
.pre-commit-config.yamlCONTRIBUTING.mdpyproject.toml
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
CONTRIBUTING.md (1)
28-33:⚠️ Potential issue | 🟡 MinorAdd a blank line before the fenced code block.
The bash block still violates MD031 because the opening fence immediately follows the list item text. Insert an empty line before
```bashso the block is surrounded by blank lines consistently.📝 Proposed fix
- Ensure your changes pass the pre-commit checks defined in `.pre-commit-config.yaml`. - Install and run pre-commit hooks locally: + ```bash uv sync --extra dev uv run pre-commit install uv run pre-commit run --all-files ```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CONTRIBUTING.md` around lines 28 - 33, The fenced bash code block after the list item in CONTRIBUTING.md (the block starting with ```bash) violates MD031 because the opening fence immediately follows the list text; fix this by inserting a blank line between the list item text and the opening ```bash fence (and ensure there's a blank line after the closing fence as well so the code block is surrounded by blank lines).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@CONTRIBUTING.md`:
- Around line 28-33: The fenced bash code block after the list item in
CONTRIBUTING.md (the block starting with ```bash) violates MD031 because the
opening fence immediately follows the list text; fix this by inserting a blank
line between the list item text and the opening ```bash fence (and ensure
there's a blank line after the closing fence as well so the code block is
surrounded by blank lines).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 365c0acb-a4cc-4628-a974-9fdeed2674b2
📒 Files selected for processing (1)
CONTRIBUTING.md
amaslenn
approved these changes
Mar 10, 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
2 participants
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
Automatically run linters on commit. They are optimized (mostly) to run only against the diff of your commit, not entire repo
Test Plan
Additional Notes
Failure in CI example (in case one committed without running pre-commit locally)