refactor: split review response validation helpers

Made-with: Cursor

Author: haasonsaas

TODO.md

@@ -96,7 +96,7 @@
 ### Review pipeline backlog
 
 - [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.
+- [x] `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/context/related.rs`: separate related-file discovery from ranking/selection.

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

@@ -1,15 +1,19 @@
+#[path = "validation/contract.rs"]
+mod contract;
+#[path = "validation/repetition.rs"]
+mod repetition;
+
+use contract::validate_structured_review_payload;
+use repetition::has_excessive_repetition;
+
 pub(super) fn validate_llm_response(response: &str) -> Result<(), String> {
     let trimmed = response.trim();
     if trimmed.is_empty() {
         return Err("Empty response from model".to_string());
     }
 
-    if let Ok(value) = serde_json::from_str::<serde_json::Value>(trimmed) {
-        if is_structured_review_payload(&value) {
-            return Ok(());
-        }
-
-        return Err("JSON response did not match the review output contract".to_string());
+    if validate_structured_review_payload(trimmed)? {
+        return Ok(());
     }
 
     if response.len() < 10 {
@@ -23,50 +27,6 @@ pub(super) fn validate_llm_response(response: &str) -> Result<(), String> {
     Ok(())
 }
 
-fn is_structured_review_payload(value: &serde_json::Value) -> bool {
-    let items = if let Some(array) = value.as_array() {
-        array
-    } else if let Some(array) = value
-        .get("comments")
-        .or_else(|| value.get("findings"))
-        .or_else(|| value.get("results"))
-        .and_then(|items| items.as_array())
-    {
-        array
-    } else {
-        return false;
-    };
-
-    items.iter().all(|item| {
-        item.is_object()
-            && (item.get("line").is_some()
-                || item.get("line_number").is_some()
-                || item.get("content").is_some()
-                || item.get("issue").is_some())
-    })
-}
-
-pub(super) fn has_excessive_repetition(text: &str) -> bool {
-    if text.len() < 100 {
-        return false;
-    }
-    let window = 20.min(text.len() / 5);
-    let search_end = text.len().saturating_sub(window);
-    for start in 0..search_end.max(1) {
-        if !text.is_char_boundary(start) || !text.is_char_boundary(start + window) {
-            continue;
-        }
-        let pattern = &text[start..start + window];
-        if pattern.trim().is_empty() {
-            continue;
-        }
-        if text.matches(pattern).count() > 5 {
-            return true;
-        }
-    }
-    false
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;

src/review/pipeline/execution/responses/validation/contract.rs

@@ -0,0 +1,34 @@
+pub(super) fn validate_structured_review_payload(response: &str) -> Result<bool, String> {
+    let Ok(value) = serde_json::from_str::<serde_json::Value>(response) else {
+        return Ok(false);
+    };
+
+    if is_structured_review_payload(&value) {
+        return Ok(true);
+    }
+
+    Err("JSON response did not match the review output contract".to_string())
+}
+
+fn is_structured_review_payload(value: &serde_json::Value) -> bool {
+    let items = if let Some(array) = value.as_array() {
+        array
+    } else if let Some(array) = value
+        .get("comments")
+        .or_else(|| value.get("findings"))
+        .or_else(|| value.get("results"))
+        .and_then(|items| items.as_array())
+    {
+        array
+    } else {
+        return false;
+    };
+
+    items.iter().all(|item| {
+        item.is_object()
+            && (item.get("line").is_some()
+                || item.get("line_number").is_some()
+                || item.get("content").is_some()
+                || item.get("issue").is_some())
+    })
+}

src/review/pipeline/execution/responses/validation/repetition.rs

@@ -0,0 +1,20 @@
+pub(super) fn has_excessive_repetition(text: &str) -> bool {
+    if text.len() < 100 {
+        return false;
+    }
+    let window = 20.min(text.len() / 5);
+    let search_end = text.len().saturating_sub(window);
+    for start in 0..search_end.max(1) {
+        if !text.is_char_boundary(start) || !text.is_char_boundary(start + window) {
+            continue;
+        }
+        let pattern = &text[start..start + window];
+        if pattern.trim().is_empty() {
+            continue;
+        }
+        if text.matches(pattern).count() > 5 {
+            return true;
+        }
+    }
+    false
+}