Skip to content

Commit e5236ae

Browse files
committed
feat(feedback): decay stale rule reinforcement
1 parent d250617 commit e5236ae

8 files changed

Lines changed: 417 additions & 24 deletions

File tree

TODO.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ This roadmap is derived from deep research into Greptile's public docs, blog, MC
2727
3. [x] Feed addressed/not-addressed outcomes into the reinforcement store alongside thumbs.
2828
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.
30-
6. [ ] Add rule-level reinforcement decay so old team preferences do not dominate forever.
30+
6. [x] 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.
3232
8. [ ] Persist explanation text from follow-up feedback replies and mine it into reusable review guidance.
3333
9. [ ] Learn "preferred phrasing" for accepted comments so comment tone and specificity improve over time.

src/commands/misc/feedback/backfill.rs

Lines changed: 61 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,12 @@ fn build_feedback_store_from_reviews(
4141
};
4242

4343
for session in &sessions {
44+
let timestamp = feedback_event_timestamp(session);
4445
for comment in &session.comments {
4546
if let Some(accepted) = normalize_feedback_label(comment.feedback.as_deref()) {
46-
if review::apply_comment_feedback_signal(&mut store, comment, accepted) {
47+
if review::apply_comment_feedback_signal_at(
48+
&mut store, comment, accepted, timestamp,
49+
) {
4750
if accepted {
4851
summary.accepted += 1;
4952
} else {
@@ -110,10 +113,11 @@ fn backfill_not_addressed_outcomes(
110113
.filter(|comment| comment.status == CommentStatus::Open)
111114
{
112115
if current_comment_ids.contains(comment.id.as_str())
113-
&& review::apply_comment_resolution_outcome_signal(
116+
&& review::apply_comment_resolution_outcome_signal_at(
114117
store,
115118
comment,
116119
review::CommentResolutionOutcome::NotAddressed,
120+
feedback_event_timestamp(current),
117121
)
118122
{
119123
not_addressed += 1;
@@ -163,6 +167,10 @@ fn normalize_feedback_label(label: Option<&str>) -> Option<bool> {
163167
}
164168
}
165169

170+
fn feedback_event_timestamp(session: &ReviewSession) -> i64 {
171+
session.completed_at.unwrap_or(session.started_at)
172+
}
173+
166174
#[cfg(test)]
167175
mod tests {
168176
use super::*;
@@ -362,4 +370,55 @@ mod tests {
362370
serde_json::to_value(&list_store).unwrap()
363371
);
364372
}
373+
374+
#[test]
375+
fn build_feedback_store_replays_rule_decay_in_chronological_order() {
376+
let half_life = 30 * 24 * 60 * 60;
377+
378+
let stale_rejects = (0..32)
379+
.map(|index| {
380+
let mut comment = sample_comment(
381+
&format!("Stale reject {index}"),
382+
Category::Security,
383+
"src/lib.rs",
384+
);
385+
comment.feedback = Some("reject".to_string());
386+
comment.rule_id = Some("SEC.SQL.INJECTION".to_string());
387+
comment.id = format!("stale-reject-{index}");
388+
comment
389+
})
390+
.collect::<Vec<_>>();
391+
let recent_accepts = (0..4)
392+
.map(|index| {
393+
let mut comment = sample_comment(
394+
&format!("Recent accept {index}"),
395+
Category::Security,
396+
"src/lib.rs",
397+
);
398+
comment.feedback = Some("accept".to_string());
399+
comment.rule_id = Some("SEC.SQL.INJECTION".to_string());
400+
comment.id = format!("recent-accept-{index}");
401+
comment
402+
})
403+
.collect::<Vec<_>>();
404+
405+
let (store, _) = build_feedback_store_from_reviews(vec![
406+
sample_review_session("stale", 1_000, "raw", None, stale_rejects),
407+
sample_review_session(
408+
"recent",
409+
1_000 + (4 * half_life),
410+
"raw",
411+
None,
412+
recent_accepts,
413+
),
414+
]);
415+
416+
let stats = &store.by_rule["sec.sql.injection"];
417+
let recent_timestamp = 1_000 + (4 * half_life) + 1;
418+
assert!(stats.acceptance_rate() < 0.2);
419+
assert!(
420+
stats.decayed_acceptance_rate_at(recent_timestamp).unwrap() > 0.6,
421+
"expected recent accepts to outweigh stale rejects after replay"
422+
);
423+
}
365424
}

src/review/feedback.rs

Lines changed: 62 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,15 @@ pub use persistence::{load_feedback_store, load_feedback_store_from_path, save_f
2020
#[allow(unused_imports)]
2121
pub use record::{
2222
apply_comment_dismissal_signal, apply_comment_feedback_signal,
23-
apply_comment_resolution_outcome_signal, record_comment_dismissal_stats,
24-
record_comment_feedback_stats, record_comment_resolution_stats, CommentResolutionOutcome,
23+
apply_comment_feedback_signal_at, apply_comment_resolution_outcome_signal,
24+
apply_comment_resolution_outcome_signal_at, record_comment_dismissal_stats,
25+
record_comment_feedback_stats, record_comment_feedback_stats_at,
26+
record_comment_resolution_stats, record_comment_resolution_stats_at, CommentResolutionOutcome,
2527
};
2628
#[allow(unused_imports)]
2729
pub use semantic::{record_semantic_feedback_example, record_semantic_feedback_examples};
2830
#[allow(unused_imports)]
29-
pub use store::{FeedbackPatternStats, FeedbackStore, FeedbackTypeStats};
31+
pub use store::{DecayedFeedbackStats, FeedbackPatternStats, FeedbackStore, FeedbackTypeStats};
3032

3133
#[cfg(test)]
3234
mod tests {
@@ -72,6 +74,21 @@ mod tests {
7274
store.dismissed.insert("c3".to_string());
7375
store.addressed.insert("c4".to_string());
7476
store.not_addressed.insert("c5".to_string());
77+
store.by_rule.insert(
78+
"style.rule".to_string(),
79+
FeedbackPatternStats {
80+
accepted: 1,
81+
rejected: 2,
82+
dismissed: 0,
83+
addressed: 0,
84+
not_addressed: 0,
85+
decayed: Some(DecayedFeedbackStats {
86+
positive: 0.75,
87+
negative: 1.25,
88+
last_event_at: Some(123),
89+
}),
90+
},
91+
);
7592
store.by_comment_type.insert(
7693
"style".to_string(),
7794
FeedbackTypeStats {
@@ -95,6 +112,10 @@ mod tests {
95112
assert_eq!(deserialized.by_comment_type["style"].dismissed, 3);
96113
assert_eq!(deserialized.by_comment_type["style"].addressed, 4);
97114
assert_eq!(deserialized.by_comment_type["style"].not_addressed, 5);
115+
assert_eq!(
116+
deserialized.by_rule["style.rule"].decayed_total_at(123),
117+
Some(2.0)
118+
);
98119
}
99120

100121
#[test]
@@ -120,6 +141,7 @@ mod tests {
120141
dismissed: 0,
121142
addressed: 0,
122143
not_addressed: 0,
144+
decayed: None,
123145
};
124146
assert_eq!(stats.acceptance_rate(), 1.0);
125147
assert_eq!(stats.total(), 10);
@@ -133,6 +155,7 @@ mod tests {
133155
dismissed: 0,
134156
addressed: 0,
135157
not_addressed: 0,
158+
decayed: None,
136159
};
137160
assert_eq!(stats.acceptance_rate(), 0.0);
138161
}
@@ -145,6 +168,7 @@ mod tests {
145168
dismissed: 0,
146169
addressed: 0,
147170
not_addressed: 0,
171+
decayed: None,
148172
};
149173
assert!((stats.acceptance_rate() - 0.3).abs() < f32::EPSILON);
150174
}
@@ -157,12 +181,47 @@ mod tests {
157181
dismissed: 0,
158182
addressed: 3,
159183
not_addressed: 1,
184+
decayed: None,
160185
};
161186

162187
assert!((stats.acceptance_rate() - (4.0 / 6.0)).abs() < f32::EPSILON);
163188
assert_eq!(stats.total(), 6);
164189
}
165190

191+
#[test]
192+
fn rule_feedback_decay_prioritizes_recent_signals() {
193+
let half_life = 30 * 24 * 60 * 60;
194+
let mut store = FeedbackStore::default();
195+
196+
for _ in 0..32 {
197+
store.record_rule_feedback_patterns_at("sec.sql.injection", &["*.rs"], false, 1_000);
198+
}
199+
for _ in 0..4 {
200+
store.record_rule_feedback_patterns_at(
201+
"sec.sql.injection",
202+
&["*.rs"],
203+
true,
204+
1_000 + (4 * half_life),
205+
);
206+
}
207+
208+
let stats = &store.by_rule["sec.sql.injection"];
209+
let decayed_total = stats.decayed_total_at(1_000 + (4 * half_life)).unwrap();
210+
let decayed_rate = stats
211+
.decayed_acceptance_rate_at(1_000 + (4 * half_life))
212+
.unwrap();
213+
214+
assert!(
215+
decayed_total >= 5.0,
216+
"expected enough fresh signal, got {decayed_total}"
217+
);
218+
assert!(
219+
decayed_rate > 0.6,
220+
"expected recent accepts to outweigh stale rejects, got {decayed_rate}"
221+
);
222+
assert!(stats.acceptance_rate() < 0.2);
223+
}
224+
166225
// ── record_feedback tests ─────────────────────────────────────────────
167226

168227
#[test]

src/review/feedback/record.rs

Lines changed: 93 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,27 @@ pub fn record_comment_feedback_stats(
3131
}
3232
}
3333

34+
pub fn record_comment_feedback_stats_at(
35+
store: &mut FeedbackStore,
36+
comment: &core::Comment,
37+
accepted: bool,
38+
timestamp: i64,
39+
) {
40+
let key = classify_comment_type(comment).as_str().to_string();
41+
let stats = store.by_comment_type.entry(key).or_default();
42+
if accepted {
43+
stats.accepted = stats.accepted.saturating_add(1);
44+
} else {
45+
stats.rejected = stats.rejected.saturating_add(1);
46+
}
47+
48+
let file_patterns = derive_file_patterns(&comment.file_path);
49+
store.record_feedback_patterns(&comment.category.to_string(), &file_patterns, accepted);
50+
if let Some(rule_id) = normalize_rule_id(comment.rule_id.as_deref()) {
51+
store.record_rule_feedback_patterns_at(&rule_id, &file_patterns, accepted, timestamp);
52+
}
53+
}
54+
3455
pub fn record_comment_dismissal_stats(store: &mut FeedbackStore, comment: &core::Comment) {
3556
let key = classify_comment_type(comment).as_str().to_string();
3657
let stats = store.by_comment_type.entry(key).or_default();
@@ -65,6 +86,29 @@ pub fn record_comment_resolution_stats(
6586
}
6687
}
6788

89+
pub fn record_comment_resolution_stats_at(
90+
store: &mut FeedbackStore,
91+
comment: &core::Comment,
92+
outcome: CommentResolutionOutcome,
93+
timestamp: i64,
94+
) {
95+
let key = classify_comment_type(comment).as_str().to_string();
96+
let stats = store.by_comment_type.entry(key).or_default();
97+
let addressed = matches!(outcome, CommentResolutionOutcome::Addressed);
98+
99+
if addressed {
100+
stats.addressed = stats.addressed.saturating_add(1);
101+
} else {
102+
stats.not_addressed = stats.not_addressed.saturating_add(1);
103+
}
104+
105+
let file_patterns = derive_file_patterns(&comment.file_path);
106+
store.record_outcome_patterns(&comment.category.to_string(), &file_patterns, addressed);
107+
if let Some(rule_id) = normalize_rule_id(comment.rule_id.as_deref()) {
108+
store.record_rule_outcome_patterns_at(&rule_id, &file_patterns, addressed, timestamp);
109+
}
110+
}
111+
68112
pub fn apply_comment_feedback_signal(
69113
store: &mut FeedbackStore,
70114
comment: &core::Comment,
@@ -85,6 +129,27 @@ pub fn apply_comment_feedback_signal(
85129
changed
86130
}
87131

132+
pub fn apply_comment_feedback_signal_at(
133+
store: &mut FeedbackStore,
134+
comment: &core::Comment,
135+
accepted: bool,
136+
timestamp: i64,
137+
) -> bool {
138+
let changed = if accepted {
139+
store.suppress.remove(&comment.id);
140+
store.accept.insert(comment.id.clone())
141+
} else {
142+
store.accept.remove(&comment.id);
143+
store.suppress.insert(comment.id.clone())
144+
};
145+
146+
if changed {
147+
record_comment_feedback_stats_at(store, comment, accepted, timestamp);
148+
}
149+
150+
changed
151+
}
152+
88153
pub fn apply_comment_dismissal_signal(store: &mut FeedbackStore, comment: &core::Comment) -> bool {
89154
let changed = store.dismissed.insert(comment.id.clone());
90155

@@ -112,6 +177,24 @@ pub fn apply_comment_resolution_outcome_signal(
112177
changed
113178
}
114179

180+
pub fn apply_comment_resolution_outcome_signal_at(
181+
store: &mut FeedbackStore,
182+
comment: &core::Comment,
183+
outcome: CommentResolutionOutcome,
184+
timestamp: i64,
185+
) -> bool {
186+
let changed = match outcome {
187+
CommentResolutionOutcome::Addressed => store.addressed.insert(comment.id.clone()),
188+
CommentResolutionOutcome::NotAddressed => store.not_addressed.insert(comment.id.clone()),
189+
};
190+
191+
if changed {
192+
record_comment_resolution_stats_at(store, comment, outcome, timestamp);
193+
}
194+
195+
changed
196+
}
197+
115198
#[cfg(test)]
116199
mod tests {
117200
use std::path::PathBuf;
@@ -143,8 +226,12 @@ mod tests {
143226
let comment = sample_comment();
144227
let mut store = FeedbackStore::default();
145228

146-
assert!(apply_comment_feedback_signal(&mut store, &comment, true));
147-
assert!(!apply_comment_feedback_signal(&mut store, &comment, true));
229+
assert!(apply_comment_feedback_signal_at(
230+
&mut store, &comment, true, 1_000
231+
));
232+
assert!(!apply_comment_feedback_signal_at(
233+
&mut store, &comment, true, 1_000,
234+
));
148235

149236
assert!(store.accept.contains(&comment.id));
150237
assert_eq!(store.by_category["Security"].accepted, 1);
@@ -162,6 +249,10 @@ mod tests {
162249
store.by_rule_file_pattern["sec.sql.injection|*.rs"].accepted,
163250
1
164251
);
252+
assert_eq!(
253+
store.by_rule["sec.sql.injection"].decayed_total_at(1_000),
254+
Some(1.0)
255+
);
165256
}
166257

167258
#[test]

0 commit comments

Comments
 (0)