@sigitcodereview / sigit / commits / 406ee9d

fix(compaction): survive summarization failures (#125)

Manual and automatic compaction kept failing the same way: the summarization round shipped the whole conversation transcript in one request, which is by definition already over budget when compaction runs, so siGit Code Cloud answered 504 — and the gateway's HTML error page was pasted into chat before the session was told to start over. Three changes make compaction a strategy instead of a single point of failure: - Cap the summarization transcript (~12k estimated tokens, cut from the middle so the opening request and current state survive) — the request can no longer be as big as the conversation that overflowed. - When the summarization round still fails, fall back to a deterministic truncation: drop the oldest messages (tool rounds whole, never orphaning a `role: "tool"` result) until the rebuild fits the budget, with a placeholder explaining the gap. Compaction only errors when even that cannot fit — the genuinely stuck case — and the ACP message now says `/clear` instead of suggesting a retry that cannot help. - HTML error pages (gateway 504s, Rails error pages) no longer get pasted into the error message; the status line already says everything the page does. The summarization round also no longer mutates live history (`OpenAiBackend::summarize` posts a one-off request), so a failed round leaves the session exactly as it was and the fallback decides from a clean snapshot. The local backend gets the same capped transcript and fallback. Fixes getsigit/sigit#125 Co-Authored-By: OndeCode <noreply@ondeinference.com>

Seto Elkahfi committed Oct 4, 2026 at 12:57 UTC 406ee9da6839dea70c3c58db6a8a7425a29514d9
3 files changed +492 -79
src/backend.rs
+472 -66
@@ -76,6 +76,71 @@ pub const COMPACT_KEEP_LAST: usize = 6;
76 const SUMMARIZE_PROMPT: &str = "Summarize this coding session so far: decisions made, \
77 files touched, current state, open items. Be concise and factual.";
78
79 +/// Largest transcript (in estimated tokens, see [`estimate_tokens`]) sent to
80 +/// the model for summarization in one go.
81 +///
82 +/// Compaction runs because the conversation has outgrown the context budget,
83 +/// so shipping the whole transcript verbatim asks the endpoint to process
84 +/// exactly the payload that is already too big — on siGit Code Cloud that
85 +/// request sits in inference until the gateway gives up (504), which is the
86 +/// failure behind issue #125. Capping the transcript keeps the summarization
87 +/// round inside the window the model actually has; the oldest and newest
88 +/// exchanges carry the intent and the current state, while the truncated
89 +/// middle is the part a summary can afford to lose.
90 +const SUMMARY_TRANSCRIPT_TOKEN_CAP: usize = 12_000;
91 +
92 +/// Trim `transcript` to about `cap` estimated tokens by cutting whole lines
93 +/// from the middle, keeping the opening and closing exchanges.
94 +///
95 +/// A middle cut preserves the two ends that matter most to a summary: how the
96 +/// session started (the original request) and where it stands now. Cutting by
97 +/// whole lines keeps individual messages intact. A transcript already under
98 +/// the cap is returned unchanged.
99 +fn truncate_transcript_middle(transcript: &str, cap: usize) -> String {
100 + let budget = cap * 4; // estimate_tokens is chars / 4; invert it.
101 + if transcript.len() <= budget {
102 + return transcript.to_string();
103 + }
104 + // Split the budget between head and tail, then walk each inward to a line
105 + // boundary so no message is severed mid-line.
106 + let half = budget / 2;
107 + let mut head_end = half.min(transcript.len());
108 + while head_end < transcript.len() && !transcript.is_char_boundary(head_end) {
109 + head_end += 1;
110 + }
111 + let head_end = transcript[..head_end]
112 + .rfind('\n')
113 + .map(|index| index + 1)
114 + .unwrap_or(0);
115 +
116 + let mut tail_start = transcript.len().saturating_sub(half);
117 + while tail_start > 0 && !transcript.is_char_boundary(tail_start) {
118 + tail_start -= 1;
119 + }
120 + let tail_start = transcript[tail_start..]
121 + .find('\n')
122 + .map(|offset| tail_start + offset + 1)
123 + .unwrap_or(transcript.len());
124 +
125 + if head_end >= tail_start {
126 + // The cut points crossed (a transcript of very long lines); fall back
127 + // to a hard tail cut rather than duplicating content.
128 + let mut end = budget.min(transcript.len());
129 + while end > 0 && !transcript.is_char_boundary(end) {
130 + end -= 1;
131 + }
132 + return format!("{}\n[…older messages omitted…]", &transcript[..end]);
133 + }
134 +
135 + let omitted_chars = transcript[head_end..tail_start].chars().count();
136 + format!(
137 + "{}\n[…{} characters of older messages omitted…]\n{}",
138 + transcript[..head_end].trim_end(),
139 + omitted_chars,
140 + transcript[tail_start..].trim_start(),
141 + )
142 +}
143 +
144 /// Render a history snapshot as a plain-text transcript, with tool calls and
145 /// tool results spelled out as prose rather than left in their wire shapes.
146 ///
@@ -197,8 +262,11 @@ pub trait InferenceBackend: Send + Sync {
262
263 /// Shrink the conversation history: summarize everything so far with one
264 /// extra (non-streaming) inference round, then rebuild history as
200 - /// `[system message, summary, last keep_last non-system messages]`. On
201 - /// error the original history is left in place.
265 + /// `[system message, summary, last keep_last non-system messages]`. When
266 + /// the summarization round itself fails, a backend should fall back to a
267 + /// deterministic shrink (dropping the oldest messages) rather than leave
268 + /// an over-budget session stuck; an `Err` here means even that fallback
269 + /// could not fit the history in the budget.
270 async fn compact_history(&self, keep_last: usize) -> Result<(), BackendError>;
271 }
272
@@ -382,42 +450,107 @@ impl InferenceBackend for LocalBackend {
450 async fn compact_history(&self, keep_last: usize) -> Result<(), BackendError> {
451 self.apply_pending_history().await?;
452 let snapshot = self.engine.history().await;
385 - // One plain (tool-free) inference round produces the summary. On error
386 - // history is untouched — send_message only mutates it on success, and
387 - // whatever it appended is wiped by the clear below anyway.
388 - let result = self
389 - .engine
390 - .send_message(SUMMARIZE_PROMPT)
391 - .await
392 - .map_err(|error| error.to_string())?;
393 - // Local models may reason in <think> blocks; keep only the visible part.
394 - let (_think, summary) = crate::chat::strip_think_blocks(&result.text);
453
396 - self.engine.clear_history().await;
397 - // Leading system messages carry the session context; keep them all.
398 - for message in snapshot
399 - .iter()
400 - .take_while(|message| message.role == ChatRole::System)
401 - {
402 - self.engine.push_history(message.clone()).await;
403 - }
404 - self.engine
405 - .push_history(ChatMessage::user(format!(
406 - "[Conversation summary]\n{summary}"
407 - )))
408 - .await;
454 let non_system: Vec<&ChatMessage> = snapshot
455 .iter()
456 .filter(|message| message.role != ChatRole::System)
457 .collect();
458 let tail_start = non_system.len().saturating_sub(keep_last);
414 - for message in &non_system[tail_start..] {
415 - self.engine.push_history((*message).clone()).await;
459 + let tail: Vec<ChatMessage> = non_system[tail_start..]
460 + .iter()
461 + .map(|message| (*message).clone())
462 + .collect();
463 +
464 + // One plain (tool-free) inference round produces the summary. The
465 + // transcript is capped like the remote path's: an over-budget session
466 + // must not send its whole history back into the window that is already
467 + // too small for it (issue #125). send_message mutates engine history,
468 + // but the clear below wipes whatever it appended, so a failed round
469 + // leaves nothing behind.
470 + let transcript = truncate_transcript_middle(
471 + &chat_messages_transcript(&snapshot),
472 + SUMMARY_TRANSCRIPT_TOKEN_CAP,
473 + );
474 + let result = self
475 + .engine
476 + .send_message(format!("{transcript}\n\n{SUMMARIZE_PROMPT}"))
477 + .await
478 + .map_err(|error| error.to_string());
479 +
480 + let rebuild: Vec<ChatMessage> = match result {
481 + Ok(result) => {
482 + // Local models may reason in <think> blocks; keep only the visible part.
483 + let (_think, summary) = crate::chat::strip_think_blocks(&result.text);
484 + let mut rebuilt: Vec<ChatMessage> = snapshot
485 + .iter()
486 + .take_while(|message| message.role == ChatRole::System)
487 + .cloned()
488 + .collect();
489 + rebuilt.push(ChatMessage::user(format!(
490 + "[Conversation summary]\n{summary}"
491 + )));
492 + rebuilt.extend(tail);
493 + rebuilt
494 + }
495 + Err(error) => {
496 + log::warn!(
497 + "local summarization round failed ({error}); falling back to plain truncation"
498 + );
499 + let system_json: Option<serde_json::Value> = snapshot
500 + .iter()
501 + .find(|message| message.role == ChatRole::System)
502 + .map(|message| serde_json::json!({
503 + "role": message.role.to_string(),
504 + "content": message.content,
505 + }));
506 + let tail_json: Vec<serde_json::Value> = tail
507 + .iter()
508 + .map(|message| serde_json::json!({
509 + "role": message.role.to_string(),
510 + "content": message.content,
511 + }))
512 + .collect();
513 + let Some(fallback) =
514 + truncate_history_to_budget(system_json.as_ref(), tail_json)
515 + else {
516 + return Err(error);
517 + };
518 + fallback
519 + .iter()
520 + .filter_map(|entry| {
521 + let role = entry["role"].as_str().unwrap_or("");
522 + let content = entry["content"].as_str().unwrap_or("").to_string();
523 + match role {
524 + "system" => Some(ChatMessage::system(content)),
525 + "user" => Some(ChatMessage::user(content)),
526 + "assistant" => Some(ChatMessage::assistant(content)),
527 + _ => None,
528 + }
529 + })
530 + .collect()
531 + }
532 + };
533 +
534 + self.engine.clear_history().await;
535 + for message in rebuild {
536 + self.engine.push_history(message).await;
537 }
538 Ok(())
539 }
540 }
541
542 +/// Flatten `onde` chat messages into the transcript shape summarization
543 +/// expects (the local engine's history has no tool-call structure, so this is
544 +/// a plain role/content rendering).
545 +fn chat_messages_transcript(history: &[ChatMessage]) -> String {
546 + history
547 + .iter()
548 + .filter(|message| message.role != ChatRole::System && !message.content.trim().is_empty())
549 + .map(|message| format!("{}: {}", message.role, message.content))
550 + .collect::<Vec<_>>()
551 + .join("\n\n")
552 +}
553 +
554 /// Drain an onde streaming receiver, forwarding each token to `sink` and
555 /// assembling the full text. onde reports stream failures as a final chunk whose
556 /// `finish_reason` is `"error: …"`; surface those as a backend error.
@@ -622,6 +755,62 @@ impl OpenAiBackend {
755 }
756 }
757
758 + /// One self-contained summarization round: summarize `transcript` without
759 + /// touching the live history.
760 + ///
761 + /// Unlike [`complete`], which replays and mutates `self.history`, this
762 + /// sends its own one-off message list. Compaction only rewrites history
763 + /// once it has a summary in hand, so a failed round leaves the session
764 + /// exactly as it was — and the caller can fall back to plain truncation
765 + /// instead of failing the session (issue #125).
766 + async fn summarize(
767 + &self,
768 + system: Option<&serde_json::Value>,
769 + transcript: &str,
770 + ) -> Result<String, BackendError> {
771 + let url = format!("{}/chat/completions", self.base_url.trim_end_matches('/'));
772 + let mut messages = Vec::with_capacity(2);
773 + if let Some(system) = system {
774 + messages.push(system.clone());
775 + }
776 + messages.push(serde_json::json!({
777 + "role": "user",
778 + "content": format!("{transcript}\n\n{SUMMARIZE_PROMPT}"),
779 + }));
780 + let body = serde_json::json!({
781 + "model": self.model,
782 + "messages": messages,
783 + "stream": false,
784 + });
785 +
786 + let response = self
787 + .http
788 + .post(&url)
789 + .bearer_auth(&self.api_key)
790 + .json(&body)
791 + .send()
792 + .await
793 + .map_err(|error| format!("request to {url} failed: {error}"))?;
794 +
795 + if !response.status().is_success() {
796 + let status = response.status();
797 + let body = response.text().await.unwrap_or_default();
798 + return Err(describe_api_error(status, &body));
799 + }
800 +
801 + let parsed: ChatCompletion = response
802 + .json()
803 + .await
804 + .map_err(|error| format!("response parse error: {error}"))?;
805 + parsed
806 + .choices
807 + .into_iter()
808 + .next()
809 + .and_then(|choice| choice.message.content)
810 + .filter(|text| !text.trim().is_empty())
811 + .ok_or_else(|| "endpoint returned no summary".to_string())
812 + }
813 +
814 /// Parse a single non-streaming chat-completion response.
815 async fn consume_json(
816 &self,
@@ -1120,34 +1309,6 @@ impl InferenceBackend for OpenAiBackend {
1309 .filter(|message| message["role"] == "system")
1310 .cloned();
1311
1123 - // Ask the endpoint for a summary of the conversation so far, through the
1124 - // ordinary completion machinery (non-streaming). The request carries the
1125 - // conversation as a flattened transcript in a single user message rather
1126 - // than the live history: this round offers no tools, and a tool-shaped
1127 - // history sent without a tool schema is rejected upstream (see
1128 - // `transcript_for_summary`).
1129 - let mut request = Vec::new();
1130 - if let Some(system) = system.clone() {
1131 - request.push(system);
1132 - }
1133 - request.push(serde_json::json!({
1134 - "role": "user",
1135 - "content": format!(
1136 - "{}\n\n{SUMMARIZE_PROMPT}",
1137 - transcript_for_summary(&snapshot),
1138 - ),
1139 - }));
1140 - *self.history.lock().await = request;
1141 -
1142 - let summary = match self.complete(&[], false, None).await {
1143 - Ok(result) => result.text,
1144 - Err(error) => {
1145 - // Roll back the summarization request; the turn never happened.
1146 - *self.history.lock().await = snapshot;
1147 - return Err(error);
1148 - }
1149 - };
1150 -
1312 let non_system: Vec<serde_json::Value> = snapshot
1313 .iter()
1314 .filter(|message| message["role"] != "system")
@@ -1165,6 +1326,35 @@ impl InferenceBackend for OpenAiBackend {
1326 tail.remove(0);
1327 }
1328
1329 + // Ask the endpoint for a summary of the conversation so far, through a
1330 + // direct completion call. The request carries the conversation as a
1331 + // flattened transcript in a single user message rather than the live
1332 + // history: this round offers no tools, and a tool-shaped history sent
1333 + // without a tool schema is rejected upstream (see
1334 + // `transcript_for_summary`). The transcript is capped so the request
1335 + // cannot be as large as the over-budget conversation that triggered
1336 + // compaction (see `SUMMARY_TRANSCRIPT_TOKEN_CAP`).
1337 + let transcript = truncate_transcript_middle(
1338 + &transcript_for_summary(&snapshot),
1339 + SUMMARY_TRANSCRIPT_TOKEN_CAP,
1340 + );
1341 + let summary = match self.summarize(system.as_ref(), &transcript).await {
1342 + Ok(summary) => summary,
1343 + Err(error) => {
1344 + log::warn!(
1345 + "summarization round failed ({error}); falling back to plain truncation"
1346 + );
1347 + // Compaction must not fail the session: fold the overflow into
1348 + // a placeholder and drop the oldest messages until the rebuild
1349 + // fits the budget. Needs no endpoint, so it cannot fail the way
1350 + // the inference round just did (issue #125).
1351 + let rebuilt = truncate_history_to_budget(system.as_ref(), tail.clone())
1352 + .ok_or(error)?;
1353 + *self.history.lock().await = rebuilt;
1354 + return Ok(());
1355 + }
1356 + };
1357 +
1358 let mut rebuilt = Vec::new();
1359 if let Some(system) = system {
1360 rebuilt.push(system);
@@ -1179,6 +1369,74 @@ impl InferenceBackend for OpenAiBackend {
1369 }
1370 }
1371
1372 +/// The placeholder message replacing dropped history when compaction falls
1373 +/// back to plain truncation (the model reads it, so it explains the gap).
1374 +const TRUNCATION_PLACEHOLDER: &str = "[Older conversation omitted: the session outgrew \
1375 + the context window and the summarization service was unavailable. Earlier \
1376 + messages were dropped, so answers may lack that context.]";
1377 +
1378 +/// Last-resort compaction when the summarization round cannot run: rebuild the
1379 +/// history as `[system, placeholder, newest messages that fit the budget]`.
1380 +///
1381 +/// Returns `None` when even the smallest rebuild exceeds the budget (a
1382 +/// pathological case — e.g. one message larger than the whole window); the
1383 +/// caller then reports the original summarization error and leaves history
1384 +/// untouched.
1385 +fn truncate_history_to_budget(
1386 + system: Option<&serde_json::Value>,
1387 + mut tail: Vec<serde_json::Value>,
1388 +) -> Option<Vec<serde_json::Value>> {
1389 + /// A `role: "tool"` entry only makes sense behind the assistant message
1390 + /// that requested it; dropping that message strands it.
1391 + fn drop_orphaned_tool_results(tail: &mut Vec<serde_json::Value>) {
1392 + while tail.first().is_some_and(|message| message["role"] == "tool") {
1393 + tail.remove(0);
1394 + }
1395 + }
1396 +
1397 + // A caller splitting mid-round can hand in a tail that starts with tool
1398 + // results whose call message already fell outside the kept window.
1399 + drop_orphaned_tool_results(&mut tail);
1400 +
1401 + while !tail.is_empty() {
1402 + let mut candidate = Vec::with_capacity(tail.len() + 2);
1403 + if let Some(system) = system {
1404 + candidate.push(system.clone());
1405 + }
1406 + candidate.push(serde_json::json!({
1407 + "role": "user",
1408 + "content": TRUNCATION_PLACEHOLDER,
1409 + }));
1410 + candidate.extend(tail.iter().cloned());
1411 + if estimate_tokens(&candidate) <= DEFAULT_CONTEXT_TOKEN_BUDGET {
1412 + return Some(candidate);
1413 + }
1414 +
1415 + if tail.len() == 1 {
1416 + // One message and it already overflows: nothing left to drop.
1417 + return None;
1418 + }
1419 + if tail[0]["role"] == "assistant"
1420 + && tail[0]["tool_calls"]
1421 + .as_array()
1422 + .is_some_and(|calls| !calls.is_empty())
1423 + && tail.get(1).is_some_and(|message| message["role"] == "tool")
1424 + {
1425 + // Dropping an assistant tool-call message would orphan its
1426 + // results; drop the whole round (call + results) together.
1427 + let mut end = 1;
1428 + while tail.get(end).is_some_and(|message| message["role"] == "tool") {
1429 + end += 1;
1430 + }
1431 + tail.drain(..end);
1432 + } else {
1433 + tail.remove(0);
1434 + drop_orphaned_tool_results(&mut tail);
1435 + }
1436 + }
1437 + None
1438 +}
1439 +
1440 // ── OpenAI error shape ────────────────────────────────────────────────────────
1441
1442 /// The OpenAI error envelope: `{"error": {"message": ..., "type": ...}}`.
@@ -1208,14 +1466,16 @@ const ERROR_BODY_LIMIT: usize = 500;
1466 /// The endpoint's own message wins when there is one: it is written for the
1467 /// user, and the status code repeats what it already says. Anything else falls
1468 /// back to the status plus whatever the body held, which is all there is to go
1211 -/// on.
1469 +/// on — except an HTML page, which is a proxy or Rails error page and tells
1470 +/// the person nothing the status doesn't (pasting it into chat is the dump
1471 +/// seen in issue #125's screenshots).
1472 fn describe_api_error(status: reqwest::StatusCode, body: &str) -> String {
1473 if let Some(message) = api_error_message(body) {
1474 return message;
1475 }
1476
1477 let body = body.trim();
1218 - if body.is_empty() {
1478 + if body.is_empty() || is_probable_html(body) {
1479 return format!("endpoint returned {status}");
1480 }
1481
@@ -1240,6 +1500,15 @@ fn api_error_message(body: &str) -> Option<String> {
1500 (!message.is_empty()).then(|| message.to_string())
1501 }
1502
1503 +/// Whether `body` looks like an HTML page rather than prose: a doctype or an
1504 +/// opening tag in the first chunk. Gateways and Rails apps answer failures
1505 +/// with full pages; none of that markup is worth showing.
1506 +fn is_probable_html(body: &str) -> bool {
1507 + let head: String = body.chars().take(512).collect();
1508 + let head = head.trim_start().to_ascii_lowercase();
1509 + head.starts_with("<!doctype") || head.starts_with("<html") || head.starts_with("<head")
1510 +}
1511 +
1512 // ── OpenAI response shapes ────────────────────────────────────────────────────────
1513
1514 #[derive(Debug, Deserialize)]
@@ -1458,10 +1727,23 @@ mod tests {
1727 #[test]
1728 fn a_body_that_is_not_an_error_envelope_keeps_the_status() {
1729 let described =
1461 - describe_api_error(reqwest::StatusCode::BAD_GATEWAY, "<html>bad gateway</html>");
1730 + describe_api_error(reqwest::StatusCode::BAD_GATEWAY, "upstream said no");
1731
1732 assert!(described.contains("502"), "{described}");
1464 - assert!(described.contains("bad gateway"), "{described}");
1733 + assert!(described.contains("upstream said no"), "{described}");
1734 + }
1735 +
1736 + /// The issue-#125 screenshots: a gateway's HTML error page pasted into
1737 + /// chat tells the person nothing the status doesn't already say.
1738 + #[test]
1739 + fn an_html_error_page_is_not_pasted_into_the_message() {
1740 + let page = "<!doctype html>\n<html lang=\"en\">\n<head>\n\
1741 + <title>We're sorry, but something went wrong (500 Internal Server Error)</title>\n\
1742 + </head>\n<body>…</body>\n</html>";
1743 +
1744 + let described = describe_api_error(reqwest::StatusCode::GATEWAY_TIMEOUT, page);
1745 +
1746 + assert_eq!(described, "endpoint returned 504 Gateway Timeout");
1747 }
1748
1749 #[test]
@@ -2044,23 +2326,147 @@ mod tests {
2326 assert!(transcript.contains("tool result: on branch main"));
2327 }
2328
2329 + /// Compaction only fails when even the truncation fallback cannot fit the
2330 + /// history in the budget — here the newest message alone is bigger than
2331 + /// the whole window.
2332 #[tokio::test]
2333 async fn compact_history_failure_leaves_history_intact() {
2049 - // No listener at this address: the summarization request fails, and
2050 - // history must roll back to exactly what it was.
2334 + // No listener at this address: the summarization request fails.
2335 let backend =
2336 OpenAiBackend::new("http://127.0.0.1:9", "", "test-model", Some("sys".into()));
2053 - backend
2054 - .history
2055 - .lock()
2056 - .await
2057 - .push(serde_json::json!({ "role": "user", "content": "hello" }));
2337 + backend.history.lock().await.push(serde_json::json!({
2338 + "role": "user",
2339 + "content": "x".repeat(DEFAULT_CONTEXT_TOKEN_BUDGET * 4 + 1000),
2340 + }));
2341 let before = backend.history_snapshot().await;
2342
2343 assert!(backend.compact_history(2).await.is_err());
2344 assert_eq!(backend.history_snapshot().await, before);
2345 }
2346
2347 + /// The issue-#125 path: the summarization endpoint is down, and the
2348 + /// session must still shrink rather than die with "start a new thread".
2349 + #[tokio::test]
2350 + async fn failed_summarization_falls_back_to_truncation() {
2351 + // Nothing listens here, so the summarization round always fails.
2352 + let backend =
2353 + OpenAiBackend::new("http://127.0.0.1:9", "", "test-model", Some("sys".into()));
2354 + {
2355 + let mut history = backend.history.lock().await;
2356 + for i in 0..20 {
2357 + history.push(serde_json::json!({
2358 + "role": "user",
2359 + "content": format!("message {i}: {}", "padding ".repeat(200)),
2360 + }));
2361 + }
2362 + }
2363 +
2364 + backend.compact_history(4).await.unwrap();
2365 +
2366 + let history = backend.history_snapshot().await;
2367 + assert_eq!(history[0]["role"], "system", "{history:?}");
2368 + assert_eq!(history[1]["content"], TRUNCATION_PLACEHOLDER, "{history:?}");
2369 + // The newest messages survive; the oldest were dropped to fit.
2370 + assert_eq!(
2371 + history.last().unwrap()["content"]
2372 + .as_str()
2373 + .unwrap()
2374 + .split(':')
2375 + .next()
2376 + .unwrap(),
2377 + "message 19",
2378 + "{history:?}"
2379 + );
2380 + assert!(
2381 + estimate_tokens(&history) <= DEFAULT_CONTEXT_TOKEN_BUDGET,
2382 + "fallback result must fit the budget: ~{} tokens",
2383 + estimate_tokens(&history)
2384 + );
2385 + }
2386 +
2387 + /// A history small enough to begin with keeps its whole tail: the
2388 + /// fallback drops nothing beyond what `keep_last` already folded away.
2389 + #[tokio::test]
2390 + async fn truncation_fallback_keeps_the_newest_messages_that_fit() {
2391 + let system = serde_json::json!({ "role": "system", "content": "sys" });
2392 + let tail: Vec<serde_json::Value> = (0..4)
2393 + .map(|i| serde_json::json!({ "role": "user", "content": format!("m{i}") }))
2394 + .collect();
2395 +
2396 + let rebuilt = truncate_history_to_budget(Some(&system), tail).unwrap();
2397 +
2398 + assert_eq!(rebuilt[0]["role"], "system");
2399 + assert_eq!(rebuilt[1]["content"], TRUNCATION_PLACEHOLDER);
2400 + assert_eq!(rebuilt.len(), 6, "{rebuilt:?}");
2401 + assert_eq!(rebuilt[2]["content"], "m0");
2402 + assert_eq!(rebuilt[5]["content"], "m3");
2403 + }
2404 +
2405 + #[test]
2406 + fn truncation_fallback_drops_orphaned_tool_results_with_their_call() {
2407 + // A caller that split mid-round hands in a tail starting with tool
2408 + // results whose assistant call message already fell outside the kept
2409 + // window; those orphans must go before anything else, or strict
2410 + // endpoints reject the next request.
2411 + let tail = vec![
2412 + serde_json::json!({ "role": "tool", "tool_call_id": "call_1", "content": "ok" }),
2413 + serde_json::json!({ "role": "user", "content": "thanks" }),
2414 + serde_json::json!({ "role": "assistant", "content": "done" }),
2415 + ];
2416 +
2417 + let rebuilt = truncate_history_to_budget(None, tail).unwrap();
2418 +
2419 + let roles: Vec<&str> = rebuilt
2420 + .iter()
2421 + .filter_map(|m| m["role"].as_str())
2422 + .collect();
2423 + assert_eq!(
2424 + roles,
2425 + ["user", "user", "assistant"],
2426 + "placeholder + the surviving exchange, orphan gone: {rebuilt:?}"
2427 + );
2428 + }
2429 +
2430 + #[test]
2431 + fn truncation_fallback_reports_when_nothing_fits() {
2432 + // A single message bigger than the whole window can never fit.
2433 + let tail = vec![serde_json::json!({
2434 + "role": "user",
2435 + "content": "x".repeat(DEFAULT_CONTEXT_TOKEN_BUDGET * 4 + 1000),
2436 + })];
2437 +
2438 + assert!(truncate_history_to_budget(None, tail).is_none());
2439 + }
2440 +
2441 + #[test]
2442 + fn truncate_transcript_middle_keeps_short_transcripts_verbatim() {
2443 + let transcript = "user: hello\n\nassistant: hi";
2444 +
2445 + assert_eq!(truncate_transcript_middle(transcript, 1_000), transcript);
2446 + }
2447 +
2448 + #[test]
2449 + fn truncate_transcript_middle_cuts_the_middle_not_the_ends() {
2450 + let mut lines: Vec<String> = Vec::new();
2451 + for i in 0..200 {
2452 + lines.push(format!("user: message {i} with some padding text"));
2453 + }
2454 + let transcript = lines.join("\n");
2455 +
2456 + let trimmed = truncate_transcript_middle(&transcript, 200); // ~800 chars
2457 +
2458 + assert!(trimmed.contains("message 0"), "opening kept: {trimmed}");
2459 + assert!(trimmed.contains("message 199"), "closing kept: {trimmed}");
2460 + assert!(
2461 + trimmed.contains("older messages omitted"),
2462 + "gap is marked: {trimmed}"
2463 + );
2464 + assert!(
2465 + trimmed.len() <= transcript.len(),
2466 + "never grows the transcript"
2467 + );
2468 + }
2469 +
2470 #[test]
2471 fn assistant_message_with_tool_calls_round_trips() {
2472 let message = ResponseMessage {
src/main.rs
+6 -2
@@ -2296,13 +2296,17 @@ impl SiGitAgent {
2296 log::info!("prompt({}) compacted to ≈{} tokens", session_id, after);
2297 }
2298 Err(error) => {
2299 + // Even the deterministic fallback could not fit the
2300 + // history in the budget — the session is genuinely
2301 + // stuck, so say what is actually recoverable.
2302 log::warn!("prompt({}) compaction failed: {error}", session_id);
2303 self.send_assistant_message(
2304 cx,
2305 session_id.clone(),
2306 format!(
2304 - "This session is too large, and siGit Code could not compact it: \
2305 - {error}. Start a new thread or run `/clear`, then retry."
2307 + "This session is too large for the context window, and \
2308 + siGit Code could not shrink it ({error}). Run `/clear` \
2309 + or start a new thread to continue."
2310 ),
2311 )
2312 .ok();
tests/acp_endpoint_errors.rs
+14 -11
@@ -308,14 +308,17 @@ fn an_error_frame_mid_stream_is_not_swallowed() {
308 std::fs::remove_dir_all(&dir).ok();
309 }
310
311 +/// A failing summarization round must not strand the session (issue #125):
312 +/// compaction falls back to plain truncation, the pending tool round runs,
313 +/// and the turn completes instead of ending with "start a new thread".
314 #[test]
312 -fn failed_compaction_in_an_oversized_acp_turn_is_reported_as_chat_text() {
315 +fn failed_compaction_falls_back_to_truncation_and_the_turn_continues() {
316 let dir = scratch("compact");
317 let port = start_fake_endpoint(vec![
318 sse_tool_call(
319 "call_1",
320 "run_command",
318 - r#"{"command":"echo should-not-run"}"#,
321 + r#"{"command":"echo truncation-fallback-works"}"#,
322 ),
323 upstream_500_reply(),
324 ]);
@@ -331,10 +334,16 @@ fn failed_compaction_in_an_oversized_acp_turn_is_reported_as_chat_text() {
334
335 assert!(
336 response.get("error").is_none(),
334 - "compaction failure should be a readable chat message, not an ACP error: {response}"
337 + "truncation fallback keeps the turn alive: {response}"
338 );
339 assert_eq!(response["result"]["stopReason"], "end_turn");
340
341 + assert!(
342 + updates
343 + .iter()
344 + .any(|update| update["sessionUpdate"] == "tool_call"),
345 + "the pending tool call should run after the truncation fallback: {updates:?}"
346 + );
347 let rendered = updates
348 .iter()
349 .filter_map(|update| {
@@ -344,14 +353,8 @@ fn failed_compaction_in_an_oversized_acp_turn_is_reported_as_chat_text() {
353 })
354 .collect::<String>();
355 assert!(
347 - rendered.contains("could not compact it: Onde Cloud upstream error (500)"),
348 - "wrong rendered message: {rendered:?}"
349 - );
350 - assert!(
351 - !updates
352 - .iter()
353 - .any(|update| update["sessionUpdate"] == "tool_call"),
354 - "tools should not run after compaction fails: {updates:?}"
356 + !rendered.contains("could not"),
357 + "no compaction-failure banner should be shown when the fallback succeeded: {rendered:?}"
358 );
359
360 std::fs::remove_dir_all(&dir).ok();