Skip to content

Commit cdd6cae

Browse files
committed
refactor: split review guidance helpers
Made-with: Cursor
1 parent a00f4f0 commit cdd6cae

4 files changed

Lines changed: 122 additions & 77 deletions

File tree

TODO.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@
100100
- [x] `src/review/pipeline/prepare/runner.rs`: split per-diff orchestration, pre-analysis/triage decisions, and progress updates.
101101
- [x] `src/review/pipeline/context/symbols.rs`: split symbol search, snippet selection, and fallback behavior.
102102
- [x] `src/review/pipeline/context/related.rs`: separate related-file discovery from ranking/selection.
103-
- [ ] `src/review/pipeline/guidance.rs`: carve guidance assembly, repo-support guidance, and prompt-facing formatting.
103+
- [x] `src/review/pipeline/guidance.rs`: carve guidance assembly, repo-support guidance, and prompt-facing formatting.
104104
- [ ] `src/review/pipeline/session.rs`: split session construction from runtime state transitions.
105105
- [ ] `src/review/pipeline/services.rs`: separate service wiring from optional feature initialization.
106106
- [ ] `src/review/pipeline/file_context/sources.rs`: split repo sources, symbol sources, and supplemental context sources.

src/review/pipeline/guidance.rs

Lines changed: 9 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -1,85 +1,18 @@
1+
#[path = "guidance/format.rs"]
2+
mod format;
3+
#[path = "guidance/sections.rs"]
4+
mod sections;
5+
16
use crate::config;
27

8+
use format::format_guidance_sections;
9+
use sections::collect_guidance_sections;
10+
311
pub fn build_review_guidance(
412
config: &config::Config,
513
path_config: Option<&config::PathConfig>,
614
) -> Option<String> {
7-
let mut sections = Vec::new();
8-
9-
let strictness_guidance = match config.strictness {
10-
1 => "Prefer high-signal findings only. Avoid low-impact nitpicks and optional suggestions.",
11-
3 => {
12-
"Be exhaustive. Surface meaningful edge cases and maintainability concerns, including lower-severity findings."
13-
}
14-
_ => "Balance precision and coverage; prioritize clear, actionable findings.",
15-
};
16-
sections.push(format!(
17-
"Strictness ({}): {}",
18-
config.strictness, strictness_guidance
19-
));
20-
if !config.comment_types.is_empty() {
21-
sections.push(format!(
22-
"Enabled comment types: {}. Do not emit findings outside these types.",
23-
config.comment_types.join(", ")
24-
));
25-
}
26-
27-
if let Some(profile) = config.review_profile.as_deref() {
28-
let guidance = match profile {
29-
"chill" => Some(
30-
"Be conservative and only surface high-confidence, high-impact issues. Avoid nitpicks and redundant comments.",
31-
),
32-
"assertive" => Some(
33-
"Be thorough and proactive. Surface edge cases, latent risks, and maintainability concerns even if they are subtle.",
34-
),
35-
_ => None,
36-
};
37-
if let Some(text) = guidance {
38-
sections.push(format!("Review profile ({}): {}", profile, text));
39-
}
40-
}
41-
42-
if let Some(instructions) = config.review_instructions.as_deref() {
43-
let trimmed = instructions.trim();
44-
if !trimmed.is_empty() {
45-
sections.push(format!("Global review instructions:\n{}", trimmed));
46-
}
47-
}
48-
49-
if let Some(pc) = path_config {
50-
if let Some(instructions) = pc.review_instructions.as_deref() {
51-
let trimmed = instructions.trim();
52-
if !trimmed.is_empty() {
53-
sections.push(format!("Path-specific instructions:\n{}", trimmed));
54-
}
55-
}
56-
}
57-
58-
if let Some(ref lang) = config.output_language {
59-
if lang != "en" && !lang.starts_with("en-") {
60-
sections.push(format!(
61-
"Write all review comments and suggestions in {}.",
62-
lang
63-
));
64-
}
65-
}
66-
67-
if !config.include_fix_suggestions {
68-
sections.push("Do not include code fix suggestions. Only describe the issue. Do not include <<<ORIGINAL/>>>SUGGESTED blocks.".to_string());
69-
} else {
70-
sections.push(
71-
"For every finding where a concrete code fix is possible, include a code suggestion block immediately after the issue line using this exact format:\n\n<<<ORIGINAL\n<the problematic code>\n===\n<the fixed code>\n>>>SUGGESTED\n\nAlways copy the original code verbatim from the diff. Only omit the block when no concrete fix can be expressed in code.".to_string(),
72-
);
73-
}
74-
75-
if sections.is_empty() {
76-
None
77-
} else {
78-
Some(format!(
79-
"Additional review guidance:\n{}",
80-
sections.join("\n\n")
81-
))
82-
}
15+
format_guidance_sections(collect_guidance_sections(config, path_config))
8316
}
8417

8518
#[cfg(test)]
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
pub(super) fn format_guidance_sections(sections: Vec<String>) -> Option<String> {
2+
if sections.is_empty() {
3+
None
4+
} else {
5+
Some(format!(
6+
"Additional review guidance:\n{}",
7+
sections.join("\n\n")
8+
))
9+
}
10+
}
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
use crate::config;
2+
3+
pub(super) fn collect_guidance_sections(
4+
config: &config::Config,
5+
path_config: Option<&config::PathConfig>,
6+
) -> Vec<String> {
7+
let mut sections = vec![strictness_section(config)];
8+
9+
push_section(&mut sections, comment_types_section(config));
10+
push_section(&mut sections, review_profile_section(config));
11+
push_section(&mut sections, global_instructions_section(config));
12+
push_section(&mut sections, path_instructions_section(path_config));
13+
push_section(&mut sections, output_language_section(config));
14+
sections.push(fix_suggestion_section(config));
15+
16+
sections
17+
}
18+
19+
fn strictness_section(config: &config::Config) -> String {
20+
let strictness_guidance = match config.strictness {
21+
1 => "Prefer high-signal findings only. Avoid low-impact nitpicks and optional suggestions.",
22+
3 => {
23+
"Be exhaustive. Surface meaningful edge cases and maintainability concerns, including lower-severity findings."
24+
}
25+
_ => "Balance precision and coverage; prioritize clear, actionable findings.",
26+
};
27+
28+
format!(
29+
"Strictness ({}): {}",
30+
config.strictness, strictness_guidance
31+
)
32+
}
33+
34+
fn comment_types_section(config: &config::Config) -> Option<String> {
35+
if config.comment_types.is_empty() {
36+
None
37+
} else {
38+
Some(format!(
39+
"Enabled comment types: {}. Do not emit findings outside these types.",
40+
config.comment_types.join(", ")
41+
))
42+
}
43+
}
44+
45+
fn review_profile_section(config: &config::Config) -> Option<String> {
46+
let profile = config.review_profile.as_deref()?;
47+
let guidance = match profile {
48+
"chill" => Some(
49+
"Be conservative and only surface high-confidence, high-impact issues. Avoid nitpicks and redundant comments.",
50+
),
51+
"assertive" => Some(
52+
"Be thorough and proactive. Surface edge cases, latent risks, and maintainability concerns even if they are subtle.",
53+
),
54+
_ => None,
55+
}?;
56+
57+
Some(format!("Review profile ({}): {}", profile, guidance))
58+
}
59+
60+
fn global_instructions_section(config: &config::Config) -> Option<String> {
61+
let instructions = config.review_instructions.as_deref()?.trim();
62+
if instructions.is_empty() {
63+
None
64+
} else {
65+
Some(format!("Global review instructions:\n{}", instructions))
66+
}
67+
}
68+
69+
fn path_instructions_section(path_config: Option<&config::PathConfig>) -> Option<String> {
70+
let instructions = path_config?.review_instructions.as_deref()?.trim();
71+
if instructions.is_empty() {
72+
None
73+
} else {
74+
Some(format!("Path-specific instructions:\n{}", instructions))
75+
}
76+
}
77+
78+
fn output_language_section(config: &config::Config) -> Option<String> {
79+
let lang = config.output_language.as_deref()?;
80+
if lang == "en" || lang.starts_with("en-") {
81+
None
82+
} else {
83+
Some(format!(
84+
"Write all review comments and suggestions in {}.",
85+
lang
86+
))
87+
}
88+
}
89+
90+
fn fix_suggestion_section(config: &config::Config) -> String {
91+
if !config.include_fix_suggestions {
92+
"Do not include code fix suggestions. Only describe the issue. Do not include <<<ORIGINAL/>>>SUGGESTED blocks.".to_string()
93+
} else {
94+
"For every finding where a concrete code fix is possible, include a code suggestion block immediately after the issue line using this exact format:\n\n<<<ORIGINAL\n<the problematic code>\n===\n<the fixed code>\n>>>SUGGESTED\n\nAlways copy the original code verbatim from the diff. Only omit the block when no concrete fix can be expressed in code.".to_string()
95+
}
96+
}
97+
98+
fn push_section(sections: &mut Vec<String>, section: Option<String>) {
99+
if let Some(section) = section {
100+
sections.push(section);
101+
}
102+
}

0 commit comments

Comments
 (0)