From a2dca32bb8cfd0667e1be1ff8564ddabe8b874cf Mon Sep 17 00:00:00 2001 From: atlas Date: Fri, 16 Oct 2026 21:31:48 +0200 Subject: [PATCH] review classifier: plain table cells, no backslash escapes --- docs/swarm/review-classifier.md | 13 ++-- .../src/review_classifier/review.rs | 60 +++++++++++++++---- swarm-review-classifier/src/prompt.rs | 2 +- 3 files changed, 57 insertions(+), 18 deletions(-) diff --git a/docs/swarm/review-classifier.md b/docs/swarm/review-classifier.md index 0ab14773..10a97388 100644 --- a/docs/swarm/review-classifier.md +++ b/docs/swarm/review-classifier.md @@ -398,11 +398,14 @@ followed by the table when there is one. A comment neither approves nor blocks the merge: nine timeouts and one pass post a comment, not an approval. -The controller flattens every cell to one line, with `|`, backticks, brackets, `<`, `>`, -`&`, `#` and other markup characters escaped, and a zero-width space after -each `@`, `#`, `!`, `://` and `www.`, so a model's `reason` can't break the -table, embed markup, mention anyone, reference an issue or PR, or carry a -link. +The controller flattens every cell to one line and backslash-escapes the +characters that could end the cell or start markup, math or raw HTML: `|`, `\`, +backticks, `[`, `]`, `<`, `>`, `&`, `*`, `~`, `$`, and `_` except between two +letters or digits (`ci_metrics.rs` stays as is). Ordinary punctuation such as +`(`, `)` and `.` passes through unescaped. Each `@`, `#`, `!`, `://` and `www.` +gets a zero-width space after it, with no backslash, so a model's `reason` +can't break the table, embed markup, mention anyone, reference an issue or PR, +or carry a link. The prompt asks the model for plain text in `reason`. ## Runtimes diff --git a/swarm-controller/src/review_classifier/review.rs b/swarm-controller/src/review_classifier/review.rs index 5e698ec0..9dc03680 100644 --- a/swarm-controller/src/review_classifier/review.rs +++ b/swarm-controller/src/review_classifier/review.rs @@ -842,26 +842,37 @@ fn overall_line(overall: Overall) -> &'static str { } } -/// `text` as one table cell's inline text: on one line, with every character -/// that could end the cell, start markup, math (`$`) or raw HTML escaped, and a zero-width -/// space after every `@`, `#`, `!`, `://` and `www.`, so it carries no live -/// mention, issue or PR reference, or link. The forge links issue and PR -/// numbers and bare URLs itself, so the escapes alone don't stop those. +/// `text` as one table cell's inline text: on one line, with a backslash before +/// every character that could end the cell, start markup, math (`$`) or raw +/// HTML, and a zero-width space after every `@`, `#`, `!`, `://` and `www.`, so +/// it carries no live mention, issue or PR reference, or link. The forge links +/// issue and PR numbers and bare URLs itself, so the escapes alone don't stop +/// those; `#` and `!` get only the zero-width space. +/// +/// `_` is escaped except between two alphanumerics, where it can't open +/// emphasis (`ci_metrics.rs`). Other punctuation, such as `(`, `)`, `.` and +/// `:`, passes through: it starts no markup inside a table cell, and the `[` +/// that would make `(` a link target is escaped. fn cell(text: &str) -> String { let mut out = String::with_capacity(text.len()); for (i, word) in text.split_whitespace().enumerate() { if i > 0 { out.push(' '); } - for c in word.chars() { + let chars: Vec = word.chars().collect(); + for (j, &c) in chars.iter().enumerate() { match c { - '\\' | '|' | '`' | '*' | '_' | '~' | '[' | ']' | '(' | ')' | '<' | '>' | '&' - | '$' => { + '_' if j > 0 + && chars[j - 1].is_alphanumeric() + && chars.get(j + 1).is_some_and(|n| n.is_alphanumeric()) => + { + out.push(c); + } + '\\' | '|' | '`' | '*' | '_' | '~' | '[' | ']' | '<' | '>' | '&' | '$' => { out.push('\\'); out.push(c); } '#' | '!' => { - out.push('\\'); out.push(c); out.push('\u{200b}'); } @@ -1932,7 +1943,7 @@ mod tests { .count(); assert_eq!(unescaped, 5, "{line}"); assert!( - line.contains("\\!\u{200b}\\[\\]") && line.contains("\\"), + line.contains("!\u{200b}\\[\\]") && line.contains("\\"), "{line}" ); } @@ -1965,8 +1976,33 @@ mod tests { let text = cell("see https://evil.example/x, WWW.evil.example, #1, o/r#3 and !4"); assert_eq!( text, - "see https://\u{200b}evil.example/x, WWW.\u{200b}evil.example, \\#\u{200b}1, \ - o/r\\#\u{200b}3 and \\!\u{200b}4" + "see https://\u{200b}evil.example/x, WWW.\u{200b}evil.example, #\u{200b}1, \ + o/r#\u{200b}3 and !\u{200b}4" + ); + } + + #[test] + fn a_plain_reason_reads_without_backslashes() { + assert_eq!( + cell("an added #[allow(clippy::foo)] in ci_metrics.rs (see main_loop_v2.rs)"), + "an added #\u{200b}\\[allow(clippy::foo)\\] in ci_metrics.rs (see main_loop_v2.rs)" + ); + } + + #[test] + fn an_underscore_escapes_unless_it_sits_between_alphanumerics() { + // Presence control: the edge underscores are escaped. + assert_eq!( + cell("_em_ __b__ a_ _b a__b snake_case"), + "\\_em\\_ \\_\\_b\\_\\_ a\\_ \\_b a\\_\\_b snake_case" + ); + } + + #[test] + fn a_reason_with_markup_characters_keeps_them_inert() { + assert_eq!( + cell(r"a\b `c` *d* ~e~ [f] &h; i|j $k"), + r"a\\b \`c\` \*d\* \~e\~ \[f\] \ \&h; i\|j \$k" ); } diff --git a/swarm-review-classifier/src/prompt.rs b/swarm-review-classifier/src/prompt.rs index 082b25c5..863e6a31 100644 --- a/swarm-review-classifier/src/prompt.rs +++ b/swarm-review-classifier/src/prompt.rs @@ -26,7 +26,7 @@ Answer with exactly one JSON object and nothing else, no code fence: - \"no-match\": the answer is no. - \"not-applicable\": the question does not concern anything this pull request changes. - \"certainty\": how sure you are of that verdict, 0 to 100. -- \"reason\": one or two sentences saying why."; +- \"reason\": one or two sentences saying why, in plain text only: no markdown, no backticks, no backslash escapes."; /// The shared context: the PR's commits with their full messages, then its /// diff with lockfiles collapsed. Built once per run.