Skip to main content

diffler_core/
feedback.rs

1//! Markdown export of review feedback: comments with diff context, ready
2//! to paste into any agent prompt.
3
4// writing into a String is infallible, so the `write!` results are discarded
5use std::fmt::Write as _;
6
7use crate::model::{DiffLine, DiffModel};
8use crate::session::{Comment, CommentStatus, Session};
9
10pub struct FeedbackOptions<'a> {
11    /// Header text; the caller builds it (repo/branch/count need `HeadInfo`).
12    pub title: &'a str,
13    /// When set, only comments anchored to this file are exported.
14    pub file_filter: Option<&'a str>,
15    pub include_resolved: bool,
16}
17
18pub fn to_markdown(session: &Session, model: &DiffModel, opts: &FeedbackOptions<'_>) -> String {
19    let mut comments: Vec<&Comment> = session
20        .comments
21        .iter()
22        .filter(|c| opts.include_resolved || c.status != CommentStatus::Resolved)
23        .filter(|c| opts.file_filter.is_none_or(|f| c.anchor.file == f))
24        .collect();
25    comments.sort_by(|a, b| (&a.anchor.file, a.anchor.line).cmp(&(&b.anchor.file, b.anchor.line)));
26
27    let mut out = String::new();
28    let _ = writeln!(out, "## {}", opts.title);
29    for comment in comments {
30        out.push('\n');
31        render_comment(&mut out, comment, model, opts.include_resolved);
32    }
33    out
34}
35
36fn render_comment(out: &mut String, comment: &Comment, model: &DiffModel, include_resolved: bool) {
37    let anchor = &comment.anchor;
38    let _ = match (anchor.line, anchor.line_end) {
39        (Some(line), Some(end)) => writeln!(out, "### {}:{line}-{end}", anchor.file),
40        (Some(line), None) => writeln!(out, "### {}:{line}", anchor.file),
41        _ => writeln!(out, "### {}", anchor.file),
42    };
43
44    if let Some(line) = anchor.line {
45        match context_snippet(model, &anchor.file, line, anchor.on_old_side) {
46            Some(snippet) => {
47                // a fence longer than any backtick run in the snippet, so
48                // diff content containing ``` can't break out of it
49                let longest_run = snippet
50                    .iter()
51                    .map(|(_, text)| longest_backtick_run(text))
52                    .max()
53                    .unwrap_or(0);
54                let fence = "`".repeat((longest_run + 1).max(3));
55                let _ = writeln!(out, "{fence}");
56                for (origin, text) in snippet {
57                    let _ = writeln!(out, "{origin}{text}");
58                }
59                let _ = writeln!(out, "{fence}");
60            }
61            None => out.push_str("_(outdated)_\n"),
62        }
63    } else if anchor.is_outdated(model) {
64        out.push_str("_(outdated)_\n");
65    }
66
67    for body_line in comment.body.lines() {
68        let _ = writeln!(out, "> {body_line}");
69    }
70    for reply in &comment.replies {
71        let mut lines = reply.body.lines();
72        if let Some(first) = lines.next() {
73            let _ = writeln!(out, "> > {}: {first}", reply.author);
74        }
75        for rest in lines {
76            let _ = writeln!(out, "> > {rest}");
77        }
78    }
79    if include_resolved && comment.status == CommentStatus::Resolved {
80        out.push_str("_(resolved)_\n");
81    }
82}
83
84/// The anchored line plus its immediate neighbors within the same hunk,
85/// each tagged with its diff origin char (' ', '-', '+'). `None` when the
86/// file or line has left the model (the comment is outdated). Shared by
87/// the markdown export and the MCP comment payloads.
88pub fn context_snippet(
89    model: &DiffModel,
90    file: &str,
91    line: u32,
92    on_old_side: bool,
93) -> Option<Vec<(char, String)>> {
94    let file = model.files.iter().find(|f| f.path == file)?;
95    for hunk in &file.hunks {
96        let Some(idx) = hunk
97            .lines
98            .iter()
99            .position(|l| line_no(l, on_old_side) == Some(line))
100        else {
101            continue;
102        };
103        let start = idx.saturating_sub(1);
104        let end = (idx + 2).min(hunk.lines.len());
105        let snippet = hunk
106            .lines
107            .get(start..end)?
108            .iter()
109            .map(|l| (l.kind.origin(), l.text.clone()))
110            .collect();
111        return Some(snippet);
112    }
113    None
114}
115
116fn line_no(line: &DiffLine, on_old_side: bool) -> Option<u32> {
117    if on_old_side {
118        line.old_no
119    } else {
120        line.new_no
121    }
122}
123
124fn longest_backtick_run(text: &str) -> usize {
125    let mut longest = 0;
126    let mut current = 0;
127    for c in text.chars() {
128        if c == '`' {
129            current += 1;
130            longest = longest.max(current);
131        } else {
132            current = 0;
133        }
134    }
135    longest
136}
137
138#[cfg(test)]
139mod tests {
140    use crate::model::{FileDiff, FileStatus, Hunk, HunkId, LineKind};
141    use crate::test_support::anchor;
142
143    use super::*;
144
145    fn diff_line(kind: LineKind, old_no: Option<u32>, new_no: Option<u32>, text: &str) -> DiffLine {
146        DiffLine::new(kind, old_no, new_no, text.to_owned())
147    }
148
149    /// One file, one hunk: context(1/1), deleted(2), added(2), context(3/3).
150    fn sample_model() -> DiffModel {
151        DiffModel {
152            files: vec![FileDiff {
153                path: "src/auth.py".into(),
154                old_path: None,
155                status: FileStatus::Modified,
156                binary: false,
157                old_text: None,
158                new_text: Some("one\nTWO\nthree\n".into()),
159                hunks: vec![Hunk {
160                    id: HunkId("h1".into()),
161                    old_start: 1,
162                    old_lines: 3,
163                    new_start: 1,
164                    new_lines: 3,
165                    context: String::new(),
166                    lines: vec![
167                        diff_line(LineKind::Context, Some(1), Some(1), "one"),
168                        diff_line(LineKind::Deleted, Some(2), None, "two"),
169                        diff_line(LineKind::Added, None, Some(2), "TWO"),
170                        diff_line(LineKind::Context, Some(3), Some(3), "three"),
171                    ],
172                }],
173                hashes: crate::model::HashCache::default(),
174            }],
175        }
176    }
177
178    fn opts<'a>() -> FeedbackOptions<'a> {
179        FeedbackOptions {
180            title: "Review feedback",
181            file_filter: None,
182            include_resolved: false,
183        }
184    }
185
186    #[test]
187    fn single_line_comment_renders_heading_and_context_fence() {
188        let mut s = Session::default();
189        s.add_comment(anchor("src/auth.py", Some(2)), "reviewer", "why uppercase?");
190        let md = to_markdown(&s, &sample_model(), &opts());
191        assert!(md.starts_with("## Review feedback\n\n"));
192        assert!(md.contains("### src/auth.py:2\n"));
193        assert!(md.contains("```\n-two\n+TWO\n three\n```\n"));
194        assert!(md.contains("> why uppercase?\n"));
195        assert!(md.ends_with('\n'));
196    }
197
198    #[test]
199    fn old_side_anchor_finds_deleted_line() {
200        let mut s = Session::default();
201        let mut a = anchor("src/auth.py", Some(2));
202        a.on_old_side = true;
203        s.add_comment(a, "reviewer", "what was wrong with two?");
204        let md = to_markdown(&s, &sample_model(), &opts());
205        assert!(md.contains("```\n one\n-two\n+TWO\n```\n"));
206    }
207
208    #[test]
209    fn range_comment_renders_start_dash_end() {
210        let mut s = Session::default();
211        let mut a = anchor("src/auth.py", Some(3));
212        a.line_end = Some(5);
213        s.add_comment(a, "reviewer", "this whole block");
214        let md = to_markdown(&s, &sample_model(), &opts());
215        assert!(md.contains("### src/auth.py:3-5\n"));
216    }
217
218    #[test]
219    fn file_filter_excludes_other_files() {
220        let mut s = Session::default();
221        s.add_comment(anchor("src/auth.py", Some(2)), "reviewer", "keep");
222        s.add_comment(anchor("other.py", Some(1)), "reviewer", "drop");
223        let o = FeedbackOptions {
224            file_filter: Some("src/auth.py"),
225            ..opts()
226        };
227        let md = to_markdown(&s, &sample_model(), &o);
228        assert!(md.contains("> keep\n"));
229        assert!(!md.contains("drop"));
230    }
231
232    #[test]
233    fn resolved_skipped_by_default_included_and_marked_with_flag() {
234        let mut s = Session::default();
235        let id = s
236            .add_comment(anchor("src/auth.py", Some(2)), "reviewer", "done already")
237            .id
238            .clone();
239        assert!(s.resolve(&id));
240        let md = to_markdown(&s, &sample_model(), &opts());
241        assert!(!md.contains("done already"));
242
243        let o = FeedbackOptions {
244            include_resolved: true,
245            ..opts()
246        };
247        let md = to_markdown(&s, &sample_model(), &o);
248        assert!(md.contains("> done already\n"));
249        assert!(md.contains("_(resolved)_\n"));
250    }
251
252    #[test]
253    fn departed_file_renders_outdated_marker() {
254        let mut s = Session::default();
255        s.add_comment(anchor("gone.py", Some(7)), "reviewer", "still matters");
256        let md = to_markdown(&s, &sample_model(), &opts());
257        assert!(md.contains("### gone.py:7\n_(outdated)_\n"));
258        assert!(md.contains("> still matters\n"));
259    }
260
261    #[test]
262    fn departed_line_renders_outdated_marker() {
263        let mut s = Session::default();
264        s.add_comment(anchor("src/auth.py", Some(99)), "reviewer", "moved on");
265        let md = to_markdown(&s, &sample_model(), &opts());
266        assert!(md.contains("### src/auth.py:99\n_(outdated)_\n"));
267    }
268
269    #[test]
270    fn file_level_comment_has_no_fence_when_file_present() {
271        let mut s = Session::default();
272        s.add_comment(anchor("src/auth.py", None), "reviewer", "overall: nice");
273        let md = to_markdown(&s, &sample_model(), &opts());
274        assert!(md.contains("### src/auth.py\n> overall: nice\n"));
275        assert!(!md.contains("```"));
276        assert!(!md.contains("_(outdated)_"));
277    }
278
279    #[test]
280    fn replies_render_as_nested_quotes() {
281        let mut s = Session::default();
282        let id = s
283            .add_comment(anchor("src/auth.py", Some(2)), "reviewer", "why?")
284            .id
285            .clone();
286        assert!(s.reply(&id, "agent", "because tests\nand style"));
287        let md = to_markdown(&s, &sample_model(), &opts());
288        assert!(md.contains("> why?\n> > agent: because tests\n> > and style\n"));
289    }
290
291    #[test]
292    fn fenced_context_survives_backticks_in_diff_content() {
293        let mut model = sample_model();
294        if let Some(line) = model.files[0].hunks[0].lines.get_mut(2) {
295            line.text = "````md".into();
296        }
297        let mut s = Session::default();
298        s.add_comment(anchor("src/auth.py", Some(2)), "reviewer", "fence bomb");
299        let md = to_markdown(&s, &model, &opts());
300        let fence = "`````";
301        assert!(
302            md.contains(&format!("{fence}\n")),
303            "fence must outrun content runs: {md}"
304        );
305        let open = md.find(fence).expect("opening fence");
306        let close = md.rfind(fence).expect("closing fence");
307        assert!(close > open);
308    }
309
310    #[test]
311    fn comments_order_by_file_then_line() {
312        let mut s = Session::default();
313        s.add_comment(anchor("z.py", Some(1)), "reviewer", "third");
314        s.add_comment(anchor("src/auth.py", Some(3)), "reviewer", "second");
315        s.add_comment(anchor("src/auth.py", Some(1)), "reviewer", "first");
316        let md = to_markdown(&s, &sample_model(), &opts());
317        let first = md.find("> first").expect("first present");
318        let second = md.find("> second").expect("second present");
319        let third = md.find("> third").expect("third present");
320        assert!(first < second && second < third);
321    }
322}