Skip to content

fix(channel): direct relay retrigger results to prevent truncated replies #367

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

Open
lutr0 wants to merge 2 commits into
base: main
Choose a base branch
from
+131 −13
Open

fix(channel): direct relay retrigger results to prevent truncated replies #367

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e1d42f3
fix(channel): direct relay retrigger results to prevent truncated rep…
lutr0 Mar 8, 2026
fdfd814
Merge branch 'main' into fix/retrigger-truncated-replies
jamiepine Mar 8, 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
144 changes: 131 additions & 13 deletions src/agent/channel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1772,22 +1772,69 @@ impl Channel {
// reply content payload, this fallback preserves a compact background
// result record for the next user turn.
if is_retrigger {
// Extract the result summaries from the metadata we attached in
// flush_pending_retrigger, so we record only the substance (not
// the retrigger instructions/template scaffolding).
let summary = message
.metadata
.get("retrigger_result_summary")
.and_then(|v| v.as_str())
.unwrap_or("[background work completed]");

let replied = replied_flag.load(std::sync::atomic::Ordering::Relaxed);
if replied && retrigger_reply_preserved {
tracing::debug!(
channel_id = %self.id,
"skipping retrigger summary injection; relay reply already preserved"
);
} else {
// Extract the result summaries from the metadata we attached in
// flush_pending_retrigger, so we record only the substance (not
// the retrigger instructions/template scaffolding).
let summary = message
.metadata
.get("retrigger_result_summary")
.and_then(|v| v.as_str())
.unwrap_or("[background work completed]");
// Guardrail: sometimes the model acknowledges the retrigger with a
// tiny teaser (e.g. "here's where we stand:") and still marks the
// turn as replied. If that happens, proactively relay the full
// summary so users don't see cut responses.
let last_assistant_len = {
let history = self.state.history.read().await;
history
.iter()
.rev()
.find_map(|m| match m {
rig::message::Message::Assistant { content, .. } => content
.iter()
.find_map(|c| match c {
rig::message::AssistantContent::Text(t) => {
Some(t.text.trim().chars().count())
}
_ => None,
}),
_ => None,
})
.unwrap_or(0)
};

if last_assistant_len < 80 && summary.chars().count() > 200 {
tracing::warn!(
Copy link
Contributor

@tembo tembo bot Mar 8, 2026

Choose a reason for hiding this comment

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

In the truncation guardrail path, we send summary as a fallback, but we don't update self.state.history, so the in-memory transcript can stay as the teaser-only assistant message. Consider replacing the last assistant message (or injecting summary) when this triggers so subsequent turns/compaction match what the user actually saw.

channel_id = %self.id,
last_assistant_len,
summary_len = summary.chars().count(),
"retrigger reply looks truncated; sending summary fallback"
);
self.state.conversation_logger.log_bot_message(
&self.state.channel_id,
summary,
);
if let Err(error) = self
.response_tx
.send(OutboundResponse::Text(summary.to_string()))
.await
{
tracing::error!(
%error,
channel_id = %self.id,
"failed to send retrigger truncation fallback reply"
);
}
} else {
tracing::debug!(
channel_id = %self.id,
"skipping retrigger summary injection; relay reply already preserved"
);
}
Comment on lines +1809 to +1836
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 8, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Truncation fallback sends message to user without updating history.

When the truncation guardrail triggers (lines 1728-1742), the full summary is sent to the user, but history still contains only the truncated assistant reply. This creates a mismatch: the user sees the full summary, but subsequent LLM turns will only see the truncated message in history.

The else branch at lines 1749-1776 correctly handles history injection. The fallback path should do the same to maintain consistency.

🔧 Proposed fix: update history after sending fallback 
                 if last_assistant_len < 80 && summary.chars().count() > 200 {
                     tracing::warn!(
                         channel_id = %self.id,
                         last_assistant_len,
                         summary_len = summary.chars().count(),
                         "retrigger reply looks truncated; sending summary fallback"
                     );
                     self.state.conversation_logger.log_bot_message(
                         &self.state.channel_id,
                         summary,
                     );
                     if let Err(error) = self
                         .response_tx
                         .send(OutboundResponse::Text(summary.to_string()))
                         .await
                     {
                         tracing::error!(
                             %error,
                             channel_id = %self.id,
                             "failed to send retrigger truncation fallback reply"
                         );
                     }
+                    // Update history so the LLM sees what the user actually received
+                    let mut history = self.state.history.write().await;
+                    pop_retrigger_bridge_message(&mut history);
+                    history.push(rig::message::Message::Assistant {
+                        id: None,
+                        content: OneOrMany::one(rig::message::AssistantContent::text(
+                            summary.to_string(),
+                        )),
+                    });
                 } else {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if last_assistant_len < 80 && summary.chars().count() > 200 {
tracing::warn!(
channel_id = %self.id,
last_assistant_len,
summary_len = summary.chars().count(),
"retrigger reply looks truncated; sending summary fallback"
);
self.state.conversation_logger.log_bot_message(
&self.state.channel_id,
summary,
);
if let Err(error) = self
.response_tx
.send(OutboundResponse::Text(summary.to_string()))
.await
{
tracing::error!(
%error,
channel_id = %self.id,
"failed to send retrigger truncation fallback reply"
);
}
} else {
tracing::debug!(
channel_id = %self.id,
"skipping retrigger summary injection; relay reply already preserved"
);
}
if last_assistant_len < 80 && summary.chars().count() > 200 {
tracing::warn!(
channel_id = %self.id,
last_assistant_len,
summary_len = summary.chars().count(),
"retrigger reply looks truncated; sending summary fallback"
);
self.state.conversation_logger.log_bot_message(
&self.state.channel_id,
summary,
);
if let Err(error) = self
.response_tx
.send(OutboundResponse::Text(summary.to_string()))
.await
{
tracing::error!(
%error,
channel_id = %self.id,
"failed to send retrigger truncation fallback reply"
);
}
// Update history so the LLM sees what the user actually received
let mut history = self.state.history.write().await;
pop_retrigger_bridge_message(&mut history);
history.push(rig::message::Message::Assistant {
id: None,
content: OneOrMany::one(rig::message::AssistantContent::text(
summary.to_string(),
)),
});
} else {
tracing::debug!(
channel_id = %self.id,
"skipping retrigger summary injection; relay reply already preserved"
);
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/agent/channel.rs` around lines 1721 - 1748, When the truncation fallback
sends the full summary to the user (the branch that calls
self.state.conversation_logger.log_bot_message and
self.response_tx.send(OutboundResponse::Text(summary.to_string())), you must
also update the conversation history the same way the else branch does so future
LLM turns see the full summary; mirror the history-injection logic from the else
branch (the same call(s) that the else path uses to persist/append the preserved
assistant reply into memory) immediately after a successful send, and handle any
errors similarly to maintain consistency between what the user saw and what is
stored.

} else {
let record = if replied {
summary.to_string()
} else {
Expand Down Expand Up @@ -2831,6 +2878,77 @@ impl Channel {

let result_count = self.pending_results.len();

// Direct relay mode for retrigger results: send full worker/branch output
// without another LLM pass. This avoids teaser/truncated retrigger replies.
let direct_relay_text = self
.pending_results
.iter()
.enumerate()
.map(|(idx, r)| {
let status = if r.success { "completed" } else { "failed" };
if result_count == 1 {
if r.success {
r.result.clone()
} else {
format!("Background task {}.\n\n{}", status, r.result)
}
} else {
format!(
"Update {} ({}, {}):\n{}",
idx + 1,
r.process_type,
status,
r.result
)
}
})
.collect::<Vec<_>>()
.join("\n\n");

if !direct_relay_text.trim().is_empty() {
tracing::info!(
channel_id = %self.id,
result_count,
relay_len = direct_relay_text.len(),
"relaying retrigger results directly without LLM"
);

self.state
.conversation_logger
.log_bot_message(&self.state.channel_id, &direct_relay_text);
if let Err(error) = self
.response_tx
.send(OutboundResponse::Text(direct_relay_text.clone()))
.await
{
tracing::error!(
%error,
channel_id = %self.id,
"failed to send direct retrigger relay"
);
}

let mut history = self.state.history.write().await;
let replaced = pop_retrigger_bridge_message(&mut history);
tracing::debug!(
channel_id = %self.id,
replaced_bridge = replaced,
"injecting direct retrigger relay into history"
);
history.push(rig::message::Message::Assistant {
id: None,
content: OneOrMany::one(rig::message::AssistantContent::text(
direct_relay_text,
)),
});

self.retrigger_count += 1;
self.pending_retrigger = false;
Copy link
Contributor

@tembo tembo bot Mar 8, 2026

Choose a reason for hiding this comment

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

Direct relay skips the retrigger LLM turn, so the StatusBlock::mark_relayed path in handle_message won't run. That means completed items can keep showing full result_summary in the status block and may get re-mentioned on later turns.

Consider marking the just-relayed process_ids as relayed here before clearing pending_results.

Suggested change
self.pending_retrigger = false;
{
let process_ids: Vec<String> = self
.pending_results
.iter()
.map(|result| result.process_id.clone())
.collect();
let mut status = self.state.status_block.write().await;
status.mark_relayed(&process_ids);
}
self.retrigger_count += 1;

self.pending_retrigger_metadata.clear();
self.pending_results.clear();
return;
}

// Build per-result summaries for the template.
let result_items: Vec<_> = self
.pending_results
Expand Down