Skip to content

fix: clean up log analysis comments on GitHub issues #3925

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
devin-ai-integration wants to merge 2 commits into from
Closed

fix: clean up log analysis comments on GitHub issues #3925

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
62ab9e2
fix: strip code fences and ANSI escapes from log analysis comments
devin-ai-integration[bot] Feb 12, 2026
d84105f
fix: disable HTML escaping in markdown templates
devin-ai-integration[bot] Feb 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions crates/api-support/src/github.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const GITHUB_OWNER: &str = "fastrepl";
const GITHUB_REPO: &str = "hyprnote";

#[derive(Template)]
#[template(path = "bug_report.md.jinja")]
#[template(path = "bug_report.md.jinja", escape = "none")]
struct BugReportBody<'a> {
description: &'a str,
platform: &'a str,
Expand All @@ -19,7 +19,7 @@ struct BugReportBody<'a> {
}

#[derive(Template)]
#[template(path = "feature_request.md.jinja")]
#[template(path = "feature_request.md.jinja", escape = "none")]
struct FeatureRequestBody<'a> {
description: &'a str,
platform: &'a str,
Expand All @@ -30,7 +30,7 @@ struct FeatureRequestBody<'a> {
}

#[derive(Template)]
#[template(path = "log_analysis.md.jinja")]
#[template(path = "log_analysis.md.jinja", escape = "none")]
struct LogAnalysisComment<'a> {
summary_section: &'a str,
tail: &'a str,
Expand Down Expand Up @@ -127,15 +127,17 @@ fn make_title(description: &str, fallback: &str) -> (String, String) {
}

async fn attach_log_analysis(state: &AppState, issue_number: u64, log_text: &str) {
let clean_logs = logs::strip_ansi_escapes(log_text);

let log_summary =
logs::analyze_logs(&state.config.openrouter.openrouter_api_key, log_text).await;
logs::analyze_logs(&state.config.openrouter.openrouter_api_key, &clean_logs).await;

let summary_section = match log_summary.as_deref() {
Some(s) if !s.trim().is_empty() => format!("### Summary\n```\n{s}\n```"),
_ => "_No errors or warnings found._".to_string(),
};

let tail = logs::safe_tail(log_text, 10000);
let tail = logs::safe_tail(&clean_logs, 10000);
let comment = LogAnalysisComment {
summary_section: &summary_section,
tail,
Expand Down
36 changes: 35 additions & 1 deletion crates/api-support/src/logs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,37 @@
fn strip_code_fences(s: &str) -> String {
let trimmed = s.trim();
let stripped = trimmed
.strip_prefix("```")
.and_then(|s| {
let s = s.strip_prefix('\n').unwrap_or(s);
s.strip_suffix("```")
})
Comment on lines +4 to +8
Copy link
Contributor Author

@devin-ai-integration devin-ai-integration bot Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 strip_code_fences fails to strip language identifier from fenced code blocks

The strip_code_fences function doesn't handle the common LLM pattern of ```text\n...\n``` or ```json\n...\n```. When the LLM returns a code block with a language identifier, the identifier leaks into the output.

Root Cause and Impact

The function does strip_prefix("```") which removes the opening triple backticks, leaving e.g. "text\ncontent\n\``". Then strip_prefix('\n')fails because the string starts withtext, not a newline. The unwrap_or(s)fallback keeps the full remainder. Finallystrip_suffix("```")removes the closing fence, yielding"text\ncontent"`—the language identifier `text` is now part of the content.

This result then gets embedded at crates/api-support/src/github.rs:136 inside format!("### Summary\n```\n{s}\n```"), producing broken output like:

### Summary
` ` `
text
ERROR: something went wrong
` ` `

Since the whole purpose of this function is to clean up LLM output that wraps responses in code fences, and LLMs very commonly use language-tagged fences (```text, ```json, ```markdown), this is a significant gap.

Fix: After strip_prefix("```"), skip everything up to and including the first \n (not just a bare \n prefix).

Suggested change
.strip_prefix("```")
.and_then(|s| {
let s = s.strip_prefix('\n').unwrap_or(s);
s.strip_suffix("```")
})
.strip_prefix("```")
.and_then(|s| {
let s = match s.find('\n') {
Some(pos) => &s[pos + 1..],
None => s,
};
s.strip_suffix("```")
})
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

.map(|s| s.trim())
.unwrap_or(trimmed);
stripped.to_string()
}
Comment on lines +1 to +12
Copy link
Contributor

@graphite-app graphite-app bot Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The strip_code_fences function fails to handle language identifiers in code fences. When Gemini outputs:

\`\`\`markdown
content
\`\`\`

The function strips the opening ``` leaving "markdown\ncontent\n```", then tries to strip '\n' which fails because "markdown" comes first, so it returns "markdown\ncontent\n```", then strips the trailing ``` leaving "markdown\ncontent". This causes "markdown" (or any language identifier) to appear in the summary section.

Fix: After stripping the opening fence, skip any non-newline characters before the newline:

fn strip_code_fences(s: &str) -> String {
    let trimmed = s.trim();
    let stripped = trimmed
        .strip_prefix("```")
        .and_then(|s| {
            // Skip language identifier if present
            let s = s.trim_start_matches(|c| c != '\n');
            let s = s.strip_prefix('\n').unwrap_or(s);
            s.strip_suffix("```")
        })
        .map(|s| s.trim())
        .unwrap_or(trimmed);
    stripped.to_string()
}
Suggested change
fn strip_code_fences(s: &str) -> String {
let trimmed = s.trim();
let stripped = trimmed
.strip_prefix("```")
.and_then(|s| {
let s = s.strip_prefix('\n').unwrap_or(s);
s.strip_suffix("```")
})
.map(|s| s.trim())
.unwrap_or(trimmed);
stripped.to_string()
}
fn strip_code_fences(s: &str) -> String {
let trimmed = s.trim();
let stripped = trimmed
.strip_prefix("```")
.and_then(|s| {
// Skip language identifier if present
let s = s.trim_start_matches(|c| c != '\n');
let s = s.strip_prefix('\n').unwrap_or(s);
s.strip_suffix("```")
})
.map(|s| s.trim())
.unwrap_or(trimmed);
stripped.to_string()
}

Spotted by Graphite Agent

Fix in Graphite

Is this helpful? React 👍 or 👎 to let us know.


pub(crate) fn strip_ansi_escapes(s: &str) -> String {
let mut result = String::with_capacity(s.len());
let mut chars = s.chars().peekable();
while let Some(c) = chars.next() {
if c == '\x1b' {
if chars.peek() == Some(&'[') {
chars.next();
while let Some(&next) = chars.peek() {
chars.next();
if next.is_ascii_alphabetic() {
break;
}
}
}
Comment on lines +18 to +27
Copy link
Contributor

@graphite-app graphite-app bot Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incomplete ANSI sequence handling: If a log ends with an incomplete ANSI escape sequence (e.g., \x1b[31 without a terminating alphabetic character), the inner while loop at line 21-26 will consume all remaining characters while searching for the terminating letter. This causes the trailing content to be silently dropped.

Fix: Add a safety limit to the inner loop or check for end-of-string:

if c == '\x1b' {
    if chars.peek() == Some(&'[') {
        chars.next();
        let mut count = 0;
        while let Some(&next) = chars.peek() {
            chars.next();
            if next.is_ascii_alphabetic() || count > 20 {
                break;
            }
            count += 1;
        }
    }
}
Suggested change
if c == '\x1b' {
if chars.peek() == Some(&'[') {
chars.next();
while let Some(&next) = chars.peek() {
chars.next();
if next.is_ascii_alphabetic() {
break;
}
}
}
if c == '\x1b' {
if chars.peek() == Some(&'[') {
chars.next();
let mut count = 0;
while let Some(&next) = chars.peek() {
chars.next();
if next.is_ascii_alphabetic() || count > 20 {
break;
}
count += 1;
}
}

Spotted by Graphite Agent

Fix in Graphite

Is this helpful? React 👍 or 👎 to let us know.

} else {
result.push(c);
}
}
result
}

pub(crate) fn safe_tail(s: &str, max_bytes: usize) -> &str {
let start = s.len().saturating_sub(max_bytes);
let start = s
Expand Down Expand Up @@ -27,5 +61,5 @@ pub(crate) async fn analyze_logs(api_key: &str, logs: &str) -> Option<String> {
let resp = client.chat_completion(&req).await.ok()?;
let content = resp.choices.first()?.message.content.as_ref()?;
let text = content.as_text()?;
Some(text.chars().take(800).collect())
Some(strip_code_fences(text).chars().take(800).collect())
}
Loading