Prompt review agents to gather codebase context before judging diffs#22
Prompt review agents to gather codebase context before judging diffs#22justinmoon merged 1 commit intomasterfrom
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR adds contextual guidance to instruction strings returned by the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
crates/rally-workflow-build/src/lib.rs (2)
219-227: Duplicated guidance text; consider extracting to a constant.The "Before judging the diff, gather context:" block is identical in both FinalReview and step Review branches. Extracting this to a shared constant improves maintainability and ensures the guidance stays consistent across review types.
♻️ Suggested extraction
Add a constant at module level:
const REVIEW_CONTEXT_GUIDANCE: &str = r#" Before judging the diff, gather context: - Read modified files in full to understand the surrounding code, not just the changed lines. - Identify callers and consumers of any new or changed functions, types, or APIs. - Check that the changes are consistent with patterns and conventions used elsewhere in the codebase. "#;Then reference it in both format strings to avoid duplication.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/rally-workflow-build/src/lib.rs` around lines 219 - 227, Duplicate "Before judging the diff, gather context:" block should be extracted to a shared module-level constant (e.g., REVIEW_CONTEXT_GUIDANCE) and referenced in both places that build the review message (the FinalReview branch and the Review step branch where the format! call uses step.number/step.title/checkpoint.commit_sha/format_criteria(&step.acceptance_criteria)/state.name/agent). Replace the repeated multiline text in those format! invocations with the constant to remove duplication and keep messages consistent across FinalReview and step Review.
213-216: Consider using a multiline raw string for readability.This instruction string is quite long and hard to read with embedded
\ncharacters. A raw string literal orindocmacro would improve maintainability.♻️ Suggested refactor using raw string
if state.phase == SessionPhase::FinalReview { - Some(format!( - "Final holistic review of full implementation\nCommit: {}\nReview the complete change for cross-step integration, architecture consistency, and missing risks.\n\nBefore judging the diff, gather context:\n- Read modified files in full to understand the surrounding code, not just the changed lines.\n- Identify callers and consumers of any new or changed functions, types, or APIs.\n- Check that the changes are consistent with patterns and conventions used elsewhere in the codebase.\n\nWhen done, run these commands IN ORDER:\n1. rally build review --session {} --as {} --verdict APPROVE|CHANGES_REQUESTED|BLOCKED\n2. /rally (to get your next instruction — do NOT stop here)", - checkpoint.commit_sha, state.name, agent - )) + Some(format!( + r#"Final holistic review of full implementation +Commit: {} +Review the complete change for cross-step integration, architecture consistency, and missing risks. + +Before judging the diff, gather context: +- Read modified files in full to understand the surrounding code, not just the changed lines. +- Identify callers and consumers of any new or changed functions, types, or APIs. +- Check that the changes are consistent with patterns and conventions used elsewhere in the codebase. + +When done, run these commands IN ORDER: +1. rally build review --session {} --as {} --verdict APPROVE|CHANGES_REQUESTED|BLOCKED +2. /rally (to get your next instruction — do NOT stop here)"#, + checkpoint.commit_sha, state.name, agent + ))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/rally-workflow-build/src/lib.rs` around lines 213 - 216, Replace the long escaped string inside the Some(format!(...)) call with a multiline raw string (or indoc::formatdoc!) to improve readability; specifically, in the expression that uses checkpoint.commit_sha, state.name, and agent in lib.rs (the Some(format!(...)) block), change the format! invocation to use a raw string literal (e.g. format!(r#"Final holistic review...Commit: {} ..."#, checkpoint.commit_sha, state.name, agent)) or indoc::formatdoc! so the embedded newlines become actual lines and the text is easier to maintain.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/rally-workflow-build/src/lib.rs`:
- Around line 219-227: Duplicate "Before judging the diff, gather context:"
block should be extracted to a shared module-level constant (e.g.,
REVIEW_CONTEXT_GUIDANCE) and referenced in both places that build the review
message (the FinalReview branch and the Review step branch where the format!
call uses
step.number/step.title/checkpoint.commit_sha/format_criteria(&step.acceptance_criteria)/state.name/agent).
Replace the repeated multiline text in those format! invocations with the
constant to remove duplication and keep messages consistent across FinalReview
and step Review.
- Around line 213-216: Replace the long escaped string inside the
Some(format!(...)) call with a multiline raw string (or indoc::formatdoc!) to
improve readability; specifically, in the expression that uses
checkpoint.commit_sha, state.name, and agent in lib.rs (the Some(format!(...))
block), change the format! invocation to use a raw string literal (e.g.
format!(r#"Final holistic review...Commit: {} ..."#, checkpoint.commit_sha,
state.name, agent)) or indoc::formatdoc! so the embedded newlines become actual
lines and the text is easier to maintain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 112e7f19-c958-4c82-bd20-c43c692fe3c4
📒 Files selected for processing (1)
crates/rally-workflow-build/src/lib.rs
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
22bcbfa to
58105e3
Compare
Summary
Test plan
cargo buildpasses🤖 Generated with Claude Code
Summary by CodeRabbit