chore: add coderabbit configuration and coding guidelines for coderabbit#1145
chore: add coderabbit configuration and coding guidelines for coderabbit#1145chtruong814 merged 3 commits intomainfrom
Conversation
Signed-off-by: Terry Kong <terryk@nvidia.com>
ℹ️ File Consistency CheckCheck based on commit: ba5a5b3 (PR #1145 from This is a test comment This check ensures that related file implementations remain synchronized across the codebase. If you believe this warning is incorrect or the files should intentionally differ, please add a comment explaining the reasoning. |
📝 WalkthroughWalkthroughAdds two CodeRabbit configuration files defining review workflows, tooling, pre-merge checks, and knowledge-base settings, plus a new CODING_GUIDELINES.md outlining coding, documentation, testing, and licensing conventions. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
CODING_GUIDELINES.md (3)
13-15: Fix heading hierarchy to satisfy markdownlint (MD001).Adjust h4→h3 and h5→h4 where appropriate.
-#### Use uv run instead of python +### Use uv run instead of python -#### Python Standard +### Python Standard -#### Indentation +### Indentation -#### Imports +### Imports -#### Naming +### Naming -##### Identifier Format +#### Identifier Format -##### Identifier Guidelines +#### Identifier Guidelines -#### Comments +### Comments -#### Docstring Syntax +### Docstring Syntax -##### Classes and Functions +#### Classes and Functions -##### Attributes and Variables +#### Attributes and Variables -#### Avoid Reflection +### Avoid Reflection -#### Error Handling +### Error Handling -#### Ensure docs/index.md is up to date +### Ensure docs/index.md is up to dateAlso applies to: 33-41, 36-38, 39-41, 61-63, 63-83, 83-86, 87-92, 92-95, 93-96, 96-111, 111-113, 129-131, 175-181
96-109: Remove extra space in heading (MD019).-##### Attributes and Variables +#### Attributes and Variables
221-224: Add languages to fenced code blocks (MD040).Use "text" for patterns, filenames, and directory trees.
-``` +```text <algo>-<model>-<nodes>n<gpus>g-<strategy-and-params>[-modifiers][-long][.vN].(yaml|sh)-
+text
sft-llama3.1-8b-1n8g-fsdp2tp1-long.yaml
dpo-llama3.1-8b-instruct-4n8g-fsdp2tp4.yaml
grpo-llama3.1-8b-instruct-1n8g-megatron-fp8.yaml
grpo-qwen2.5-7b-instruct-4n8g-fsdp2tp4sp.v3.yaml-``` +```text vlm_<algo>-<model>-<nodes>n<gpus>g-<strategy>[-modifiers][.vN].(yaml|sh)-
+text
vlm_grpo-qwen2.5-vl-3b-instruct-clevr-1n2g-dtensor2tp1.v1.yaml
vlm_grpo-smolvlm2-2.2b-instruct-clevr-1n2g-dtensor2tp1.v1.sh-``` +```text examples/configs/recipes/ llm/ <name>.yaml vlm/ <name>.yaml tests/test_suites/ llm/ common.env <name>.sh vlm/ common.env <name>.sh nightly.txtAlso applies to: 235-240, 244-247, 250-253, 261-276 </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: Path: .coderabbit.yaml **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 5a9f7acc59ed70e6eb52dd065a55ec015c895204 and ba5a5b33f95133538dc8080ce67d028a97725259. </details> <details> <summary>📒 Files selected for processing (3)</summary> * `.coderabbit-current.yaml` (1 hunks) * `.coderabbit.yaml` (1 hunks) * `CODING_GUIDELINES.md` (1 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> <details> <summary>CODING_GUIDELINES.md</summary> 13-13: Heading levels should only increment by one level at a time Expected: h3; Actual: h4 (MD001, heading-increment) --- 33-33: Heading levels should only increment by one level at a time Expected: h3; Actual: h4 (MD001, heading-increment) --- 96-96: Multiple spaces after hash on atx style heading (MD019, no-multiple-space-atx) --- 177-177: Heading levels should only increment by one level at a time Expected: h3; Actual: h4 (MD001, heading-increment) --- 221-221: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 235-235: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 244-244: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 250-250: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 261-261: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> <details> <summary>🪛 YAMLlint (1.37.1)</summary> <details> <summary>.coderabbit-current.yaml</summary> [error] 13-13: duplication of key "tone_instructions" in mapping (key-duplicates) --- [error] 14-14: duplication of key "early_access" in mapping (key-duplicates) --- [error] 15-15: duplication of key "enable_free_tier" in mapping (key-duplicates) --- [error] 376-376: syntax error: expected '<document start>', but found '<block mapping start>' (syntax) --- [error] 457-457: no new line character at the end of file (new-line-at-end-of-file) </details> </details> </details> <details> <summary>⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)</summary> * GitHub Check: Lint check * GitHub Check: Post submodule check comment / Comment on PR </details> <details> <summary>🔇 Additional comments (4)</summary><blockquote> <details> <summary>CODING_GUIDELINES.md (1)</summary><blockquote> `84-86`: **Typo: “memberes” → “members”.** [raise_nitpick_refactor] ```diff -2. Initialize all externally visible memberes of a class in the constructor. +2. Initialize all externally visible members of a class in the constructor..coderabbit.yaml (2)
22-62: Config looks consistent with schema; KB guidelines integration is correctly wired.
35-41: Confirm intent for disabled commit/review status and poem.
- commit_status: false and review_status: false will suppress CodeRabbit status checks/comments.
- poem: false disables the walkthrough poem.
Would you like these on or off for this repo?
Also applies to: 58-63, 36-39
.coderabbit-current.yaml (1)
457-457: Add newline at EOF (yamllint new-line-at-end-of-file).
[raise_nitpick_refactor]
Signed-off-by: Terry Kong <terryk@nvidia.com>
ℹ️ File Consistency CheckCheck based on commit: 711bcd3 (PR #1145 from This is a test comment This check ensures that related file implementations remain synchronized across the codebase. If you believe this warning is incorrect or the files should intentionally differ, please add a comment explaining the reasoning. |
Signed-off-by: Terry Kong <terryk@nvidia.com>
ℹ️ File Consistency CheckCheck based on commit: b4934d9 (PR #1145 from This is a test comment This check ensures that related file implementations remain synchronized across the codebase. If you believe this warning is incorrect or the files should intentionally differ, please add a comment explaining the reasoning. |
chtruong814
removed this pull request from the merge queue due to the queue being cleared
…bit (#1145) Signed-off-by: Terry Kong <terryk@nvidia.com>
…bit (NVIDIA-NeMo#1145) Signed-off-by: Terry Kong <terryk@nvidia.com>
3 participants
Adds a coderabbit configuration and some basic rules for coderabbit to make review more relevant to our repo. In the future it can pick up the project wide cursor rules.
One notable review change is it will do a pre merge check and check if the change warrants the author posting a summary of the perf change or if they shared convergence results if the change is substantial or will affect either of those two categories.
This is based on TRTLLM's coderabbit configuration: https://github.com/NVIDIA/TensorRT-LLM/blob/e3117731b3a86251ab28f45c36f48db6edba2e29/.coderabbit.yaml
Summary by CodeRabbit
Documentation
Chores