Skip to content

Commit 7ebc62e

Browse files
committed
feat(feedback): track dismissed findings separately
Record lifecycle dismissals in the feedback store without mixing them into rejection stats, and update TODO.md for verified shipped work and the new dismissal split.
1 parent 45e7c51 commit 7ebc62e

File tree

7 files changed

+132
-14
lines changed

7 files changed

+132
-14
lines changed

TODO.md

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ This roadmap is derived from deep research into Greptile's public docs, blog, MC
2525
1. [ ] Add first-class comment outcome states beyond thumbs: `new`, `accepted`, `rejected`, `addressed`, `stale`, `auto_fixed`.
2626
2. [ ] Infer "addressed by later commit" by diffing follow-up pushes against the original commented lines.
2727
3. [ ] Feed addressed/not-addressed outcomes into the reinforcement store alongside thumbs.
28-
4. [ ] Separate false-positive rejections from "valid but won't fix" dismissals in stored feedback.
28+
4. [x] Separate false-positive rejections from "valid but won't fix" dismissals in stored feedback.
2929
5. [ ] Weight reinforcement by reviewer role or trust level when GitHub identity is available.
3030
6. [ ] Add rule-level reinforcement decay so old team preferences do not dominate forever.
3131
7. [x] Add path-scoped reinforcement buckets so teams can prefer different standards in `tests/`, `scripts/`, and production code.
@@ -50,26 +50,26 @@ This roadmap is derived from deep research into Greptile's public docs, blog, MC
5050

5151
21. [ ] Build a first-class `fix until clean` loop that can run review, apply fixes, rerun review, and stop on convergence.
5252
22. [ ] Reuse the existing DAG runtime to model iterative review/fix loops as resumable workflow nodes.
53-
23. [ ] Add a max-iteration policy and loop budget controls for autonomous review convergence.
53+
23. [x] Add a max-iteration policy and loop budget controls for autonomous review convergence.
5454
24. [ ] Add "issue replay" prompts that hand unresolved findings back to a coding agent with file-local context.
5555
25. [ ] Add a handoff contract from reviewer findings to fix agents with rule IDs, evidence, and suggested diffs.
5656
26. [ ] Persist loop-level telemetry: iterations, fixes attempted, findings cleared, findings reopened.
57-
27. [ ] Add "challenge the finding" verification loops where a validator tries to falsify a suspected issue before keeping it.
57+
27. [x] Add "challenge the finding" verification loops where a validator tries to falsify a suspected issue before keeping it.
5858
28. [ ] Add caching between iterations so repeated codebase retrieval and verification runs are cheaper.
5959
29. [ ] Allow loop policies to differ by profile: conservative auditor, high-autonomy fixer, or report-only.
6060
30. [ ] Add eval fixtures specifically for loop convergence and reopened-issue regressions.
6161

6262
## 4. Code Graph and Repository Intelligence
6363

6464
31. [ ] Turn the current symbol graph into a persisted repository graph with durable storage and reload support.
65-
32. [ ] Add caller/callee expansion APIs for multi-hop impact analysis from changed symbols.
65+
32. [x] Add caller/callee expansion APIs for multi-hop impact analysis from changed symbols.
6666
33. [ ] Add contract edges between interfaces, implementations, and API endpoints.
6767
34. [ ] Add "similar implementation" lookup so repeated patterns and divergences are explicit.
6868
35. [ ] Add cross-file blast-radius summaries to findings when a change affects many callers.
6969
36. [ ] Add graph freshness/version metadata so reviews know whether they are using stale repository intelligence.
7070
37. [ ] Add graph-backed ranking of related files before semantic RAG retrieval.
7171
38. [ ] Add graph query traces to `dag_traces` or review artifacts for explainability and debugging.
72-
39. [ ] Add graph-aware eval fixtures that require multi-hop code understanding to pass.
72+
39. [x] Add graph-aware eval fixtures that require multi-hop code understanding to pass.
7373
40. [ ] Split `src/core/symbol_graph.rs` into construction, persistence, traversal, and ranking modules as it grows.
7474

7575
## 5. External Context and Pattern Repositories
@@ -107,8 +107,8 @@ This roadmap is derived from deep research into Greptile's public docs, blog, MC
107107
65. [x] Add review completeness and mean-time-to-resolution charts.
108108
66. [ ] Add feedback-learning effectiveness metrics: did reranked findings get higher acceptance after rollout?
109109
67. [ ] Add pattern-repository utilization analytics showing when extra context actually affected findings.
110-
68. [ ] Add eval-vs-production dashboards comparing benchmark strength against real-world acceptance.
111-
69. [ ] Add drill-downs from trend charts directly into the affected reviews, findings, and rules.
110+
68. [x] Add eval-vs-production dashboards comparing benchmark strength against real-world acceptance.
111+
69. [x] Add drill-downs from trend charts directly into the affected reviews, findings, and rules.
112112
70. [x] Add exportable JSON/CSV reports for review quality, lifecycle, and reinforcement metrics.
113113

114114
## 8. APIs, Automation, and MCP-Like Surfaces
@@ -130,7 +130,7 @@ This roadmap is derived from deep research into Greptile's public docs, blog, MC
130130
82. [ ] Split `src/server/state.rs` into session lifecycle, persistence, progress, and GitHub coordination modules.
131131
83. [ ] Add queue depth and worker saturation metrics for long-running review and eval jobs.
132132
84. [ ] Add retention policies for review artifacts, eval artifacts, and trend histories.
133-
85. [ ] Add storage migrations for richer comment lifecycle and reinforcement schemas.
133+
85. [x] Add storage migrations for richer comment lifecycle and reinforcement schemas.
134134
86. [ ] Add deployment docs for self-hosted review + analytics + trend retention setups.
135135
87. [ ] Add secret-management guidance and validation for multi-provider enterprise installs.
136136
88. [ ] Add background jobs for recomputing analytics after schema or scoring changes.
@@ -180,4 +180,4 @@ This roadmap is derived from deep research into Greptile's public docs, blog, MC
180180
- [x] Add path-scoped reinforcement buckets so feedback can distinguish `tests/**`, `scripts/**`, and broader code areas.
181181
- [x] Add review completeness metrics across summaries, PR readiness, and ReviewView surfaces.
182182
- [x] Add per-PR readiness timelines that preserve historical mergeability checkpoints.
183-
- [ ] Commit and push each validated checkpoint before moving to the next epic.
183+
- [x] Commit and push each validated checkpoint before moving to the next epic.

src/review/feedback.rs

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@ pub use patterns::derive_file_patterns;
1818
#[allow(unused_imports)]
1919
pub use persistence::{load_feedback_store, load_feedback_store_from_path, save_feedback_store};
2020
#[allow(unused_imports)]
21-
pub use record::{apply_comment_feedback_signal, record_comment_feedback_stats};
21+
pub use record::{
22+
apply_comment_dismissal_signal, apply_comment_feedback_signal, record_comment_dismissal_stats,
23+
record_comment_feedback_stats,
24+
};
2225
#[allow(unused_imports)]
2326
pub use semantic::{record_semantic_feedback_example, record_semantic_feedback_examples};
2427
#[allow(unused_imports)]
@@ -34,6 +37,7 @@ mod tests {
3437
let store = FeedbackStore::default();
3538
assert!(store.suppress.is_empty());
3639
assert!(store.accept.is_empty());
40+
assert!(store.dismissed.is_empty());
3741
assert!(store.by_comment_type.is_empty());
3842
assert!(store.by_category.is_empty());
3943
assert!(store.by_file_pattern.is_empty());
@@ -62,20 +66,24 @@ mod tests {
6266
let mut store = FeedbackStore::default();
6367
store.suppress.insert("c1".to_string());
6468
store.accept.insert("c2".to_string());
69+
store.dismissed.insert("c3".to_string());
6570
store.by_comment_type.insert(
6671
"style".to_string(),
6772
FeedbackTypeStats {
6873
accepted: 1,
6974
rejected: 2,
75+
dismissed: 3,
7076
},
7177
);
7278

7379
let json = serde_json::to_string(&store).unwrap();
7480
let deserialized: FeedbackStore = serde_json::from_str(&json).unwrap();
7581
assert!(deserialized.suppress.contains("c1"));
7682
assert!(deserialized.accept.contains("c2"));
83+
assert!(deserialized.dismissed.contains("c3"));
7784
assert_eq!(deserialized.by_comment_type["style"].accepted, 1);
7885
assert_eq!(deserialized.by_comment_type["style"].rejected, 2);
86+
assert_eq!(deserialized.by_comment_type["style"].dismissed, 3);
7987
}
8088

8189
#[test]
@@ -98,6 +106,7 @@ mod tests {
98106
let stats = FeedbackPatternStats {
99107
accepted: 10,
100108
rejected: 0,
109+
dismissed: 0,
101110
};
102111
assert_eq!(stats.acceptance_rate(), 1.0);
103112
assert_eq!(stats.total(), 10);
@@ -108,6 +117,7 @@ mod tests {
108117
let stats = FeedbackPatternStats {
109118
accepted: 0,
110119
rejected: 10,
120+
dismissed: 0,
111121
};
112122
assert_eq!(stats.acceptance_rate(), 0.0);
113123
}
@@ -117,6 +127,7 @@ mod tests {
117127
let stats = FeedbackPatternStats {
118128
accepted: 3,
119129
rejected: 7,
130+
dismissed: 0,
120131
};
121132
assert!((stats.acceptance_rate() - 0.3).abs() < f32::EPSILON);
122133
}

src/review/feedback/record.rs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,18 @@ pub fn record_comment_feedback_stats(
2525
}
2626
}
2727

28+
pub fn record_comment_dismissal_stats(store: &mut FeedbackStore, comment: &core::Comment) {
29+
let key = classify_comment_type(comment).as_str().to_string();
30+
let stats = store.by_comment_type.entry(key).or_default();
31+
stats.dismissed = stats.dismissed.saturating_add(1);
32+
33+
let file_patterns = derive_file_patterns(&comment.file_path);
34+
store.record_dismissal_patterns(&comment.category.to_string(), &file_patterns);
35+
if let Some(rule_id) = normalize_rule_id(comment.rule_id.as_deref()) {
36+
store.record_rule_dismissal_patterns(&rule_id, &file_patterns);
37+
}
38+
}
39+
2840
pub fn apply_comment_feedback_signal(
2941
store: &mut FeedbackStore,
3042
comment: &core::Comment,
@@ -45,6 +57,16 @@ pub fn apply_comment_feedback_signal(
4557
changed
4658
}
4759

60+
pub fn apply_comment_dismissal_signal(store: &mut FeedbackStore, comment: &core::Comment) -> bool {
61+
let changed = store.dismissed.insert(comment.id.clone());
62+
63+
if changed {
64+
record_comment_dismissal_stats(store, comment);
65+
}
66+
67+
changed
68+
}
69+
4870
#[cfg(test)]
4971
mod tests {
5072
use std::path::PathBuf;
@@ -113,4 +135,23 @@ mod tests {
113135
assert_eq!(store.by_rule["sec.sql.injection"].accepted, 1);
114136
assert_eq!(store.by_rule["sec.sql.injection"].rejected, 1);
115137
}
138+
139+
#[test]
140+
fn apply_comment_dismissal_signal_is_idempotent() {
141+
let comment = sample_comment();
142+
let mut store = FeedbackStore::default();
143+
144+
assert!(apply_comment_dismissal_signal(&mut store, &comment));
145+
assert!(!apply_comment_dismissal_signal(&mut store, &comment));
146+
147+
assert!(store.dismissed.contains(&comment.id));
148+
assert_eq!(store.by_comment_type["logic"].dismissed, 1);
149+
assert_eq!(store.by_category["Security"].dismissed, 1);
150+
assert_eq!(store.by_file_pattern["src/**"].dismissed, 1);
151+
assert_eq!(store.by_rule["sec.sql.injection"].dismissed, 1);
152+
assert_eq!(
153+
store.by_rule_file_pattern["sec.sql.injection|*.rs"].dismissed,
154+
1
155+
);
156+
}
116157
}

src/review/feedback/store.rs

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ pub struct FeedbackTypeStats {
88
pub accepted: usize,
99
#[serde(default)]
1010
pub rejected: usize,
11+
#[serde(default)]
12+
pub dismissed: usize,
1113
}
1214

1315
/// Tracks acceptance/rejection counts for a specific pattern (category, file extension, etc.)
@@ -17,6 +19,8 @@ pub struct FeedbackPatternStats {
1719
pub accepted: usize,
1820
#[serde(default)]
1921
pub rejected: usize,
22+
#[serde(default)]
23+
pub dismissed: usize,
2024
}
2125

2226
impl FeedbackPatternStats {
@@ -40,6 +44,8 @@ pub struct FeedbackStore {
4044
#[serde(default)]
4145
pub accept: HashSet<String>,
4246
#[serde(default)]
47+
pub dismissed: HashSet<String>,
48+
#[serde(default)]
4349
pub by_comment_type: HashMap<String, FeedbackTypeStats>,
4450
/// Feedback stats keyed by category (e.g., "Bug", "Security", "Performance").
4551
#[serde(default)]
@@ -122,6 +128,43 @@ impl FeedbackStore {
122128
update_pattern_stats(comp_stats, accepted);
123129
}
124130
}
131+
132+
/// Record a dismissal event across one or more file-pattern buckets.
133+
pub fn record_dismissal_patterns<S>(&mut self, category: &str, file_patterns: &[S])
134+
where
135+
S: AsRef<str>,
136+
{
137+
let cat_stats = self.by_category.entry(category.to_string()).or_default();
138+
update_pattern_dismissed(cat_stats);
139+
140+
for pattern in collect_unique_patterns(file_patterns) {
141+
let fp_stats = self.by_file_pattern.entry(pattern.clone()).or_default();
142+
update_pattern_dismissed(fp_stats);
143+
144+
let composite = format!("{category}|{pattern}");
145+
let comp_stats = self.by_category_file_pattern.entry(composite).or_default();
146+
update_pattern_dismissed(comp_stats);
147+
}
148+
}
149+
150+
/// Record a dismissal event across normalized rule-id buckets.
151+
pub fn record_rule_dismissal_patterns<S>(&mut self, rule_id: &str, file_patterns: &[S])
152+
where
153+
S: AsRef<str>,
154+
{
155+
let Some(rule_id) = normalize_feedback_key(rule_id) else {
156+
return;
157+
};
158+
159+
let rule_stats = self.by_rule.entry(rule_id.clone()).or_default();
160+
update_pattern_dismissed(rule_stats);
161+
162+
for pattern in collect_unique_patterns(file_patterns) {
163+
let composite = format!("{}|{}", rule_id, pattern);
164+
let comp_stats = self.by_rule_file_pattern.entry(composite).or_default();
165+
update_pattern_dismissed(comp_stats);
166+
}
167+
}
125168
}
126169

127170
fn update_pattern_stats(stats: &mut FeedbackPatternStats, accepted: bool) {
@@ -132,6 +175,10 @@ fn update_pattern_stats(stats: &mut FeedbackPatternStats, accepted: bool) {
132175
}
133176
}
134177

178+
fn update_pattern_dismissed(stats: &mut FeedbackPatternStats) {
179+
stats.dismissed += 1;
180+
}
181+
135182
fn collect_unique_patterns<S>(file_patterns: &[S]) -> HashSet<String>
136183
where
137184
S: AsRef<str>,

src/review/filters.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ mod tests {
108108
super::super::feedback::FeedbackTypeStats {
109109
accepted: 0,
110110
rejected: 3,
111+
dismissed: 0,
111112
},
112113
)]),
113114
..Default::default()
@@ -144,6 +145,7 @@ mod tests {
144145
super::super::feedback::FeedbackTypeStats {
145146
accepted: 0,
146147
rejected: 10,
148+
dismissed: 0,
147149
},
148150
)]),
149151
..Default::default()
@@ -170,6 +172,7 @@ mod tests {
170172
super::super::feedback::FeedbackTypeStats {
171173
accepted: 0,
172174
rejected: 10,
175+
dismissed: 0,
173176
},
174177
)]),
175178
..Default::default()

src/review/mod.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,9 @@ pub use context_helpers::{
1313
resolve_pattern_repositories,
1414
};
1515
pub use feedback::{
16-
apply_comment_feedback_signal, derive_file_patterns, load_feedback_store,
17-
load_feedback_store_from_path, record_semantic_feedback_examples, save_feedback_store,
16+
apply_comment_dismissal_signal, apply_comment_feedback_signal, derive_file_patterns,
17+
load_feedback_store, load_feedback_store_from_path, record_semantic_feedback_examples,
18+
save_feedback_store,
1819
};
1920
#[allow(unused_imports)]
2021
pub use filters::apply_review_filters;

src/server/api.rs

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1278,14 +1278,21 @@ pub async fn update_comment_lifecycle(
12781278

12791279
let mut session = load_review_session_for_update(&state, &id).await?;
12801280

1281-
let status_changed = {
1281+
let (status_changed, dismissed_comment) = {
12821282
let comment = session
12831283
.comments
12841284
.iter_mut()
12851285
.find(|c| c.id == request.comment_id)
12861286
.ok_or(StatusCode::NOT_FOUND)?;
12871287

1288-
apply_comment_lifecycle_transition(comment, next_status, current_timestamp())
1288+
let changed = apply_comment_lifecycle_transition(comment, next_status, current_timestamp());
1289+
let dismissed_comment = if changed && next_status == CommentStatus::Dismissed {
1290+
Some(comment.clone())
1291+
} else {
1292+
None
1293+
};
1294+
1295+
(changed, dismissed_comment)
12891296
};
12901297

12911298
if status_changed {
@@ -1295,6 +1302,14 @@ pub async fn update_comment_lifecycle(
12951302
previous_summary.as_ref(),
12961303
));
12971304
persist_updated_review_session(&state, session).await;
1305+
1306+
if let Some(comment) = dismissed_comment {
1307+
let config = state.config.read().await.clone();
1308+
let mut feedback_store = crate::review::load_feedback_store(&config);
1309+
if crate::review::apply_comment_dismissal_signal(&mut feedback_store, &comment) {
1310+
let _ = crate::review::save_feedback_store(&config.feedback_path, &feedback_store);
1311+
}
1312+
}
12981313
}
12991314

13001315
Ok(Json(CommentLifecycleResponse { ok: true }))

0 commit comments

Comments
 (0)