forge_notify: diff mentions against raw excerpt to avoid heading-mention duplication

This commit is contained in:
damocles 2026-05-28 20:01:05 +02:00 committed by Mara
commit b5be139fb7

View file

@ -508,21 +508,23 @@ async fn format_comment_notification(
let author = if actor_login.is_empty() { "?" } else { actor_login };
let NotifMeta { title, notif_type, num, repo, meta_suffix, .. } = meta;
// Escape ATX headings in the user-authored body so the embedded
// text doesn't blow into top-level h1/h2 in the wrapper message
// when the dashboard renders it (closes #455). Done once here
// because both code paths fall through the same truncate+embed
// pattern.
let escaped = escape_md_headings(body_text);
let body_for_embed = truncate(&escaped, BODY_TRUNCATE);
// Truncate the raw body first so the mention-overflow diff compares
// like-for-like (escape_md_headings rewrites `# foo` to `\# foo`, so
// doing it before the diff would re-surface heading-prefixed mention
// lines as fake overflow). Escape happens after for display only.
let raw_excerpt = truncate(body_text, BODY_TRUNCATE);
// Surface @mentions that fell outside the truncation window so an
// addressed agent never silently misses a tag on a long comment
// (closes #539). Skipped when the embed wasn't actually truncated.
let truncated_mentions = if body_text.len() > BODY_TRUNCATE {
render_truncated_mentions(&extract_truncated_mention_lines(body_text, &body_for_embed))
render_truncated_mentions(&extract_truncated_mention_lines(body_text, &raw_excerpt))
} else {
String::new()
};
// Escape ATX headings in the user-authored body so the embedded
// text doesn't blow into top-level h1/h2 in the wrapper message
// when the dashboard renders it (closes #455).
let body_for_embed = escape_md_headings(&raw_excerpt);
if let Some(review_label) = review_state {
// Review submission on a PR.
let kind = format!("PR {review_label}{num}{repo}");
@ -610,10 +612,14 @@ fn format_state_change_notification(
.map(str::trim)
.filter(|s| !s.is_empty())
.map(|raw| {
let escaped = escape_md_headings(raw);
let excerpt = truncate(&escaped, BODY_TRUNCATE);
let truncated = extract_truncated_mention_lines(raw, &excerpt);
// Truncate raw first so mention diff sees the same heading
// markers as full_body (see comment in
// format_comment_notification). Escape happens after for
// display only.
let raw_excerpt = truncate(raw, BODY_TRUNCATE);
let truncated = extract_truncated_mention_lines(raw, &raw_excerpt);
let mentions = render_truncated_mentions(&truncated);
let excerpt = escape_md_headings(&raw_excerpt);
format!("\n\n{excerpt}{mentions}")
})
.unwrap_or_default();
@ -917,6 +923,21 @@ mod tests {
assert_eq!(lines, vec!["@argus reviewer"]);
}
#[test]
fn extract_truncated_does_not_resurface_heading_mention_inside_window() {
// Regression for argus's nit on PR #544: the diff used to
// compare full_body against the *escaped* excerpt. Lines like
// `# @argus check this` survived as-is in the body but became
// `\# @argus check this` in the excerpt, so the `contains`
// check failed and the mention was re-surfaced as if it had
// fallen outside the window. Pass the unescaped excerpt and
// the duplicate disappears.
let full = "# @argus check this\nmore body\n";
let raw_excerpt = full; // fits entirely
let lines = extract_truncated_mention_lines(full, raw_excerpt);
assert!(lines.is_empty(), "heading+mention inside window must not be re-surfaced, got {lines:?}");
}
#[test]
fn render_truncated_mentions_empty_is_empty_string() {
// Zero overhead on the healthy short-body path: caller