chore: fix all clippy -D warnings + add pre-commit hooks

- Fix items_after_test_module in misc.rs, context.rs, interactive.rs
  by moving code before test modules
- Fix cloned_ref_to_slice_refs: use std::slice::from_ref
- Fix redundant_closure in compression.rs
- Fix len_zero comparisons in context.rs and verification.rs
- Fix manual_range_contains in smart_response.rs
- Add .githooks/pre-commit running fmt, clippy -D warnings, and tests

983 tests pass, zero clippy warnings.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: haasonsaas

.githooks/pre-commit

@@ -0,0 +1,25 @@
+#!/usr/bin/env bash
+set -e
+
+echo "==> Running cargo fmt --check..."
+cargo fmt --check
+if [ $? -ne 0 ]; then
+    echo "ERROR: cargo fmt found formatting issues. Run 'cargo fmt' to fix."
+    exit 1
+fi
+
+echo "==> Running cargo clippy..."
+cargo clippy --all-targets -- -D warnings
+if [ $? -ne 0 ]; then
+    echo "ERROR: cargo clippy found warnings. Fix them before committing."
+    exit 1
+fi
+
+echo "==> Running cargo test..."
+cargo test
+if [ $? -ne 0 ]; then
+    echo "ERROR: Tests failed. Fix them before committing."
+    exit 1
+fi
+
+echo "==> All pre-commit checks passed."

src/commands/misc.rs

@@ -404,76 +404,6 @@ fn resolve_convention_store_path_for_feedback(config: &config::Config) -> Option
     dirs::data_local_dir().map(|d| d.join("diffscope").join("conventions.json"))
 }
 
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    #[test]
-    fn test_select_discussion_comment_empty_comments() {
-        // Should return an error, not panic
-        let result = select_discussion_comment(&[], None, None);
-        assert!(result.is_err());
-    }
-
-    #[test]
-    fn test_select_discussion_comment_defaults_to_first() {
-        let comment = core::Comment {
-            id: "cmt_1".to_string(),
-            file_path: PathBuf::from("test.rs"),
-            line_number: 1,
-            content: "test".to_string(),
-            rule_id: None,
-            severity: core::comment::Severity::Info,
-            category: core::comment::Category::BestPractice,
-            suggestion: None,
-            confidence: 0.8,
-            code_suggestion: None,
-            tags: vec![],
-            fix_effort: core::comment::FixEffort::Low,
-            feedback: None,
-        };
-        let result = select_discussion_comment(&[comment.clone()], None, None).unwrap();
-        assert_eq!(result.id, "cmt_1");
-    }
-
-    #[test]
-    fn test_feedback_stats_not_double_counted() {
-        // Simulate accepting the same comment twice — stats should only increment once
-        let mut store = review::FeedbackStore::default();
-        let comment = core::Comment {
-            id: "cmt_dup".to_string(),
-            file_path: PathBuf::from("test.rs"),
-            line_number: 1,
-            content: "test".to_string(),
-            rule_id: None,
-            severity: core::comment::Severity::Warning,
-            category: core::comment::Category::Bug,
-            suggestion: None,
-            confidence: 0.8,
-            code_suggestion: None,
-            tags: vec![],
-            fix_effort: core::comment::FixEffort::Low,
-            feedback: None,
-        };
-
-        let comments = vec![comment];
-
-        // Accept the same batch of comments twice
-        for _ in 0..2 {
-            apply_feedback_accept(&mut store, &comments);
-        }
-
-        let key = review::classify_comment_type(&comments[0])
-            .as_str()
-            .to_string();
-        let stats = &store.by_comment_type[&key];
-        assert_eq!(
-            stats.accepted, 1,
-            "Stats should only count 1 acceptance, not 2 (double-counting bug)"
-        );
-    }
-}
-
 fn load_discussion_thread(path: Option<&std::path::Path>, comment_id: &str) -> DiscussionThread {
     let Some(path) = path else {
         return DiscussionThread {
@@ -567,3 +497,73 @@ async fn answer_discussion_question(
     let response = adapter.complete(request).await?;
     Ok(response.content)
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_select_discussion_comment_empty_comments() {
+        // Should return an error, not panic
+        let result = select_discussion_comment(&[], None, None);
+        assert!(result.is_err());
+    }
+
+    #[test]
+    fn test_select_discussion_comment_defaults_to_first() {
+        let comment = core::Comment {
+            id: "cmt_1".to_string(),
+            file_path: PathBuf::from("test.rs"),
+            line_number: 1,
+            content: "test".to_string(),
+            rule_id: None,
+            severity: core::comment::Severity::Info,
+            category: core::comment::Category::BestPractice,
+            suggestion: None,
+            confidence: 0.8,
+            code_suggestion: None,
+            tags: vec![],
+            fix_effort: core::comment::FixEffort::Low,
+            feedback: None,
+        };
+        let result = select_discussion_comment(std::slice::from_ref(&comment), None, None).unwrap();
+        assert_eq!(result.id, "cmt_1");
+    }
+
+    #[test]
+    fn test_feedback_stats_not_double_counted() {
+        // Simulate accepting the same comment twice — stats should only increment once
+        let mut store = review::FeedbackStore::default();
+        let comment = core::Comment {
+            id: "cmt_dup".to_string(),
+            file_path: PathBuf::from("test.rs"),
+            line_number: 1,
+            content: "test".to_string(),
+            rule_id: None,
+            severity: core::comment::Severity::Warning,
+            category: core::comment::Category::Bug,
+            suggestion: None,
+            confidence: 0.8,
+            code_suggestion: None,
+            tags: vec![],
+            fix_effort: core::comment::FixEffort::Low,
+            feedback: None,
+        };
+
+        let comments = vec![comment];
+
+        // Accept the same batch of comments twice
+        for _ in 0..2 {
+            apply_feedback_accept(&mut store, &comments);
+        }
+
+        let key = review::classify_comment_type(&comments[0])
+            .as_str()
+            .to_string();
+        let stats = &store.by_comment_type[&key];
+        assert_eq!(
+            stats.accepted, 1,
+            "Stats should only count 1 acceptance, not 2 (double-counting bug)"
+        );
+    }
+}

src/core/context.rs

@@ -288,6 +288,16 @@ fn truncate_with_notice(mut content: String, max_chars: usize) -> String {
     content
 }
 
+async fn read_file_lossy(path: &Path) -> Result<String> {
+    match tokio::fs::read_to_string(path).await {
+        Ok(content) => Ok(content),
+        Err(_) => {
+            let bytes = tokio::fs::read(path).await?;
+            Ok(String::from_utf8_lossy(&bytes).to_string())
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -304,7 +314,7 @@ mod tests {
         assert!(result.contains("[Truncated]"));
         // Verify the result is valid UTF-8 (it is since it's a String, but
         // the point is truncate() would have panicked)
-        assert!(result.len() > 0);
+        assert!(!result.is_empty());
     }
 
     #[test]
@@ -364,13 +374,3 @@ mod tests {
         assert!(chunk.content.contains("pub fn process"));
     }
 }
-
-async fn read_file_lossy(path: &Path) -> Result<String> {
-    match tokio::fs::read_to_string(path).await {
-        Ok(content) => Ok(content),
-        Err(_) => {
-            let bytes = tokio::fs::read(path).await?;
-            Ok(String::from_utf8_lossy(&bytes).to_string())
-        }
-    }
-}

src/core/interactive.rs

@@ -246,6 +246,42 @@ Interactive commands respect these configurations."#
     }
 }
 
+/// Manages per-session ignore patterns from @diffscope ignore commands.
+/// Will be wired into the review pipeline's triage filter.
+#[allow(dead_code)]
+pub struct InteractiveProcessor {
+    ignored_patterns: HashSet<String>,
+}
+
+#[allow(dead_code)]
+impl InteractiveProcessor {
+    pub fn new() -> Self {
+        Self {
+            ignored_patterns: HashSet::new(),
+        }
+    }
+
+    pub fn add_ignore_pattern(&mut self, pattern: &str) {
+        self.ignored_patterns.insert(pattern.to_string());
+    }
+
+    pub fn should_ignore(&self, path: &str) -> bool {
+        self.ignored_patterns.iter().any(|pattern| {
+            // Simple glob matching
+            if pattern.contains('*') {
+                // Escape regex metacharacters, then convert glob * to .*
+                let escaped = regex::escape(pattern).replace(r"\*", ".*");
+                let regex_pattern = format!("^{}$", escaped);
+                regex::Regex::new(&regex_pattern)
+                    .map(|re| re.is_match(path))
+                    .unwrap_or(false)
+            } else {
+                path.contains(pattern)
+            }
+        })
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -461,39 +497,3 @@ mod tests {
         );
     }
 }
-
-/// Manages per-session ignore patterns from @diffscope ignore commands.
-/// Will be wired into the review pipeline's triage filter.
-#[allow(dead_code)]
-pub struct InteractiveProcessor {
-    ignored_patterns: HashSet<String>,
-}
-
-#[allow(dead_code)]
-impl InteractiveProcessor {
-    pub fn new() -> Self {
-        Self {
-            ignored_patterns: HashSet::new(),
-        }
-    }
-
-    pub fn add_ignore_pattern(&mut self, pattern: &str) {
-        self.ignored_patterns.insert(pattern.to_string());
-    }
-
-    pub fn should_ignore(&self, path: &str) -> bool {
-        self.ignored_patterns.iter().any(|pattern| {
-            // Simple glob matching
-            if pattern.contains('*') {
-                // Escape regex metacharacters, then convert glob * to .*
-                let escaped = regex::escape(pattern).replace(r"\*", ".*");
-                let regex_pattern = format!("^{}$", escaped);
-                regex::Regex::new(&regex_pattern)
-                    .map(|re| re.is_match(path))
-                    .unwrap_or(false)
-            } else {
-                path.contains(pattern)
-            }
-        })
-    }
-}

src/parsing/smart_response.rs

@@ -173,7 +173,7 @@ pub fn parse_smart_confidence(value: &str) -> Option<f32> {
     let has_percent = raw.ends_with('%');
     let trimmed = raw.trim_end_matches('%');
     if let Ok(num) = trimmed.parse::<f32>() {
-        if !has_percent && num >= 0.0 && num <= 1.0 {
+        if !has_percent && (0.0..=1.0).contains(&num) {
             // Bare decimal in 0..1 range — treat as already-normalized confidence
             Some(num.clamp(0.0, 1.0))
         } else {

src/review/compression.rs

@@ -846,7 +846,7 @@ mod tests {
                 )])],
             ),
         ];
-        let total = diffs.iter().map(|d| estimate_diff_tokens(d)).sum::<usize>();
+        let total = diffs.iter().map(estimate_diff_tokens).sum::<usize>();
         // Budget smaller than total forces Stage 2, where deletion-only hunks get removed
         let result = compress_diffs(&diffs, total / 2, 5);
         // All files should be skipped since they're deletion-only after compression

src/review/verification.rs

@@ -462,7 +462,7 @@ mod tests {
         // Both should be captured (first one wins in filter since find() returns first)
         let c1_results: Vec<_> = results.iter().filter(|r| r.comment_id == "c1").collect();
         assert!(
-            c1_results.len() >= 1,
+            !c1_results.is_empty(),
             "Should have at least one result for c1"
         );
     }