refactor: split discussion selection helpers

Separate discussion comment loading and ID repair from selection rules so the CLI can evolve lookup behavior without mixing file parsing logic.

Made-with: Cursor

Author: haasonsaas

TODO.md

@@ -88,7 +88,7 @@
 - [x] `src/commands/misc/feedback/command.rs`: separate file loading/ID normalization from store persistence.
 - [x] `src/commands/misc/feedback/apply.rs`: split acceptance/rejection counters from store mutation helpers.
 - [x] `src/commands/misc/discussion/command.rs`: separate the interactive loop from single-shot execution.
-- [ ] `src/commands/misc/discussion/selection.rs`: split file loading/ID repair from selection rules.
+- [x] `src/commands/misc/discussion/selection.rs`: split file loading/ID repair from selection rules.
 - [ ] `src/commands/misc/changelog.rs`: evaluate splitting changelog collection from output formatting.
 - [x] `src/commands/eval/command.rs`: separate CLI option prep, fixture execution, and report lifecycle.
 - [x] `src/commands/feedback_eval/command.rs`: separate input loading from report/output orchestration.

src/commands/misc/discussion/selection.rs

@@ -1,92 +1,7 @@
-use anyhow::Result;
-use std::path::Path;
+#[path = "selection/load.rs"]
+mod load;
+#[path = "selection/rules.rs"]
+mod rules;
 
-use crate::core;
-
-pub(super) async fn load_discussion_comments(review_path: &Path) -> Result<Vec<core::Comment>> {
-    let content = tokio::fs::read_to_string(review_path).await?;
-    let mut comments: Vec<core::Comment> = serde_json::from_str(&content)?;
-    if comments.is_empty() {
-        anyhow::bail!("No comments found in {}", review_path.display());
-    }
-
-    for comment in &mut comments {
-        if comment.id.trim().is_empty() {
-            comment.id = core::comment::compute_comment_id(
-                &comment.file_path,
-                &comment.content,
-                &comment.category,
-            );
-        }
-    }
-
-    Ok(comments)
-}
-
-pub(super) fn select_discussion_comment(
-    comments: &[core::Comment],
-    comment_id: Option<String>,
-    comment_index: Option<usize>,
-) -> Result<core::Comment> {
-    if comment_id.is_some() && comment_index.is_some() {
-        anyhow::bail!("Specify only one of --comment-id or --comment-index");
-    }
-
-    if let Some(id) = comment_id {
-        let selected = comments
-            .iter()
-            .find(|comment| comment.id == id)
-            .cloned()
-            .ok_or_else(|| anyhow::anyhow!("Comment id not found: {}", id))?;
-        return Ok(selected);
-    }
-
-    if let Some(index) = comment_index {
-        if index == 0 {
-            anyhow::bail!("comment-index is 1-based");
-        }
-        let selected = comments
-            .get(index - 1)
-            .cloned()
-            .ok_or_else(|| anyhow::anyhow!("Comment index out of range: {}", index))?;
-        return Ok(selected);
-    }
-
-    comments
-        .first()
-        .cloned()
-        .ok_or_else(|| anyhow::anyhow!("No comments available"))
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-    use std::path::PathBuf;
-
-    #[test]
-    fn test_select_discussion_comment_empty_comments() {
-        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");
-    }
-}
+pub(super) use load::load_discussion_comments;
+pub(super) use rules::select_discussion_comment;

src/commands/misc/discussion/selection/load.rs

@@ -0,0 +1,29 @@
+use anyhow::Result;
+use std::path::Path;
+
+use crate::core;
+
+pub(in super::super) async fn load_discussion_comments(
+    review_path: &Path,
+) -> Result<Vec<core::Comment>> {
+    let content = tokio::fs::read_to_string(review_path).await?;
+    let mut comments: Vec<core::Comment> = serde_json::from_str(&content)?;
+    if comments.is_empty() {
+        anyhow::bail!("No comments found in {}", review_path.display());
+    }
+
+    normalize_comment_ids(&mut comments);
+    Ok(comments)
+}
+
+fn normalize_comment_ids(comments: &mut [core::Comment]) {
+    for comment in comments {
+        if comment.id.trim().is_empty() {
+            comment.id = core::comment::compute_comment_id(
+                &comment.file_path,
+                &comment.content,
+                &comment.category,
+            );
+        }
+    }
+}

src/commands/misc/discussion/selection/rules.rs

@@ -0,0 +1,71 @@
+use anyhow::Result;
+
+use crate::core;
+
+pub(in super::super) fn select_discussion_comment(
+    comments: &[core::Comment],
+    comment_id: Option<String>,
+    comment_index: Option<usize>,
+) -> Result<core::Comment> {
+    if comment_id.is_some() && comment_index.is_some() {
+        anyhow::bail!("Specify only one of --comment-id or --comment-index");
+    }
+
+    if let Some(id) = comment_id {
+        let selected = comments
+            .iter()
+            .find(|comment| comment.id == id)
+            .cloned()
+            .ok_or_else(|| anyhow::anyhow!("Comment id not found: {}", id))?;
+        return Ok(selected);
+    }
+
+    if let Some(index) = comment_index {
+        if index == 0 {
+            anyhow::bail!("comment-index is 1-based");
+        }
+        let selected = comments
+            .get(index - 1)
+            .cloned()
+            .ok_or_else(|| anyhow::anyhow!("Comment index out of range: {}", index))?;
+        return Ok(selected);
+    }
+
+    comments
+        .first()
+        .cloned()
+        .ok_or_else(|| anyhow::anyhow!("No comments available"))
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::path::PathBuf;
+
+    #[test]
+    fn test_select_discussion_comment_empty_comments() {
+        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");
+    }
+}