Update CodeRabbit instructions #4962
Conversation
Signed-off-by: Tomoyuki Morita <moritato@amazon.com>
ykmr1224
added
the
infrastructure
ykmr1224
requested review from
GumpacG,
LantaoJin,
MaxKsyunz,
RyanL1997,
Swiddis,
YANG-DB,
Yury-Fridlyand,
acarbonetto,
anirudha,
dai-chen,
derek-ho,
forestmvey,
joshuali925,
kavithacm,
mengweieric,
noCharger,
penghuo,
ps48,
qianheng-aws,
seankao-az,
vamsimanohar and
yuancu
as code owners
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThis update enriches the .coderabbit.yaml configuration file with comprehensive review instructions, adding an overarching guidance block and multiple path-specific review criteria covering grammar files, Java code patterns, tests, integration tests, PPL, Calcite integration, and documentation standards. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.coderabbit.yaml(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: opensearch-project/sql PR: 0
File: .rules/REVIEW_GUIDELINES.md:0-0
Timestamp: 2025-12-02T17:27:55.938Z
Learning: Encourage comprehensive test coverage in code reviews
Learnt from: CR
Repo: opensearch-project/sql PR: 0
File: .rules/REVIEW_GUIDELINES.md:0-0
Timestamp: 2025-12-02T17:27:55.938Z
Learning: Encourage meaningful error messages in code reviews
Learnt from: CR
Repo: opensearch-project/sql PR: 0
File: .rules/REVIEW_GUIDELINES.md:0-0
Timestamp: 2025-12-02T17:27:55.938Z
Learning: Encourage clear and descriptive naming in code reviews
📚 Learning: 2025-12-02T17:27:55.938Z
Learnt from: CR
Repo: opensearch-project/sql PR: 0
File: .rules/REVIEW_GUIDELINES.md:0-0
Timestamp: 2025-12-02T17:27:55.938Z
Learning: Encourage comprehensive test coverage in code reviews
Applied to files:
.coderabbit.yaml
⏰ 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). (27)
- GitHub Check: bwc-tests-full-restart (21)
- GitHub Check: build-linux (25, doc)
- GitHub Check: build-linux (21, integration)
- GitHub Check: build-linux (25, integration)
- GitHub Check: build-linux (21, unit)
- GitHub Check: build-linux (21, doc)
- GitHub Check: build-linux (25, unit)
- GitHub Check: bwc-tests-full-restart (25)
- GitHub Check: bwc-tests-rolling-upgrade (21)
- GitHub Check: bwc-tests-rolling-upgrade (25)
- GitHub Check: security-it-linux (21)
- GitHub Check: security-it-linux (25)
- GitHub Check: build-windows-macos (macos-14, 21, unit)
- GitHub Check: build-windows-macos (windows-latest, 21, -PbuildPlatform=windows, integration)
- GitHub Check: build-windows-macos (macos-14, 25, doc)
- GitHub Check: build-windows-macos (windows-latest, 25, -PbuildPlatform=windows, integration)
- GitHub Check: build-windows-macos (macos-14, 25, unit)
- GitHub Check: build-windows-macos (macos-14, 25, integration)
- GitHub Check: build-windows-macos (macos-14, 21, doc)
- GitHub Check: build-windows-macos (windows-latest, 25, -PbuildPlatform=windows, unit)
- GitHub Check: build-windows-macos (macos-14, 21, integration)
- GitHub Check: build-windows-macos (windows-latest, 21, -PbuildPlatform=windows, unit)
- GitHub Check: security-it-windows-macos (macos-14, 25)
- GitHub Check: security-it-windows-macos (macos-14, 21)
- GitHub Check: security-it-windows-macos (windows-latest, 21)
- GitHub Check: security-it-windows-macos (windows-latest, 25)
- GitHub Check: CodeQL-Scan (java)
🔇 Additional comments (6)
.coderabbit.yaml (6)
36-48: LGTM!The grammar file instructions are well-structured, covering scope creep detection, pattern adherence, and code reuse considerations—all relevant for ANTLR grammar changes.
50-64: LGTM with minor observation.The Java file instructions are comprehensive, covering complexity thresholds, naming conventions, error handling, and resource management. The 50-line method and 500-line class thresholds are reasonable heuristics.
Note: Line 58's instruction to "Flag code that appears AI-generated" is inherently subjective and may be difficult to enforce consistently. Consider whether this provides actionable guidance or could be removed.
66-86: LGTM!The core Java path instructions provide clear, enforceable requirements including mandatory unit tests for new functions, JavaDoc standards, and AST immutability patterns. This aligns well with the project's emphasis on comprehensive test coverage. Based on learnings.
88-101: LGTM!Thorough test coverage validation criteria including NULL handling, boundary conditions, error cases, and naming conventions. This enforces meaningful assertions over smoke tests. Based on learnings about encouraging comprehensive test coverage.
103-121: LGTM!The integration test instructions provide actionable guidance specific to this project's test infrastructure, including validation of test data resources and the specific example about non-existent indices. The addition of test resources path instruction (lines 117-121) ensures test data quality.
123-157: LGTM!Well-tailored instructions for PPL parser changes, Calcite integration patterns, and documentation quality. The specific guidance for
CalciteRelNodeVisitor.java(lines 143-148) acknowledges known complexity and encourages refactoring—a pragmatic approach for maintaining readability in complex files.
| # General review instructions (applied to all reviews) | ||
| instructions: | ||
| # Architectural Decision Prompts | ||
| - "For new features: Check if similar functionality exists that could be enhanced instead" | ||
| - "Question whether new code is needed vs reusing/extending existing code" | ||
| - "Identify opportunities for code reuse across the codebase" |
There was a problem hiding this comment.
Schema violation: instructions property is not valid under reviews.
According to the CodeRabbit configuration schema, the reviews object does not support a top-level instructions property. The schema has additionalProperties: false, so this block will likely be ignored or cause validation errors.
The valid approach is to include these general instructions within individual path_instructions entries or use a catch-all pattern.
Consider moving these instructions into a catch-all path instruction:
- # General review instructions (applied to all reviews)
- instructions:
- # Architectural Decision Prompts
- - "For new features: Check if similar functionality exists that could be enhanced instead"
- - "Question whether new code is needed vs reusing/extending existing code"
- - "Identify opportunities for code reuse across the codebase"
-
# Path-specific review instructions
path_instructions:
+ # General review instructions (applied to all files)
+ - path: "**/*"
+ instructions: |
+ - For new features: Check if similar functionality exists that could be enhanced instead
+ - Question whether new code is needed vs reusing/extending existing code
+ - Identify opportunities for code reuse across the codebase
+
# Grammar Files - Architectural Decision Prompts
- path: "**/*.g4"🤖 Prompt for AI Agents
.coderabbit.yaml lines 27-32: the top-level "instructions" key is invalid under
"reviews" and will violate the schema; remove this block and instead add these
strings as a catch-all path instruction under "reviews.path_instructions" (e.g.
a new entry with a pattern that matches all files, like ".*" or a suitable
repo-wide pattern) placing the listed strings under that entry's "instructions"
array so the YAML conforms to the schema and the guidance is applied globally.
dai-chen
left a comment
dai-chen
left a comment
There was a problem hiding this comment.
Thanks for the changes!
3 participants
Description
Related Issues
Check List
--signoffor-s.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.