Skip to content

Commit bbc49af

Browse files
committed
refactor: split feedback apply helpers
Separate acceptance and rejection mutation loops from feedback stats bookkeeping so store updates stay easier to reason about. Made-with: Cursor
1 parent 5d434de commit bbc49af

File tree

5 files changed

+121
-87
lines changed

5 files changed

+121
-87
lines changed

TODO.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@
8686
- [x] `src/commands/git/suggest.rs`: split commit-message prompting from PR-title prompting and response extraction.
8787
- [x] `src/commands/review/command.rs`: split review/check/compare entrypoints if they keep diverging.
8888
- [x] `src/commands/misc/feedback/command.rs`: separate file loading/ID normalization from store persistence.
89-
- [ ] `src/commands/misc/feedback/apply.rs`: split acceptance/rejection counters from store mutation helpers.
89+
- [x] `src/commands/misc/feedback/apply.rs`: split acceptance/rejection counters from store mutation helpers.
9090
- [ ] `src/commands/misc/discussion/command.rs`: separate the interactive loop from single-shot execution.
9191
- [ ] `src/commands/misc/discussion/selection.rs`: split file loading/ID repair from selection rules.
9292
- [ ] `src/commands/misc/changelog.rs`: evaluate splitting changelog collection from output formatting.
Lines changed: 9 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -1,86 +1,9 @@
1-
use crate::core;
2-
use crate::review;
3-
4-
pub(super) fn apply_feedback_accept(
5-
store: &mut review::FeedbackStore,
6-
comments: &[core::Comment],
7-
) -> usize {
8-
let mut updated = 0;
9-
for comment in comments {
10-
let is_new = store.accept.insert(comment.id.clone());
11-
if is_new {
12-
updated += 1;
13-
let key = review::classify_comment_type(comment).as_str().to_string();
14-
let stats = store.by_comment_type.entry(key).or_default();
15-
stats.accepted = stats.accepted.saturating_add(1);
16-
let file_patterns = review::derive_file_patterns(&comment.file_path);
17-
store.record_feedback_patterns(&comment.category.to_string(), &file_patterns, true);
18-
}
19-
store.suppress.remove(&comment.id);
20-
}
21-
updated
22-
}
23-
24-
pub(super) fn apply_feedback_reject(
25-
store: &mut review::FeedbackStore,
26-
comments: &[core::Comment],
27-
) -> usize {
28-
let mut updated = 0;
29-
for comment in comments {
30-
let is_new = store.suppress.insert(comment.id.clone());
31-
if is_new {
32-
updated += 1;
33-
let key = review::classify_comment_type(comment).as_str().to_string();
34-
let stats = store.by_comment_type.entry(key).or_default();
35-
stats.rejected = stats.rejected.saturating_add(1);
36-
let file_patterns = review::derive_file_patterns(&comment.file_path);
37-
store.record_feedback_patterns(&comment.category.to_string(), &file_patterns, false);
38-
}
39-
store.accept.remove(&comment.id);
40-
}
41-
updated
42-
}
43-
44-
#[cfg(test)]
45-
mod tests {
46-
use super::*;
47-
use std::path::PathBuf;
48-
49-
#[test]
50-
fn test_feedback_stats_not_double_counted() {
51-
let mut store = review::FeedbackStore::default();
52-
let comment = core::Comment {
53-
id: "cmt_dup".to_string(),
54-
file_path: PathBuf::from("test.rs"),
55-
line_number: 1,
56-
content: "test".to_string(),
57-
rule_id: None,
58-
severity: core::comment::Severity::Warning,
59-
category: core::comment::Category::Bug,
60-
suggestion: None,
61-
confidence: 0.8,
62-
code_suggestion: None,
63-
tags: vec![],
64-
fix_effort: core::comment::FixEffort::Low,
65-
feedback: None,
66-
};
67-
68-
let comments = vec![comment];
69-
70-
for _ in 0..2 {
71-
apply_feedback_accept(&mut store, &comments);
72-
}
73-
74-
let key = review::classify_comment_type(&comments[0])
75-
.as_str()
76-
.to_string();
77-
let stats = &store.by_comment_type[&key];
78-
assert_eq!(
79-
stats.accepted, 1,
80-
"Stats should only count 1 acceptance, not 2 (double-counting bug)"
81-
);
82-
assert_eq!(store.by_category["Bug"].accepted, 1);
83-
assert_eq!(store.by_file_pattern["*.rs"].accepted, 1);
84-
assert_eq!(store.by_category_file_pattern["Bug|*.rs"].accepted, 1);
85-
}
86-
}
1+
#[path = "apply/accept.rs"]
2+
mod accept;
3+
#[path = "apply/reject.rs"]
4+
mod reject;
5+
#[path = "apply/stats.rs"]
6+
mod stats;
7+
8+
pub(super) use accept::apply_feedback_accept;
9+
pub(super) use reject::apply_feedback_reject;
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
use crate::core;
2+
use crate::review;
3+
4+
use super::stats::record_accepted_feedback;
5+
6+
pub(in super::super) fn apply_feedback_accept(
7+
store: &mut review::FeedbackStore,
8+
comments: &[core::Comment],
9+
) -> usize {
10+
let mut updated = 0;
11+
for comment in comments {
12+
let is_new = store.accept.insert(comment.id.clone());
13+
if is_new {
14+
updated += 1;
15+
record_accepted_feedback(store, comment);
16+
}
17+
store.suppress.remove(&comment.id);
18+
}
19+
updated
20+
}
21+
22+
#[cfg(test)]
23+
mod tests {
24+
use super::*;
25+
use std::path::PathBuf;
26+
27+
#[test]
28+
fn test_feedback_stats_not_double_counted() {
29+
let mut store = review::FeedbackStore::default();
30+
let comment = core::Comment {
31+
id: "cmt_dup".to_string(),
32+
file_path: PathBuf::from("test.rs"),
33+
line_number: 1,
34+
content: "test".to_string(),
35+
rule_id: None,
36+
severity: core::comment::Severity::Warning,
37+
category: core::comment::Category::Bug,
38+
suggestion: None,
39+
confidence: 0.8,
40+
code_suggestion: None,
41+
tags: vec![],
42+
fix_effort: core::comment::FixEffort::Low,
43+
feedback: None,
44+
};
45+
46+
let comments = vec![comment];
47+
48+
for _ in 0..2 {
49+
apply_feedback_accept(&mut store, &comments);
50+
}
51+
52+
let key = review::classify_comment_type(&comments[0])
53+
.as_str()
54+
.to_string();
55+
let stats = &store.by_comment_type[&key];
56+
assert_eq!(
57+
stats.accepted, 1,
58+
"Stats should only count 1 acceptance, not 2 (double-counting bug)"
59+
);
60+
assert_eq!(store.by_category["Bug"].accepted, 1);
61+
assert_eq!(store.by_file_pattern["*.rs"].accepted, 1);
62+
assert_eq!(store.by_category_file_pattern["Bug|*.rs"].accepted, 1);
63+
}
64+
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
use crate::core;
2+
use crate::review;
3+
4+
use super::stats::record_rejected_feedback;
5+
6+
pub(in super::super) fn apply_feedback_reject(
7+
store: &mut review::FeedbackStore,
8+
comments: &[core::Comment],
9+
) -> usize {
10+
let mut updated = 0;
11+
for comment in comments {
12+
let is_new = store.suppress.insert(comment.id.clone());
13+
if is_new {
14+
updated += 1;
15+
record_rejected_feedback(store, comment);
16+
}
17+
store.accept.remove(&comment.id);
18+
}
19+
updated
20+
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
use crate::core;
2+
use crate::review;
3+
4+
pub(super) fn record_accepted_feedback(store: &mut review::FeedbackStore, comment: &core::Comment) {
5+
record_feedback_stats(store, comment, true);
6+
}
7+
8+
pub(super) fn record_rejected_feedback(store: &mut review::FeedbackStore, comment: &core::Comment) {
9+
record_feedback_stats(store, comment, false);
10+
}
11+
12+
fn record_feedback_stats(
13+
store: &mut review::FeedbackStore,
14+
comment: &core::Comment,
15+
accepted: bool,
16+
) {
17+
let key = review::classify_comment_type(comment).as_str().to_string();
18+
let stats = store.by_comment_type.entry(key).or_default();
19+
if accepted {
20+
stats.accepted = stats.accepted.saturating_add(1);
21+
} else {
22+
stats.rejected = stats.rejected.saturating_add(1);
23+
}
24+
25+
let file_patterns = review::derive_file_patterns(&comment.file_path);
26+
store.record_feedback_patterns(&comment.category.to_string(), &file_patterns, accepted);
27+
}

0 commit comments

Comments
 (0)