Skip to content

fix(work): cap total inlined comment size in build_prompt's dispatch prompt #449

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
getappz wants to merge 1 commit into from
Closed

fix(work): cap total inlined comment size in build_prompt's dispatch prompt #449

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
56 changes: 55 additions & 1 deletion src/cli/work.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,15 @@ pub struct WorkArgs {
/// success-comment reply doing on the way out.
const HANDOFF_ASSET_MAX_CHARS: usize = 8_000;

/// Cap on the total size of the "Prior discussion" section `build_prompt`
/// inlines from an item's comments. #81 bounds each individual posted
/// comment's body, but a thread can still accumulate many bounded comments
/// into an unbounded total -- and since #441 moved prompt delivery from argv
/// (which had the OS's E2BIG as an accidental backstop) to stdin (which has
/// none), nothing else catches that growth. Same tail-and-pointer discipline
/// as `HANDOFF_ASSET_MAX_CHARS`.
const COMMENTS_INLINE_MAX_CHARS: usize = 8_000;

/// The last `max_chars` characters of `s`, UTF-8-boundary-safe.
fn tail_chars(s: &str, max_chars: usize) -> &str {
match s.char_indices().rev().nth(max_chars.saturating_sub(1)) {
Expand Down Expand Up @@ -176,8 +185,22 @@ fn build_prompt(
};
if !comments.is_empty() {
prompt.push_str("\nPrior discussion:\n");
let mut section = String::new();
for c in comments {
prompt.push_str(&format!("- [{}] {}\n", c.author_agent, c.body));
section.push_str(&format!("- [{}] {}\n", c.author_agent, c.body));
}
let total_chars = section.chars().count();
Comment on lines +188 to +192

@coderabbitai coderabbitai Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win

Apply the cap before materializing the full history.

The code renders every comment into section before it checks total_chars. A large thread can still allocate the complete unbounded history, even though the final prompt contains only the tail. Count rendered lengths first, then build only the bounded tail when truncation is required.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli/work.rs` around lines 188 - 192, Update the comment-history
construction around the comments loop and total_chars calculation to measure
each rendered comment’s length before appending it, avoiding allocation of the
full unbounded section. When the history exceeds the cap, build only the
required tail; otherwise preserve the complete rendered history.

if total_chars <= COMMENTS_INLINE_MAX_CHARS {
prompt.push_str(&section);
} else {
prompt.push_str(&format!(
"(showing the last {COMMENTS_INLINE_MAX_CHARS} of {total_chars} chars across \
{} comments -- full history via `mcp__flare__comment` action=list \
item_id={})\n\n{}\n",
comments.len(),
item.id,
tail_chars(&section, COMMENTS_INLINE_MAX_CHARS)
));
Comment on lines +196 to +203

@coderabbitai coderabbitai Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Report omitted counts explicitly.

The message reports “the last 8,000 of total_chars chars across comments.len() comments.” It does not state how many comments were omitted, and it makes the character count implicit. Compute omission counts from the truncation boundary and include them in the prompt metadata.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli/work.rs` around lines 196 - 203, Update the prompt metadata in the
truncation branch using the existing truncation boundary and comment collection
to compute and explicitly report the omitted comment count and omitted character
count. Preserve the existing displayed tail content and total counts, and make
the character units explicit in the message.

}
}
if let Some(handoff) = latest_handoff {
Expand Down Expand Up @@ -1110,6 +1133,37 @@ mod tests {
assert!(handoff.ends_with(&"x".repeat(HANDOFF_ASSET_MAX_CHARS)));
}

#[test]
fn build_prompt_caps_an_oversized_comment_thread_at_the_tail_with_a_pointer() {
// #85: each individual comment is bounded by #81, but a thread with
// many bounded comments can still sum to an unbounded total -- and
// since #441 moved prompt delivery to stdin, nothing else catches
// that growth. Total inlined comment text must be capped, not
// dumped unbounded into the prompt.
let item = test_item();
let comments: Vec<agentflare_backend::comment::ItemComment> = (0..50)
.map(|i| agentflare_backend::comment::ItemComment {
id: format!("c{i}"),
item_id: "item-1".into(),
author_agent: "alice".into(),
body: "x".repeat(500),
created_at: 0,
updated_at: 0,
})
.collect();
let prompt = build_prompt(&item, &comments, None);
assert!(
prompt.len() < comments.len() * 500,
"capped prompt must be much shorter than the unbounded concatenation"
);
assert!(prompt.contains("full history via `mcp__flare__comment` action=list"));
assert!(prompt.contains(&item.id));
assert!(prompt.contains("50 comments"));
// the tail cut lands inside the run of bounded comments, so the very
// last (most recent) comment's full body must still be intact.
assert!(prompt.contains(&format!("{}\n", "x".repeat(500))));
Comment on lines +1144 to +1164

@coderabbitai coderabbitai Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Make the regression test prove the truncation contract.

All comment bodies are identical, so the final contains assertion passes even if an older comment is retained. prompt.len() measures bytes and only checks a loose threshold. Give each body a unique marker, include multibyte text, assert the latest marker is present, and assert the extracted tail has at most COMMENTS_INLINE_MAX_CHARS characters.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli/work.rs` around lines 1144 - 1164, Strengthen the regression test
around build_prompt by assigning each comment a unique body containing multibyte
text, then assert the latest comment’s marker is present. Extract the inline
comment tail from the generated prompt and assert its character count is at most
COMMENTS_INLINE_MAX_CHARS, replacing the byte-length threshold and ambiguous
identical-body assertion.

}

#[test]
fn parse_claude_reply_extracts_structured_fields() {
let raw = r#"{"result":"Fixed the race by adding a mutex.","session_id":"sess-123","total_cost_usd":0.0842}"#;
Expand Down
Loading