diff --git a/src/app/runtime/actions/mod.rs b/src/app/runtime/actions/mod.rs index 46bf8d8..f1e91eb 100644 --- a/src/app/runtime/actions/mod.rs +++ b/src/app/runtime/actions/mod.rs @@ -545,21 +545,44 @@ pub fn apply_action(state: &mut AppStateRest, action: Action) { state.push_toast(Toast::new(ToastKind::Warning, "Aborting generation...".to_string())); } Action::Compact => { - let max_wire_tokens = state.app_config.model_roles.values() - .find(|role| role.provider == state.settings.provider && role.model == state.settings.model) - .and_then(|role| role.context_window) - .unwrap_or(state.app_config.default_context_window) as usize; - - if let Some(ref mut rt) = state.session_runtime { - let total_chars: usize = rt.messages.iter() - .filter_map(|m| m.content.as_deref()) - .map(str::len) - .sum(); - let token_estimate = total_chars / 3; - rt.messages = crate::app::runtime::context::shaping::shape_messages(&rt.messages, token_estimate, max_wire_tokens, true, None); - state.push_toast(Toast::new(ToastKind::Success, "Conversation history compacted.".to_string())); - state.dirty = true; + let Some(messages) = state.session_runtime.as_ref().map(|rt| rt.messages.clone()) else { + return; + }; + if messages.is_empty() { + return; } + let (api_key, model, base_url) = match resolve_llm_client_config(state) { + Ok(v) => v, + Err(msg) => { + state.push_toast(Toast::new(ToastKind::Error, msg)); + return; + } + }; + let max_wire_tokens = crate::app::runtime::context::window::resolve(&state.app_config, &state.settings); + let turn_events = state.turn_events.clone(); + + state.push_toast(Toast::new(ToastKind::Info, "Compacting conversation history...".to_string())); + + // Manual /compact previously ran synchronously and always + // passed `client: None` to shape_messages, so it never got + // LLM summarization — only automatic mid-turn compaction did. + // Running this on a background thread (same pattern as + // spawn_turn) fixes that asymmetry: both paths now summarize + // dropped history with the LLM instead of one silently + // falling back to a bare placeholder. + std::thread::spawn(move || { + let client = crate::service::provider::LlmClient::new(api_key, model, base_url); + let (deduped, _) = crate::app::runtime::context::dedup::collapse(&messages); + let token_count: usize = deduped.iter() + .map(crate::app::runtime::context::tokens::count_message_tokens) + .sum(); + let compacted = crate::app::runtime::context::shaping::shape_messages( + &deduped, token_count, max_wire_tokens, true, Some(&client), + ); + if let Ok(mut q) = turn_events.lock() { + q.push_back(TurnEvent::Compacted(compacted)); + } + }); } Action::LessonAccept { name } => { if let Some(ref rt) = state.session_runtime { @@ -600,6 +623,45 @@ pub fn apply_action(state: &mut AppStateRest, action: Action) { } } +/// Resolve the API key, model name, and base URL for the currently +/// configured provider. +/// +/// Flow: look up the provider's `ProviderConfig` for its `api_base` -> +/// resolve the API key from `Settings.api_keys`, falling back to the +/// provider's `api_key_env` environment variable, then its +/// `default_api_key`, then the crate-wide empty-string default. +/// +/// Why: this exact resolution was duplicated between `spawn_turn` and +/// needed again for `Action::Compact`'s background-thread LLM call — +/// factored out so both stay in sync. +/// +/// Return: `Ok((api_key, model, base_url))`, or `Err(message)` — a +/// user-facing string — if the configured provider has no entry in +/// `AppConfig.providers` at all. +fn resolve_llm_client_config(state: &AppStateRest) -> Result<(String, String, Option), String> { + let base_url = state.app_config.providers.get(&state.settings.provider).map(|p| p.api_base.clone()); + let Some(base_url) = base_url else { + return Err(format!( + "Provider '{}' is not configured — no matching entry found. \ + Pick a different provider in Settings, or configure it.", + state.settings.provider + )); + }; + let mut api_key = state.settings.api_keys.get(&state.settings.provider).cloned().unwrap_or_default(); + if api_key.is_empty() { + if let Some(provider_cfg) = state.app_config.providers.get(&state.settings.provider) { + api_key = provider_cfg.api_key_env.as_ref() + .and_then(|env| std::env::var(env).ok()) + .or_else(|| provider_cfg.default_api_key.clone()) + .unwrap_or_default(); + } + } + if api_key.is_empty() { + api_key = crate::service::provider::DEFAULT_API_KEY.to_string(); + } + Ok((api_key, state.settings.model.clone(), Some(base_url))) +} + /// Spawn a background thread that runs one full LLM turn. /// /// Flow: check that no turn is currently in-flight → bail if so → @@ -630,42 +692,16 @@ fn spawn_turn(state: &AppStateRest) { if messages.is_empty() { return; } - let mut api_key = state.settings.api_keys.get(&state.settings.provider).cloned().unwrap_or_default(); - let model = state.settings.model.clone(); - let base_url = state.app_config.providers.get(&state.settings.provider) - .map(|p| p.api_base.clone()); - let context_window = state.app_config.model_roles.values() - .find(|role| role.provider == state.settings.provider && role.model == state.settings.model) - .and_then(|role| role.context_window) - .unwrap_or(state.app_config.default_context_window) as usize; - // The selected provider has no entry in app_config at all (e.g. the - // Claude-settings auto-detection that registers "claude" found nothing - // this run). Without this check, LlmClient::new silently falls back to - // the zen default base URL while keeping this provider's model name — - // a mismatched request that reaches a real server and comes back as a - // confusing "Missing API key" 401 from an unrelated provider, instead - // of the actual problem: the configured provider doesn't exist. - if base_url.is_none() { - if let Ok(mut q) = state.turn_events.lock() { - q.push_back(TurnEvent::Error(format!( - "Provider '{}' is not configured — no matching entry found. \ - Pick a different provider in Settings, or configure it.", - state.settings.provider - ))); + let (api_key, model, base_url) = match resolve_llm_client_config(state) { + Ok(v) => v, + Err(msg) => { + if let Ok(mut q) = state.turn_events.lock() { + q.push_back(TurnEvent::Error(msg)); + } + return; } - return; - } - if api_key.is_empty() { - if let Some(provider_cfg) = state.app_config.providers.get(&state.settings.provider) { - api_key = provider_cfg.api_key_env.as_ref() - .and_then(|env| std::env::var(env).ok()) - .or_else(|| provider_cfg.default_api_key.clone()) - .unwrap_or_default(); - } - } - if api_key.is_empty() { - api_key = crate::service::provider::DEFAULT_API_KEY.to_string(); - } + }; + let context_window = crate::app::runtime::context::window::resolve(&state.app_config, &state.settings); let (temperature, max_tokens) = crate::app::mode::effort::generation_params( state.misc.effort_level, state.settings.max_tokens, @@ -1144,33 +1180,43 @@ fn run_agent_turn( let mut todo_retry_count = 0usize; loop { - let total_chars: usize = msgs.iter() - .filter_map(|m| m.content.as_deref()) - .map(str::len) + // Dedup runs every iteration, unconditionally — repeated + // read-only tool calls (same tool + same arguments) are + // collapsed to their latest result before anything else, so + // context stays minimal from turn 1 instead of only shrinking + // once shaping's budget threshold trips. + let (deduped, dedup_changed) = crate::app::runtime::context::dedup::collapse(&msgs); + let token_count: usize = deduped.iter() + .map(crate::app::runtime::context::tokens::count_message_tokens) .sum(); - let token_estimate = total_chars / 4; let max_wire_tokens = tc.context_window; - // Skip message compaction if abort was requested — the non-streaming - // LLM call for summarization would block without checking abort_flag. + // Skip shaping if abort was requested — the non-streaming LLM + // call for summarization would block without checking abort_flag. let wire_msgs = if !tc.abort_flag.load(std::sync::atomic::Ordering::SeqCst) - && crate::app::runtime::context::shaping::should_shape(token_estimate, max_wire_tokens, prev_shaped) + && crate::app::runtime::context::shaping::should_shape(token_count, max_wire_tokens, prev_shaped) { prev_shaped = true; - let compacted = crate::app::runtime::context::shaping::shape_messages(&msgs, token_estimate, max_wire_tokens, false, Some(&tc.client)); - - // Dispatch the compacted messages to the main thread so the local session history - // is permanently compacted and doesn't trigger shaping again immediately on next turn. + let compacted = crate::app::runtime::context::shaping::shape_messages(&deduped, token_count, max_wire_tokens, false, Some(&tc.client)); + + // Dispatch to the main thread so the local session history is + // permanently updated and doesn't re-trigger shaping immediately + // on the next turn. if let Ok(mut q) = events_q.lock() { q.push_back(TurnEvent::Compacted(compacted.clone())); } - - // Also update our local `msgs` variable so the rest of the loop operates on the compacted version + msgs.clone_from(&compacted); compacted } else { prev_shaped = false; - msgs.clone() + if dedup_changed { + if let Ok(mut q) = events_q.lock() { + q.push_back(TurnEvent::Compacted(deduped.clone())); + } + msgs.clone_from(&deduped); + } + deduped }; let mut stream_started = false; diff --git a/src/app/runtime/shortsend.rs b/src/app/runtime/shortsend.rs deleted file mode 100644 index d2bd469..0000000 --- a/src/app/runtime/shortsend.rs +++ /dev/null @@ -1,129 +0,0 @@ -#![allow(clippy::cast_possible_truncation, clippy::cast_sign_loss, clippy::cast_precision_loss, clippy::cast_possible_wrap)] -//! Short-send / message shaping: compacts long conversation histories so -//! they fit within the provider's context window before being sent to the -//! LLM API. -use crate::dto::chat::message::ChatMessage; - -/// Decide whether the message list should be shaped (compacted) before -/// sending to the LLM. -/// -/// Flow: trigger based on token estimate. If `token_estimate` exceeds -/// the threshold, we shape. When `prev_shaped` is true, the threshold is -/// raised (95%) to avoid fluttering — compaction only re-triggers when -/// the context is genuinely full again. When `prev_shaped` is false, the -/// threshold is lower (85%) so compaction starts proactively. -/// -/// Why: hysteresis prevents repeated compaction on every turn when the -/// token count hovers near the boundary. -/// -/// Return: `true` if shaping should be applied. -pub fn should_shape(token_estimate: usize, max_wire_tokens: usize, prev_shaped: bool) -> bool { - let threshold = if prev_shaped { - // Higher threshold when already shaped — defer re-shaping until - // the buffer is genuinely full again (95%). - (max_wire_tokens as f32 * 0.95) as usize - } else { - // Lower threshold when not yet shaped — trigger shaping sooner - // (85%) to avoid hitting the context window limit. - (max_wire_tokens as f32 * 0.85) as usize - }; - token_estimate >= threshold -} - -/// Compact a long message list by dropping middle messages and inserting -/// a summary placeholder. -/// -/// Flow: if the estimated token count is within budget and not forced, return messages -/// unchanged → otherwise keep the system message and the most recent -/// messages (up to `MAX_WIRE_TOKENS / 200` of them) with a `[prior -/// conversation compacted]` system message in between. -/// -/// Why: keeps context-size overhead roughly constant regardless of -/// session length. -/// -pub fn shape_messages( - messages: &[ChatMessage], - token_count: usize, - max_wire_tokens: usize, - force: bool, - client: Option<&crate::service::provider::LlmClient>, -) -> Vec { - if !force && (token_count <= max_wire_tokens || messages.len() < 5) { - return messages.to_vec(); - } - - let target_tokens = (max_wire_tokens as f32 * 0.70) as usize; - let mut current_tokens = 0; - let mut keep_recent = Vec::new(); - let mut dropped_msgs = Vec::new(); - - // Always keep the very first message (System Prompt) which we don't count here - // as we just blindly preserve it later. - let mut msgs_to_eval = messages.to_vec(); - let first = if msgs_to_eval.is_empty() { - None - } else { - Some(msgs_to_eval.remove(0)) - }; - - // Iterate backwards from the most recent to oldest - for m in msgs_to_eval.into_iter().rev() { - let text = m.content.as_deref().unwrap_or(""); - // Estimate tokens: ~1 token per 3 bytes for mixed content (code, - // prose, multi-byte). Conservative enough to stay under provider - // limits while avoiding premature compaction. - let msg_tokens = text.len() / 3; - - if current_tokens + msg_tokens <= target_tokens { - current_tokens += msg_tokens; - keep_recent.push(m); - } else { - dropped_msgs.push(m); // These will end up in reverse chronological order - } - } - - // Reverse dropped_msgs so they are back in chronological order - dropped_msgs.reverse(); - - let mut result = Vec::new(); - if let Some(f) = first { - result.push(f); - } - - if !dropped_msgs.is_empty() { - let mut summary_text = "[prior conversation compacted]".to_string(); - - if let Some(llm) = client { - let prompt = format!( - "Summarize the following dropped conversation history briefly. Focus on main goals, decisions made, and files modified, so the context is preserved for future turns. Keep it concise.\n\nHistory:\n{}", - dropped_msgs.iter() - .map(|m| format!("[{}]: {}", if m.role == crate::dto::chat::message::Role::User { "User" } else { "Assistant" }, m.content.as_deref().unwrap_or(""))) - .collect::>() - .join("\n\n") - ); - - let req_msgs = vec![ChatMessage::user(prompt)]; - match llm.chat_with_tools_non_streaming(&req_msgs, None) { - Ok(resp) => { - if let Some(content) = resp.0.content { - summary_text = format!("[Summary of compacted prior conversation:\n{content}\n]"); - } - } - Err(e) => { - tracing::warn!( - "[shortsend] LLM summarization failed: {}. \ - Prior conversation history is lost — no summary available. \ - This means the model will lose context about earlier parts of \ - the conversation.", - e, - ); - } - } - } - - result.push(ChatMessage::system(summary_text)); - } - - result.extend(keep_recent.into_iter().rev()); - result -}