Conversation
…prompt build_prompt's comment loop concatenated every prior comment on an item with no aggregate cap on total count/size, unlike latest_handoff_content's HANDOFF_ASSET_MAX_CHARS tail-and-pointer discipline. #81 bounds each individual posted comment, but a thread with many bounded comments could still sum to an unbounded total -- and since #441 moved prompt delivery from argv (E2BIG as an accidental backstop) to stdin (no OS limit), nothing else caught that growth. Apply the same tail-cap-and-pointer pattern to the comments section, with a regression test.
📝 WalkthroughWalkthroughThe CLI now caps the combined “Prior discussion” prompt section at 8,000 characters. Oversized comment histories use a UTF-8-safe tail preview with metadata and MCP retrieval instructions. A test verifies truncation and latest-comment preservation. ChangesComment History Prompt Handling
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with 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.
Inline comments:
In `@src/cli/work.rs`:
- Around line 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.
- Around line 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.
- Around line 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.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 439c9afc-96a2-4d76-9631-dea39bc134c0
📒 Files selected for processing (1)
src/cli/work.rs
| 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(); |
There was a problem hiding this comment.
🚀 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.
| 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(§ion, COMMENTS_INLINE_MAX_CHARS) | ||
| )); |
There was a problem hiding this comment.
🎯 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.
| 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)))); |
There was a problem hiding this comment.
🎯 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.
|
Duplicate — the same fix already landed via #450 (independently, same problem/pattern: |
1 participant
Summary
build_prompt's comment loop (src/cli/work.rs) concatenated every prior comment on an item with no aggregate cap on total count/size — unlikelatest_handoff_content'sHANDOFF_ASSET_MAX_CHARStail-and-pointer discipline in the same file.latest_handoff_contentto the comments section: caps total inlined comment text atCOMMENTS_INLINE_MAX_CHARS(8,000 chars), and when truncated tells the agent how many comments/chars were omitted and points atmcp__flare__comment action=listfor the full history.Test plan
cargo build --bin agentflare --libcargo test --bin agentflare cli::work::(34 passed, including newbuild_prompt_caps_an_oversized_comment_thread_at_the_tail_with_a_pointer)cargo clippy --locked --workspace --all-targets --all-features -- -D warnings -A unsafe_code -A clippy::pedanticcargo fmt --checkCloses #85.
Summary by CodeRabbit