refactor(telegram): address code review findings

- Add strip_tool_call_tags() to finalize_draft to prevent Markdown
  parse failures from tool-call tags reaching Telegram API
- Deduplicate parse_reply_target() call in update_draft (was called
  twice, discarding thread_id both times)
- Replace body.as_object_mut().unwrap() mutation with separate
  plain_body JSON literal (eliminates unwrap in runtime path)
- Clean up per-chat rate-limit HashMap entry in finalize_draft to
  prevent unbounded growth over long uptimes
- Extract magic number 80 to STREAM_CHUNK_MIN_CHARS constant in
  agent loop
This commit is contained in:
Xiangjun Ma 2026-02-18 00:32:35 -08:00 committed by Chummy
parent e326e12039
commit f1db63219c
2 changed files with 20 additions and 9 deletions

View file

@ -15,6 +15,9 @@ use std::sync::{Arc, LazyLock};
use std::time::Instant; use std::time::Instant;
use uuid::Uuid; use uuid::Uuid;
/// Minimum characters per chunk when relaying LLM text to a streaming draft.
const STREAM_CHUNK_MIN_CHARS: usize = 80;
/// Default maximum agentic tool-use iterations per user message to prevent runaway loops. /// Default maximum agentic tool-use iterations per user message to prevent runaway loops.
/// Used as a safe fallback when `max_tool_iterations` is unset or configured as zero. /// Used as a safe fallback when `max_tool_iterations` is unset or configured as zero.
const DEFAULT_MAX_TOOL_ITERATIONS: usize = 10; const DEFAULT_MAX_TOOL_ITERATIONS: usize = 10;
@ -944,11 +947,12 @@ pub(crate) async fn run_tool_call_loop(
// If a streaming sender is provided, relay the text in small chunks // If a streaming sender is provided, relay the text in small chunks
// so the channel can progressively update the draft message. // so the channel can progressively update the draft message.
if let Some(ref tx) = on_delta { if let Some(ref tx) = on_delta {
// Split on whitespace boundaries, accumulating ~80-char chunks. // Split on whitespace boundaries, accumulating chunks of at least
// STREAM_CHUNK_MIN_CHARS characters for progressive draft updates.
let mut chunk = String::new(); let mut chunk = String::new();
for word in display_text.split_inclusive(char::is_whitespace) { for word in display_text.split_inclusive(char::is_whitespace) {
chunk.push_str(word); chunk.push_str(word);
if chunk.len() >= 80 { if chunk.len() >= STREAM_CHUNK_MIN_CHARS {
if tx.send(std::mem::take(&mut chunk)).await.is_err() { if tx.send(std::mem::take(&mut chunk)).await.is_err() {
break; // receiver dropped break; // receiver dropped
} }

View file

@ -1264,11 +1264,12 @@ impl Channel for TelegramChannel {
message_id: &str, message_id: &str,
text: &str, text: &str,
) -> anyhow::Result<()> { ) -> anyhow::Result<()> {
let (chat_id, _) = Self::parse_reply_target(recipient);
// Rate-limit edits per chat // Rate-limit edits per chat
{ {
let (chat_id_for_limit, _) = Self::parse_reply_target(recipient);
let last_edits = self.last_draft_edit.lock(); let last_edits = self.last_draft_edit.lock();
if let Some(last_time) = last_edits.get(&chat_id_for_limit) { if let Some(last_time) = last_edits.get(&chat_id) {
let elapsed = u64::try_from(last_time.elapsed().as_millis()).unwrap_or(u64::MAX); let elapsed = u64::try_from(last_time.elapsed().as_millis()).unwrap_or(u64::MAX);
if elapsed < self.draft_update_interval_ms { if elapsed < self.draft_update_interval_ms {
return Ok(()); return Ok(());
@ -1276,8 +1277,6 @@ impl Channel for TelegramChannel {
} }
} }
let (chat_id, _) = Self::parse_reply_target(recipient);
// Truncate to Telegram limit for mid-stream edits (UTF-8 safe) // Truncate to Telegram limit for mid-stream edits (UTF-8 safe)
let display_text = if text.len() > TELEGRAM_MAX_MESSAGE_LENGTH { let display_text = if text.len() > TELEGRAM_MAX_MESSAGE_LENGTH {
let mut end = 0; let mut end = 0;
@ -1333,8 +1332,12 @@ impl Channel for TelegramChannel {
message_id: &str, message_id: &str,
text: &str, text: &str,
) -> anyhow::Result<()> { ) -> anyhow::Result<()> {
let text = &strip_tool_call_tags(text);
let (chat_id, thread_id) = Self::parse_reply_target(recipient); let (chat_id, thread_id) = Self::parse_reply_target(recipient);
// Clean up rate-limit tracking for this chat
self.last_draft_edit.lock().remove(&chat_id);
// If text exceeds limit, delete draft and send as chunked messages // If text exceeds limit, delete draft and send as chunked messages
if text.len() > TELEGRAM_MAX_MESSAGE_LENGTH { if text.len() > TELEGRAM_MAX_MESSAGE_LENGTH {
let msg_id = match message_id.parse::<i64>() { let msg_id = match message_id.parse::<i64>() {
@ -1375,7 +1378,7 @@ impl Channel for TelegramChannel {
}; };
// Try editing with Markdown formatting // Try editing with Markdown formatting
let mut body = serde_json::json!({ let body = serde_json::json!({
"chat_id": chat_id, "chat_id": chat_id,
"message_id": msg_id, "message_id": msg_id,
"text": text, "text": text,
@ -1394,12 +1397,16 @@ impl Channel for TelegramChannel {
} }
// Markdown failed — retry without parse_mode // Markdown failed — retry without parse_mode
body.as_object_mut().unwrap().remove("parse_mode"); let plain_body = serde_json::json!({
"chat_id": chat_id,
"message_id": msg_id,
"text": text,
});
let resp = self let resp = self
.client .client
.post(self.api_url("editMessageText")) .post(self.api_url("editMessageText"))
.json(&body) .json(&plain_body)
.send() .send()
.await?; .await?;