refactor: split review response processing helpers

Made-with: Cursor

Author: haasonsaas

TODO.md

@@ -95,7 +95,7 @@
 
 ### Review pipeline backlog
 
-- [ ] `src/review/pipeline/execution/responses/processing.rs`: split raw response normalization, comment extraction, and merge logic.
+- [x] `src/review/pipeline/execution/responses/processing.rs`: split raw response normalization, comment extraction, and merge logic.
 - [ ] `src/review/pipeline/execution/responses/validation.rs`: separate schema validation from per-comment sanitization.
 - [ ] `src/review/pipeline/prepare/runner.rs`: split per-diff orchestration, pre-analysis/triage decisions, and progress updates.
 - [ ] `src/review/pipeline/context/symbols.rs`: split symbol search, snippet selection, and fallback behavior.

src/review/pipeline/execution/responses/processing.rs

@@ -1,15 +1,18 @@
-use anyhow::Result;
+#[path = "processing/comments.rs"]
+mod comments;
+#[path = "processing/fallback.rs"]
+mod fallback;
+#[path = "processing/merge.rs"]
+mod merge;
+#[path = "processing/run.rs"]
+mod run;
+#[path = "processing/usage.rs"]
+mod usage;
+
 use std::path::PathBuf;
-use tracing::warn;
 
-use crate::adapters::llm::LLMResponse;
 use crate::core;
-use crate::parsing::parse_llm_response;
-use crate::review::{apply_rule_overrides, filter_comments_for_diff, AgentActivity};
-
-use super::super::dispatcher::DispatchedJobResult;
-use super::overrides::{apply_path_severity_overrides, apply_specialized_pass_tags};
-use super::validation::validate_llm_response;
+use crate::review::AgentActivity;
 
 pub(in super::super) struct ProcessedJobResult {
     pub file_path: PathBuf,
@@ -31,131 +34,4 @@ struct ResponseUsage {
     total_tokens: usize,
 }
 
-pub(in super::super) fn process_job_result(
-    result: DispatchedJobResult,
-    diff: &core::UnifiedDiff,
-    is_local: bool,
-) -> Result<ProcessedJobResult> {
-    let DispatchedJobResult {
-        active_rules,
-        path_config,
-        file_path,
-        deterministic_comments,
-        pass_kind,
-        mark_file_complete,
-        response,
-        latency_ms,
-        agent_data,
-        ..
-    } = result;
-
-    match response {
-        Err(error) => {
-            warn!("LLM request failed for {}: {}", file_path.display(), error);
-            Ok(fallback_result(
-                file_path,
-                deterministic_comments,
-                mark_file_complete,
-                latency_ms,
-                ResponseUsage::default(),
-                agent_data,
-            ))
-        }
-        Ok(response) => {
-            let usage = response_usage(&response);
-
-            if let Err(validation_error) = validate_llm_response(&response.content) {
-                eprintln!(
-                    "Warning: LLM response validation failed for {}: {}",
-                    file_path.display(),
-                    validation_error
-                );
-                if is_local {
-                    eprintln!(
-                        "Hint: Try a larger model or reduce diff size for better results with local models."
-                    );
-                }
-                return Ok(fallback_result(
-                    file_path,
-                    deterministic_comments,
-                    mark_file_complete,
-                    latency_ms,
-                    usage,
-                    agent_data,
-                ));
-            }
-
-            let Ok(raw_comments) = parse_llm_response(&response.content, &diff.file_path) else {
-                return Ok(fallback_result(
-                    file_path,
-                    deterministic_comments,
-                    mark_file_complete,
-                    latency_ms,
-                    usage,
-                    agent_data,
-                ));
-            };
-
-            let mut comments = core::CommentSynthesizer::synthesize(raw_comments)?;
-            apply_specialized_pass_tags(&mut comments, pass_kind);
-            apply_path_severity_overrides(&mut comments, path_config.as_ref());
-
-            let comments = apply_rule_overrides(comments, &active_rules);
-
-            let mut comments = filter_comments_for_diff(diff, comments);
-            comments.extend(deterministic_comments);
-
-            Ok(ProcessedJobResult {
-                file_path,
-                comment_count: comments.len(),
-                comments,
-                latency_ms,
-                prompt_tokens: usage.prompt_tokens,
-                completion_tokens: usage.completion_tokens,
-                total_tokens: usage.total_tokens,
-                pass_tag: Some(
-                    pass_kind
-                        .map(|kind| kind.tag().to_string())
-                        .unwrap_or_else(|| "default".to_string()),
-                ),
-                mark_file_complete,
-                agent_data,
-            })
-        }
-    }
-}
-
-fn fallback_result(
-    file_path: PathBuf,
-    comments: Vec<core::Comment>,
-    mark_file_complete: bool,
-    latency_ms: u64,
-    usage: ResponseUsage,
-    agent_data: Option<AgentActivity>,
-) -> ProcessedJobResult {
-    let comment_count = comments.len();
-    ProcessedJobResult {
-        file_path,
-        comments,
-        latency_ms,
-        prompt_tokens: usage.prompt_tokens,
-        completion_tokens: usage.completion_tokens,
-        total_tokens: usage.total_tokens,
-        comment_count,
-        pass_tag: None,
-        mark_file_complete,
-        agent_data,
-    }
-}
-
-fn response_usage(response: &LLMResponse) -> ResponseUsage {
-    response
-        .usage
-        .as_ref()
-        .map(|usage| ResponseUsage {
-            prompt_tokens: usage.prompt_tokens,
-            completion_tokens: usage.completion_tokens,
-            total_tokens: usage.total_tokens,
-        })
-        .unwrap_or_default()
-}
+pub(in super::super) use run::process_job_result;

src/review/pipeline/execution/responses/processing/comments.rs

@@ -0,0 +1,27 @@
+use anyhow::Result;
+
+use crate::config;
+use crate::core;
+use crate::parsing::parse_llm_response;
+use crate::review::apply_rule_overrides;
+
+use super::super::overrides::{apply_path_severity_overrides, apply_specialized_pass_tags};
+
+pub(super) fn extract_processed_comments(
+    response_content: &str,
+    diff: &core::UnifiedDiff,
+    active_rules: &[core::ReviewRule],
+    path_config: Option<&config::PathConfig>,
+    pass_kind: Option<core::SpecializedPassKind>,
+) -> Result<Option<Vec<core::Comment>>> {
+    let Ok(raw_comments) = parse_llm_response(response_content, &diff.file_path) else {
+        return Ok(None);
+    };
+
+    let mut comments = core::CommentSynthesizer::synthesize(raw_comments)?;
+    apply_specialized_pass_tags(&mut comments, pass_kind);
+    apply_path_severity_overrides(&mut comments, path_config);
+
+    let comments = apply_rule_overrides(comments, active_rules);
+    Ok(Some(comments))
+}

src/review/pipeline/execution/responses/processing/fallback.rs

@@ -0,0 +1,29 @@
+use std::path::PathBuf;
+
+use crate::core;
+use crate::review::AgentActivity;
+
+use super::{ProcessedJobResult, ResponseUsage};
+
+pub(super) fn fallback_result(
+    file_path: PathBuf,
+    comments: Vec<core::Comment>,
+    mark_file_complete: bool,
+    latency_ms: u64,
+    usage: ResponseUsage,
+    agent_data: Option<AgentActivity>,
+) -> ProcessedJobResult {
+    let comment_count = comments.len();
+    ProcessedJobResult {
+        file_path,
+        comments,
+        latency_ms,
+        prompt_tokens: usage.prompt_tokens,
+        completion_tokens: usage.completion_tokens,
+        total_tokens: usage.total_tokens,
+        comment_count,
+        pass_tag: None,
+        mark_file_complete,
+        agent_data,
+    }
+}

src/review/pipeline/execution/responses/processing/merge.rs

@@ -0,0 +1,12 @@
+use crate::core;
+use crate::review::filter_comments_for_diff;
+
+pub(super) fn merge_processed_comments(
+    diff: &core::UnifiedDiff,
+    comments: Vec<core::Comment>,
+    deterministic_comments: Vec<core::Comment>,
+) -> Vec<core::Comment> {
+    let mut comments = filter_comments_for_diff(diff, comments);
+    comments.extend(deterministic_comments);
+    comments
+}

src/review/pipeline/execution/responses/processing/run.rs

@@ -0,0 +1,106 @@
+use anyhow::Result;
+use tracing::warn;
+
+use crate::core;
+
+use super::super::super::dispatcher::DispatchedJobResult;
+use super::super::validation::validate_llm_response;
+use super::comments::extract_processed_comments;
+use super::fallback::fallback_result;
+use super::merge::merge_processed_comments;
+use super::usage::response_usage;
+use super::{ProcessedJobResult, ResponseUsage};
+
+pub(in super::super::super) fn process_job_result(
+    result: DispatchedJobResult,
+    diff: &core::UnifiedDiff,
+    is_local: bool,
+) -> Result<ProcessedJobResult> {
+    let DispatchedJobResult {
+        active_rules,
+        path_config,
+        file_path,
+        deterministic_comments,
+        pass_kind,
+        mark_file_complete,
+        response,
+        latency_ms,
+        agent_data,
+        ..
+    } = result;
+
+    match response {
+        Err(error) => {
+            warn!("LLM request failed for {}: {}", file_path.display(), error);
+            Ok(fallback_result(
+                file_path,
+                deterministic_comments,
+                mark_file_complete,
+                latency_ms,
+                ResponseUsage::default(),
+                agent_data,
+            ))
+        }
+        Ok(response) => {
+            let usage = response_usage(&response);
+
+            if let Err(validation_error) = validate_llm_response(&response.content) {
+                eprintln!(
+                    "Warning: LLM response validation failed for {}: {}",
+                    file_path.display(),
+                    validation_error
+                );
+                if is_local {
+                    eprintln!(
+                        "Hint: Try a larger model or reduce diff size for better results with local models."
+                    );
+                }
+                return Ok(fallback_result(
+                    file_path,
+                    deterministic_comments,
+                    mark_file_complete,
+                    latency_ms,
+                    usage,
+                    agent_data,
+                ));
+            }
+
+            let Some(comments) = extract_processed_comments(
+                &response.content,
+                diff,
+                &active_rules,
+                path_config.as_ref(),
+                pass_kind,
+            )?
+            else {
+                return Ok(fallback_result(
+                    file_path,
+                    deterministic_comments,
+                    mark_file_complete,
+                    latency_ms,
+                    usage,
+                    agent_data,
+                ));
+            };
+
+            let comments = merge_processed_comments(diff, comments, deterministic_comments);
+
+            Ok(ProcessedJobResult {
+                file_path,
+                comment_count: comments.len(),
+                comments,
+                latency_ms,
+                prompt_tokens: usage.prompt_tokens,
+                completion_tokens: usage.completion_tokens,
+                total_tokens: usage.total_tokens,
+                pass_tag: Some(
+                    pass_kind
+                        .map(|kind| kind.tag().to_string())
+                        .unwrap_or_else(|| "default".to_string()),
+                ),
+                mark_file_complete,
+                agent_data,
+            })
+        }
+    }
+}

src/review/pipeline/execution/responses/processing/usage.rs

@@ -0,0 +1,15 @@
+use crate::adapters::llm::LLMResponse;
+
+use super::ResponseUsage;
+
+pub(super) fn response_usage(response: &LLMResponse) -> ResponseUsage {
+    response
+        .usage
+        .as_ref()
+        .map(|usage| ResponseUsage {
+            prompt_tokens: usage.prompt_tokens,
+            completion_tokens: usage.completion_tokens,
+            total_tokens: usage.total_tokens,
+        })
+        .unwrap_or_default()
+}