recursive/compact/

mod.rs

//! Context compaction.
//!
//! Two layers:
//!
//! * [`Compactor`] — LLM-driven summarisation of older transcript messages
//!   into a single system message. Disabled by default (threshold =
//!   `usize::MAX`), enabled via `AgentRuntimeBuilder::compactor(...)`.
//!
//! * [`Microcompactor`] — no-LLM proactive pruning of old tool results by
//!   count. Runs before the LLM-driven compactor each step so the transcript
//!   may drop below the compaction threshold and the expensive LLM summary
//!   is skipped entirely.

pub mod micro;
pub mod prompt;
pub mod reinject;
pub mod retry;

pub use micro::{Microcompactor, MICROCOMPACT_PLACEHOLDER};
pub use reinject::{
    build_skill_reinjector_from_env, FileReinjector, PlanTodoReinjector, SkillReinjector,
};
pub use retry::{truncate_head_for_retry, MAX_PTL_RETRIES};

/// Stop attempting proactive compaction after this many consecutive
/// failures. Emergency compaction (`compact_on_overflow`) is exempt —
/// it is the last-resort recovery for an already-failed turn.
pub const MAX_CONSECUTIVE_COMPACT_FAILURES: u32 = 3;

use crate::error::Result;
use crate::llm::{ChatProvider, StructuredRequest, ToolSpec};
use crate::message::Message;

/// Configuration for LLM-driven transcript compaction.
#[derive(Debug, Clone)]
pub struct Compactor {
    /// Character-count threshold above which compaction is triggered.
    /// Defaults to `usize::MAX` (disabled).
    ///
    /// This estimate is reliable for English content (~4 chars/token) but
    /// underestimates token density in CJK languages. When actual
    /// `prompt_tokens` from the API are available, `threshold_prompt_tokens`
    /// is checked first and takes priority.
    pub threshold_chars: usize,
    /// Token-count threshold: if the last turn's `prompt_tokens` from the
    /// API response meets or exceeds this value, compaction is triggered.
    ///
    /// When `Some` and the last prompt_tokens is non-zero, this check takes
    /// priority over `threshold_chars`. This eliminates the 4-char/token
    /// assumption that fails for CJK content. Set to `None` to rely solely
    /// on the char-based heuristic.
    pub threshold_prompt_tokens: Option<u32>,
    /// Number of most-recent messages to keep verbatim during compaction.
    pub keep_recent_n: usize,
}

impl Default for Compactor {
    fn default() -> Self {
        Self {
            threshold_chars: usize::MAX,
            threshold_prompt_tokens: None,
            keep_recent_n: 8,
        }
    }
}

impl Compactor {
    /// Create a new compactor with the given threshold and default `keep_recent_n` (8).
    pub fn new(threshold_chars: usize) -> Self {
        Self {
            threshold_chars,
            threshold_prompt_tokens: None,
            keep_recent_n: 8,
        }
    }

    /// Set the token-count threshold for compaction.
    ///
    /// When the last turn's `prompt_tokens` (from the API response) meets or
    /// exceeds this value, compaction is triggered instead of relying on the
    /// character-count estimate. Prefer this for non-English workloads.
    pub fn threshold_prompt_tokens(mut self, n: u32) -> Self {
        self.threshold_prompt_tokens = Some(n);
        self
    }

    /// Set the number of recent messages to preserve verbatim.
    pub fn keep_recent_n(mut self, n: usize) -> Self {
        self.keep_recent_n = n;
        self
    }

    /// Estimate the prompt byte count of a transcript.
    ///
    /// This is a rough proxy for token count (≈4 bytes/token for English;
    /// over-counts CJK ~3× because CJK chars are 3 UTF-8 bytes each). The
    /// agent uses this to decide whether compaction is needed before the
    /// next LLM call. It is intentionally byte-based and kept in bytes —
    /// the char/byte threshold calibration depends on it.
    /// Includes tool_calls arguments and reasoning_content which were previously
    /// omitted, causing the threshold to be systematically underestimated.
    pub fn estimate_bytes(transcript: &[Message]) -> usize {
        transcript
            .iter()
            .map(|m| {
                m.content.len()
                    + m.tool_calls
                        .iter()
                        .map(|tc| tc.name.len() + tc.arguments.to_string().len() + 32)
                        .sum::<usize>()
                    + m.reasoning_content.as_deref().map_or(0, |r| r.len())
            })
            .sum()
    }

    /// Decide whether compaction should run, preferring the token-based
    /// threshold (accurate, CJK-safe) when actual API `prompt_tokens` are
    /// available, falling back to the byte estimate.
    ///
    /// `last_prompt_tokens` of `0` means "no API reading yet" (first step,
    /// or a provider that never reports usage) — the byte estimate is used.
    pub fn should_compact(&self, estimate_bytes: usize, last_prompt_tokens: u32) -> bool {
        match (self.threshold_prompt_tokens, last_prompt_tokens) {
            (Some(token_threshold), actual) if actual > 0 => actual >= token_threshold,
            _ => estimate_bytes >= self.threshold_chars,
        }
    }

    /// JSON schema for structured compaction output.
    const COMPACT_SCHEMA: &'static str = r#"{"type":"object","properties":{"summary":{"type":"string","description":"1-3 paragraph summary of the conversation so far, preserving key decisions, file paths touched, and outcomes."},"kept_facts":{"type":"array","items":{"type":"string"},"description":"Discrete facts worth remembering across compaction (e.g. 'goal=add_X_to_Y', 'compaction happened at step N', 'tool X failed 3 times')."},"next_steps":{"type":"array","items":{"type":"string"},"description":"Outstanding TODOs the agent identified before compaction (each one a single-sentence imperative)."}},"required":["summary","kept_facts"]}"#;

    /// Render a structured compaction result into the message format.
    fn render_structured(
        summary: &str,
        kept_facts: &[String],
        next_steps: &[String],
        step: usize,
    ) -> String {
        let mut rendered = format!(
            "[compacted: structured at step {step}]\n\nSummary: {summary}\n\nKey facts to remember:\n"
        );
        for fact in kept_facts {
            rendered.push_str(&format!("- {fact}\n"));
        }
        if !next_steps.is_empty() {
            rendered.push_str("\nOutstanding TODOs:\n");
            for step in next_steps {
                rendered.push_str(&format!("- {step}\n"));
            }
        }
        rendered
    }

    /// Try structured compaction, returning the rendered string on success.
    /// Returns None if the provider doesn't support it or the response is invalid.
    async fn try_structured_compact(
        &self,
        provider: &dyn ChatProvider,
        older_text: &str,
        step: usize,
    ) -> Option<String> {
        let structured_prompt = format!(
            "Summarize the following conversation. \
             Preserve: file paths modified, key technical decisions, test \
             outcomes, and any errors not yet resolved. Drop: file contents, \
             repeated tool errors, exploratory dead-ends.\n\n\
             Conversation to summarize:\n{older_text}"
        );

        let schema = match serde_json::from_str(Self::COMPACT_SCHEMA) {
            Ok(v) => v,
            Err(e) => {
                tracing::error!(error = %e, "COMPACT_SCHEMA is invalid JSON, falling back");
                return None;
            }
        };
        let structured_req = StructuredRequest {
            messages: vec![Message::user(structured_prompt)],
            schema,
            schema_name: "compaction_result".to_string(),
        };

        let json_val = match provider.complete_structured(structured_req).await {
            Ok(v) => v,
            Err(e) => {
                tracing::info!(error = %e, "structured compaction not available, falling back to free-text");
                return None;
            }
        };

        let obj = match json_val.as_object() {
            Some(o) => o,
            None => {
                tracing::warn!(
                    "structured compaction returned non-object, falling back to free-text"
                );
                return None;
            }
        };

        let summary = match obj.get("summary").and_then(|v| v.as_str()) {
            Some(s) => s.to_string(),
            None => {
                tracing::warn!(
                    "structured compaction missing 'summary' field, falling back to free-text"
                );
                return None;
            }
        };

        let kept_facts: Vec<String> = obj
            .get("kept_facts")
            .and_then(|v| v.as_array())
            .map(|arr| {
                arr.iter()
                    .filter_map(|v| v.as_str().map(|s| s.to_string()))
                    .collect()
            })
            .unwrap_or_default();

        let next_steps: Vec<String> = obj
            .get("next_steps")
            .and_then(|v| v.as_array())
            .map(|arr| {
                arr.iter()
                    .filter_map(|v| v.as_str().map(|s| s.to_string()))
                    .collect()
            })
            .unwrap_or_default();

        Some(Self::render_structured(
            &summary,
            &kept_facts,
            &next_steps,
            step,
        ))
    }

    /// Compute the safe split point for compaction: the index at which to
    /// divide "older messages to summarize" from "recent messages to keep".
    ///
    /// The retained ("kept") segment must start with a User (or System)
    /// message, because OpenAI/Anthropic require that the first non-System
    /// message after the compaction summary is a User message.  Two cases
    /// can violate this invariant:
    ///
    /// 1. The split point lands on a `Tool` result message — backing up
    ///    preserves tool-call / tool-result pairs.
    /// 2. The split point lands on an `Assistant` message that carries
    ///    `tool_calls` — backing up avoids starting the kept segment with
    ///    an Assistant-with-tool-calls, which would produce an invalid
    ///    `[System(summary), Assistant(tool_calls), Tool, …]` sequence.
    pub fn safe_split_point(transcript: &[Message], keep_n: usize) -> usize {
        let mut split = transcript.len().saturating_sub(keep_n);
        // Back up past Tool / Assistant-with-tool_calls to preserve pairing.
        loop {
            if split == 0 || split >= transcript.len() {
                break;
            }
            let msg = &transcript[split];
            let should_back_up = msg.role == crate::message::Role::Tool
                || (msg.role == crate::message::Role::Assistant && !msg.tool_calls.is_empty());
            if should_back_up {
                split -= 1;
            } else {
                break;
            }
        }
        // Advance past leading System messages — never compact the system
        // prompt.  Goal 345: without this a compaction whose `keep_recent_n`
        // would put the system message into `older` produces a garbage summary
        // ("No conversation found to summarize").
        while split < transcript.len() && transcript[split].role == crate::message::Role::System {
            split += 1;
        }
        split
    }

    /// Apply compaction in-place to `transcript`, returning `(removed, summary_chars)`.
    ///
    /// Finds the correct split point (never splits inside a tool-call pair),
    /// calls `compact()` to get the summary message, splices the transcript,
    /// and returns how many messages were removed and the summary char count.
    ///
    /// Returns `None` when the transcript is too short to compact
    /// (`< keep_recent_n + 2` messages).
    /// Minimum number of non-System conversational messages (User or
    /// Assistant) that must appear in the `older` slice for a compaction
    /// to be worth running.  Fewer than this means the older portion is
    /// essentially a system-prompt preamble and compacting it produces a
    /// garbage summary ("No conversation found to summarize").
    const MIN_CONVERSATIONAL_IN_OLDER: usize = 2;

    /// Pure helper: the split point for a compaction and whether the `older`
    /// slice is acceptable to summarise. Returns `(split, false)` when the
    /// transcript is too short or the older slice is degenerate (fewer than
    /// `MIN_CONVERSATIONAL_IN_OLDER` conversational messages). Single source
    /// of truth shared by [`apply_to_transcript`] and [`would_compact`] so the
    /// "will compaction actually fire?" decision cannot drift between them.
    /// (Goal 345)
    fn split_for_compaction(transcript: &[Message], keep_n: usize) -> (usize, bool) {
        if transcript.len() < keep_n + 2 {
            return (0, false);
        }
        let split = Self::safe_split_point(transcript, keep_n);
        let acceptable = if split > 0 {
            let conversational_count = transcript[..split]
                .iter()
                .filter(|m| {
                    m.role == crate::message::Role::User
                        || m.role == crate::message::Role::Assistant
                })
                .count();
            conversational_count >= Self::MIN_CONVERSATIONAL_IN_OLDER
        } else {
            // split == 0: older slice is empty. Preserve the pre-existing
            // behaviour (proceed) rather than tightening it here — the g331
            // failure mode was split > 0 with only a system-prompt older, not
            // the empty-older edge. Tightening split == 0 is out of scope and
            // risks breaking tests that rely on compaction firing at the
            // minimum-length boundary.
            true
        };
        (split, acceptable)
    }

    /// Returns `true` iff [`apply_to_transcript`] would actually compact this
    /// transcript (return `Ok(Some(..))`): long enough AND the older slice has
    /// enough real conversation. Pure — no provider call, no mutation.
    ///
    /// Proactive-compaction callers should consult this **before** dispatching
    /// `PreCompact` so that `PreCompact` and `PostCompact` stay balanced (both
    /// fire when compaction runs, neither fires when it is rejected). Without
    /// this guard the Goal 345 degenerate-slice rejection made `PreCompact`
    /// fire without a matching `PostCompact`. (Goal 345)
    pub fn would_compact(&self, transcript: &[Message]) -> bool {
        Self::split_for_compaction(transcript, self.keep_recent_n).1
    }

    pub async fn apply_to_transcript(
        &self,
        provider: &dyn ChatProvider,
        transcript: &mut Vec<Message>,
        step: usize,
    ) -> Result<Option<(usize, usize)>> {
        // Goal 345: delegate the "is this worth compacting?" decision to
        // `split_for_compaction` (shared with `would_compact`) so callers can
        // pre-check without dispatching `PreCompact` into the void.
        let (split, acceptable) = Self::split_for_compaction(transcript, self.keep_recent_n);
        if !acceptable {
            return Ok(None);
        }

        let summary_msg = self.compact(provider, transcript, step).await?;
        let summary_chars = summary_msg.content.len();
        let removed = split;
        transcript.drain(..split);
        transcript.insert(0, summary_msg);
        Ok(Some((removed, summary_chars)))
    }

    /// Render a slice of messages into a single text block for the
    /// summarisation prompt. Each message is wrapped in XML-style tags
    /// with tool_call summaries appended.
    fn render_for_summarize(messages: &[Message]) -> String {
        messages
            .iter()
            .map(|m| {
                let role_tag = match m.role {
                    crate::message::Role::System => "system",
                    crate::message::Role::User => "user",
                    crate::message::Role::Assistant => "assistant",
                    crate::message::Role::Tool => "tool",
                };
                let mut body = m.content.replace('<', "&lt;").replace('>', "&gt;");
                if !m.tool_calls.is_empty() {
                    let calls_summary: Vec<String> = m
                        .tool_calls
                        .iter()
                        .map(|tc| format!("{}({})", tc.name, tc.arguments))
                        .collect();
                    body.push_str(&format!("\n[tool_calls: {}]", calls_summary.join(", ")));
                }
                format!("<{role_tag}>{body}</{role_tag}>")
            })
            .collect::<Vec<_>>()
            .join("\n")
    }

    /// Ask the provider to summarise the given messages.
    /// Tries structured output first, falls back to free-text completion.
    async fn summarize(
        &self,
        provider: &dyn ChatProvider,
        messages: &[Message],
        step: usize,
    ) -> Result<String> {
        let older_text = Self::render_for_summarize(messages);

        // Try structured output first
        match self
            .try_structured_compact(provider, &older_text, step)
            .await
        {
            Some(rendered) => Ok(rendered),
            None => {
                // Fall back to free-text path with structured 9-section template
                let summary_prompt = format!(
                    "{}\n\nConversation to summarize:\n{}",
                    crate::compact::prompt::FREE_TEXT_COMPACT_PROMPT,
                    older_text,
                );
                let completion = provider
                    .complete(&[Message::user(summary_prompt)], &[] as &[ToolSpec])
                    .await?;
                Ok(crate::compact::prompt::format_compact_summary(
                    &completion.content,
                ))
            }
        }
    }

    /// Compact the transcript: summarize older messages into a single system
    /// message, keeping the last `keep_recent_n` messages verbatim.
    ///
    /// `step` is the current turn number and is embedded in the compaction
    /// header for debuggability.
    ///
    /// Returns the summary `Message` that should replace the older portion.
    /// The caller is responsible for splicing it into the transcript.
    ///
    /// If the summarisation call returns a context-window-exceeded error,
    /// the oldest message groups are dropped (via [`truncate_head_for_retry`])
    /// and the call is retried up to [`MAX_PTL_RETRIES`] times.
    #[tracing::instrument(skip(self, provider, transcript))]
    pub async fn compact(
        &self,
        provider: &dyn ChatProvider,
        transcript: &[Message],
        step: usize,
    ) -> Result<Message> {
        let split = Self::safe_split_point(transcript, self.keep_recent_n);
        let older = &transcript[..split];
        let _recent = &transcript[split..];

        // Retry loop: if the summarisation call itself exceeds the context
        // window, drop oldest message groups and try again.
        let mut to_summarize: Vec<Message> = older.to_vec();
        let mut ptl_attempts = 0_usize;

        let summary = loop {
            match self.summarize(provider, &to_summarize, step).await {
                Ok(text) => break text,
                Err(e) if crate::error::is_context_window_exceeded(&e) => {
                    ptl_attempts += 1;
                    if ptl_attempts > MAX_PTL_RETRIES {
                        return Err(e);
                    }
                    let target = retry::estimate_target_from_error(&e);
                    match retry::truncate_head_for_retry(&to_summarize, target) {
                        Some(truncated) => {
                            tracing::info!(
                                target: "recursive::compact",
                                ptl_attempts,
                                original_len = to_summarize.len(),
                                truncated_len = truncated.len(),
                                "compaction PTL retry: dropped oldest groups"
                            );
                            to_summarize = truncated;
                        }
                        None => {
                            // Cannot truncate further — propagate the error.
                            return Err(e);
                        }
                    }
                }
                Err(e) => return Err(e),
            }
        };

        let summary_chars = summary.len();

        let header = format!(
            "[compacted: {} messages → {} chars at step {step}]\n{}",
            older.len(),
            summary_chars,
            summary
        );

        Ok(Message::system(header).with_compaction_summary())
    }
}

#[cfg(test)]
mod tests {
    use super::*;
    use crate::llm::{Completion, MockProvider};

    #[tokio::test]
    async fn compact_returns_system_message_with_summary() {
        let provider = MockProvider::new(vec![Completion {
            content: "Key decisions: added adder tool. Tests pass.".to_string(),
            tool_calls: vec![],
            finish_reason: Some("stop".to_string()),
            usage: None,
            reasoning_content: None,
        }]);

        let transcript = vec![
            Message::system("You are a coding agent.".to_string()),
            Message::user("Add an adder tool".to_string()),
            Message::assistant("Let me create the tool.".to_string()),
            Message::user("Done. Now test it.".to_string()),
            Message::assistant("Tests pass.".to_string()),
        ];

        let compactor = Compactor::new(200).keep_recent_n(2);
        let summary_msg = compactor.compact(&provider, &transcript, 0).await.unwrap();

        assert_eq!(summary_msg.role, crate::message::Role::System);
        assert!(
            summary_msg.is_compaction_summary,
            "compactor must mark summary message with the bit"
        );
        assert!(
            summary_msg.content.contains("[compacted:"),
            "compactor must still include the [compacted: header for debuggability"
        );
        assert!(summary_msg.content.contains("Key decisions:"));
        assert!(summary_msg.content.contains("Tests pass."));
    }

    #[tokio::test]
    async fn compact_preserves_recent_messages() {
        let provider = MockProvider::new(vec![Completion {
            content: "Summary of older messages.".to_string(),
            tool_calls: vec![],
            finish_reason: Some("stop".to_string()),
            usage: None,
            reasoning_content: None,
        }]);

        let transcript = vec![
            Message::system("sys".to_string()),
            Message::user("old goal".to_string()),
            Message::assistant("old reply".to_string()),
            Message::user("recent goal".to_string()),
            Message::assistant("recent reply".to_string()),
        ];

        // keep_recent_n=2 should keep the last 2 messages verbatim
        let compactor = Compactor::new(100).keep_recent_n(2);
        let summary_msg = compactor.compact(&provider, &transcript, 5).await.unwrap();

        assert!(summary_msg.content.contains("[compacted: 3 messages →"));
        // The summary should mention the older messages
        assert!(summary_msg.content.contains("Summary of older messages."));
    }

    #[tokio::test]
    async fn compact_handles_empty_older_portion() {
        let provider = MockProvider::new(vec![Completion {
            content: "nothing to summarize".to_string(),
            tool_calls: vec![],
            finish_reason: Some("stop".to_string()),
            usage: None,
            reasoning_content: None,
        }]);

        let transcript = vec![Message::user("only message".to_string())];

        // keep_recent_n=5 means all messages are "recent", none to compact
        let compactor = Compactor::new(100).keep_recent_n(5);
        let summary_msg = compactor.compact(&provider, &transcript, 0).await.unwrap();

        // Should still produce a summary (even if older portion is empty-ish)
        assert_eq!(summary_msg.role, crate::message::Role::System);
        assert!(
            summary_msg.is_compaction_summary,
            "compactor must mark summary message with the bit"
        );
        assert!(
            summary_msg.content.contains("[compacted:"),
            "compactor must still include the [compacted: header for debuggability"
        );
    }

    #[test]
    fn estimate_bytes_sums_content_lengths() {
        let transcript = vec![
            Message::user("hello".to_string()),
            Message::assistant("world".to_string()),
        ];
        assert_eq!(Compactor::estimate_bytes(&transcript), 10);
    }

    #[test]
    fn default_threshold_is_max() {
        let c = Compactor::default();
        assert_eq!(c.threshold_chars, usize::MAX);
        assert_eq!(c.threshold_prompt_tokens, None);
        assert_eq!(c.keep_recent_n, 8);
    }

    #[test]
    fn builder_methods_work() {
        let c = Compactor::new(500).keep_recent_n(4);
        assert_eq!(c.threshold_chars, 500);
        assert_eq!(c.threshold_prompt_tokens, None);
        assert_eq!(c.keep_recent_n, 4);
    }

    #[test]
    fn threshold_prompt_tokens_setter_works() {
        let c = Compactor::new(500_000).threshold_prompt_tokens(144_000);
        assert_eq!(c.threshold_chars, 500_000);
        assert_eq!(c.threshold_prompt_tokens, Some(144_000));
        assert_eq!(c.keep_recent_n, 8);
    }

    // ========================================================================
    // should_compact tests (Goal 330)
    // ========================================================================

    #[test]
    fn should_compact_uses_token_threshold_when_available() {
        let c = Compactor::new(usize::MAX).threshold_prompt_tokens(1000);
        // At threshold → true
        assert!(c.should_compact(0, 1000));
        // Below threshold → false
        assert!(!c.should_compact(0, 999));
        // Far below → false
        assert!(!c.should_compact(0, 1));
    }

    #[test]
    fn should_compact_falls_back_to_chars_when_tokens_zero() {
        let c = Compactor::new(500).threshold_prompt_tokens(1000);
        // last_prompt_tokens = 0 means no reading yet → use char threshold
        // 600 chars >= 500 → true
        assert!(c.should_compact(600, 0));
        // 400 chars < 500 → false
        assert!(!c.should_compact(400, 0));
    }

    #[test]
    fn should_compact_falls_back_to_chars_when_threshold_none() {
        let c = Compactor::new(500);
        assert_eq!(c.threshold_prompt_tokens, None);
        // 600 chars >= 500 → true
        assert!(c.should_compact(600, 42));
        // 400 chars < 500 → false
        assert!(!c.should_compact(400, 42));
    }

    #[test]
    fn should_compact_zero_token_threshold_fires_on_zero() {
        // threshold_prompt_tokens = Some(0) is a degenerate case:
        // only match when last_prompt_tokens == 0, the predicate
        // guard `actual > 0` prevents it, so it falls through to char.
        let c = Compactor::new(usize::MAX).threshold_prompt_tokens(0);
        // last_prompt_tokens = 0 → guard fails, falls to char threshold
        // usize::MAX chars → 0 >= usize::MAX is false
        assert!(!c.should_compact(0, 0));
        // With actual > 0, guard is active: 1 >= 0 is true
        assert!(c.should_compact(0, 1));
    }

    /// Regression test — cross-turn compaction must use `last_prompt_tokens`
    /// (the single most-recent LLM call's value) not the accumulated sum over all
    /// LLM calls in the turn.  Before the fix, `runtime.rs` passed
    /// `turn_outcome.usage.prompt_tokens` (accumulated), which caused premature
    /// compaction on multi-step turns even when the actual context was well below
    /// the threshold.
    ///
    /// Scenario: deepseek-v4-flash (1 M context), threshold = 784 000 tokens.
    /// A turn with 8 LLM calls each at ~100 K tokens:
    ///   accumulated = 8 × 100_000 = 800_000  →  should_compact = TRUE  (OLD WRONG behaviour)
    ///   last_call   =     100_000            →  should_compact = FALSE (CORRECT)
    #[test]
    fn cross_turn_compact_uses_last_not_accumulated_prompt_tokens() {
        // deepseek-v4-flash threshold: (1_000_000 - 20_000) * 0.80 = 784_000
        let threshold = 784_000u32;
        let c = Compactor::new(usize::MAX).threshold_prompt_tokens(threshold);

        let per_call_tokens: u32 = 100_000;
        let llm_calls_in_turn: u32 = 8;
        let accumulated_tokens = per_call_tokens * llm_calls_in_turn; // 800_000
        let last_call_tokens = per_call_tokens; // 100_000

        // Confirm that the accumulated value WOULD have triggered compaction
        // (this is what the old code passed — it was wrong).
        assert!(
            c.should_compact(0, accumulated_tokens),
            "accumulated tokens ({accumulated_tokens}) exceeds threshold \
             ({threshold}): should_compact is true — shows why old code was wrong"
        );

        // With the correct last_call value, compaction must NOT fire.
        // runtime.rs now passes TurnOutcome::last_prompt_tokens here.
        assert!(
            !c.should_compact(0, last_call_tokens),
            "last_call tokens ({last_call_tokens}) is well below threshold \
             ({threshold}): compaction must NOT fire"
        );
    }

    // ========================================================================
    // Structured compaction tests
    // ========================================================================

    #[tokio::test]
    async fn compactor_structured_happy_path() {
        let json = serde_json::json!({
            "summary": "Added adder tool and verified tests pass.",
            "kept_facts": [
                "goal=add_adder_tool",
                "tool adder created successfully",
                "tests pass"
            ],
            "next_steps": [
                "Add subtractor tool",
                "Run integration tests"
            ]
        });
        let provider = MockProvider::new(vec![]).with_structured_responses(vec![Ok(json)]);

        let transcript = vec![
            Message::system("You are a coding agent.".to_string()),
            Message::user("Add an adder tool".to_string()),
            Message::assistant("Let me create the tool.".to_string()),
            Message::user("Done. Now test it.".to_string()),
            Message::assistant("Tests pass.".to_string()),
        ];

        let compactor = Compactor::new(200).keep_recent_n(2);
        let summary_msg = compactor.compact(&provider, &transcript, 3).await.unwrap();

        assert_eq!(summary_msg.role, crate::message::Role::System);
        // Should contain the structured rendering format with the step number
        assert!(summary_msg
            .content
            .contains("[compacted: structured at step 3]"));
        assert!(summary_msg.content.contains("Summary: Added adder tool"));
        assert!(summary_msg.content.contains("Key facts to remember:"));
        assert!(summary_msg.content.contains("- goal=add_adder_tool"));
        assert!(summary_msg
            .content
            .contains("- tool adder created successfully"));
        assert!(summary_msg.content.contains("- tests pass"));
        assert!(summary_msg.content.contains("Outstanding TODOs:"));
        assert!(summary_msg.content.contains("- Add subtractor tool"));
        assert!(summary_msg.content.contains("- Run integration tests"));
    }

    #[tokio::test]
    async fn compactor_falls_back_on_structured_error() {
        // MockProvider with no structured responses configured -> returns error
        let provider = MockProvider::new(vec![Completion {
            content: "Free-text fallback summary.".to_string(),
            tool_calls: vec![],
            finish_reason: Some("stop".to_string()),
            usage: None,
            reasoning_content: None,
        }]);

        let transcript = vec![
            Message::user("goal".to_string()),
            Message::assistant("response".to_string()),
        ];

        let compactor = Compactor::new(100).keep_recent_n(1);
        let summary_msg = compactor.compact(&provider, &transcript, 0).await.unwrap();

        assert_eq!(summary_msg.role, crate::message::Role::System);
        // Should have fallen back to free-text format
        assert!(
            summary_msg.is_compaction_summary,
            "compactor must mark summary message with the bit"
        );
        assert!(
            summary_msg.content.contains("[compacted:"),
            "compactor must still include the [compacted: header for debuggability"
        );
        assert!(summary_msg.content.contains("Free-text fallback summary."));
    }

    #[tokio::test]
    async fn compactor_structured_invalid_response_falls_back() {
        // Return valid JSON but not matching the schema (missing 'summary')
        let json = serde_json::json!({
            "foo": "bar"
        });
        let provider = MockProvider::new(vec![Completion {
            content: "Fallback after invalid structured response.".to_string(),
            tool_calls: vec![],
            finish_reason: Some("stop".to_string()),
            usage: None,
            reasoning_content: None,
        }])
        .with_structured_responses(vec![Ok(json)]);

        let transcript = vec![
            Message::user("goal".to_string()),
            Message::assistant("response".to_string()),
        ];

        let compactor = Compactor::new(100).keep_recent_n(1);
        let summary_msg = compactor.compact(&provider, &transcript, 0).await.unwrap();

        assert_eq!(summary_msg.role, crate::message::Role::System);
        // Should have fallen back to free-text format
        assert!(
            summary_msg.is_compaction_summary,
            "compactor must mark summary message with the bit"
        );
        assert!(
            summary_msg.content.contains("[compacted:"),
            "compactor must still include the [compacted: header for debuggability"
        );
        assert!(summary_msg
            .content
            .contains("Fallback after invalid structured response."));
    }

    #[tokio::test]
    async fn compact_freetext_fallback_uses_structured_prompt_and_formats() {
        // MockProvider with no structured responses -> falls back to free-text path.
        // The free-text fallback now uses the 9-section template and
        // format_compact_summary post-processor. This test verifies that:
        //   - The <analysis>...<analysis> block is stripped.
        //   - The <summary>...</summary> is converted to "Summary:" header.
        //   - The [compacted: header is still present from compact().
        let provider = MockProvider::new(vec![Completion {
            content: "<analysis>scratchpad thinking</analysis>\n\
                       <summary>Added feature X. Tests pass.</summary>"
                .to_string(),
            tool_calls: vec![],
            finish_reason: Some("stop".to_string()),
            usage: None,
            reasoning_content: None,
        }]);

        let transcript = vec![
            Message::user("Add feature X".to_string()),
            Message::assistant("Working on it.".to_string()),
            Message::user("Test it.".to_string()),
            Message::assistant("Tests pass.".to_string()),
        ];

        let compactor = Compactor::new(100).keep_recent_n(2);
        let summary_msg = compactor.compact(&provider, &transcript, 0).await.unwrap();

        assert_eq!(summary_msg.role, crate::message::Role::System);
        // Must carry the [compacted: header from compact()
        assert!(
            summary_msg.content.contains("[compacted:"),
            "compactor must include the [compacted: header, got: {}",
            summary_msg.content
        );
        // The analysis block must have been stripped
        assert!(
            !summary_msg.content.contains("scratchpad thinking"),
            "analysis scratchpad must be stripped"
        );
        // The summary tag must be converted to a "Summary:" header
        assert!(
            summary_msg.content.contains("Summary:"),
            "summary must contain 'Summary:' header, got: {}",
            summary_msg.content
        );
        // The actual summary content must be present
        assert!(
            summary_msg.content.contains("Added feature X"),
            "summary content must be preserved, got: {}",
            summary_msg.content
        );
        assert!(
            summary_msg.content.contains("Tests pass."),
            "summary content must be preserved, got: {}",
            summary_msg.content
        );
    }

    #[test]
    fn safe_split_point_keep_n_zero_does_not_panic() {
        // keep_n=0 means split = transcript.len(); the while loop must not
        // index transcript[transcript.len()] — that would be out of bounds.
        let msgs = vec![
            Message::user("a".to_string()),
            Message::assistant("b".to_string()),
        ];
        let split = Compactor::safe_split_point(&msgs, 0);
        assert_eq!(split, msgs.len(), "keep_n=0 should return full length");
    }

    #[test]
    fn safe_split_point_empty_transcript_does_not_panic() {
        let split = Compactor::safe_split_point(&[], 0);
        assert_eq!(split, 0);
        let split = Compactor::safe_split_point(&[], 5);
        assert_eq!(split, 0);
    }

    #[test]
    fn safe_split_backs_up_past_tool_and_assistant_with_tool_calls() {
        // Sequence: [user, asst+tc, tool_result, user, asst]  (indices 0..4)
        // keep_n=3: initial split = 5-3 = 2 (Tool message)
        //   → backs up to 1 (Assistant+tool_calls)
        //   → backs up to 0 (loop breaks: split==0)
        use crate::llm::ToolCall;
        let asst_with_calls = Message::assistant_with_tool_calls(
            "thinking".to_string(),
            vec![ToolCall {
                id: "call_1".into(),
                name: "Read".into(),
                arguments: serde_json::json!("{}"),
            }],
        );
        let msgs = vec![
            Message::user("start".to_string()),
            asst_with_calls,
            Message::tool_result("call_1", "file contents"),
            Message::user("continue".to_string()),
            Message::assistant("done".to_string()),
        ];
        let split = Compactor::safe_split_point(&msgs, 3);
        assert_eq!(
            split, 0,
            "should back up past both Tool and Assistant-with-tool-calls to index 0"
        );
    }

    #[test]
    fn safe_split_backs_up_when_landing_directly_on_assistant_with_tool_calls() {
        // Sequence: [user, asst+tc, user, asst]  (indices 0..3)
        // keep_n=3: initial split = 4-3 = 1 (Assistant+tool_calls)
        //   → backs up to 0 (loop breaks: split==0)
        use crate::llm::ToolCall;
        let asst_with_calls = Message::assistant_with_tool_calls(
            "planning".to_string(),
            vec![ToolCall {
                id: "c1".into(),
                name: "Glob".into(),
                arguments: serde_json::json!("{}"),
            }],
        );
        let msgs = vec![
            Message::user("first question".to_string()),
            asst_with_calls,
            Message::user("follow-up".to_string()),
            Message::assistant("answer".to_string()),
        ];
        let split = Compactor::safe_split_point(&msgs, 3);
        assert_eq!(
            split, 0,
            "split landing directly on Assistant+tool_calls should retreat to 0"
        );
    }

    #[test]
    fn safe_split_no_backup_when_split_is_already_valid() {
        // Sequence: [user, asst+tc, tool_result, user, asst]  (indices 0..4)
        // keep_n=2: initial split = 5-2 = 3 (User message) → no backup needed
        use crate::llm::ToolCall;
        let asst_with_calls = Message::assistant_with_tool_calls(
            "".to_string(),
            vec![ToolCall {
                id: "x".into(),
                name: "Write".into(),
                arguments: serde_json::json!("{}"),
            }],
        );
        let msgs = vec![
            Message::user("start".to_string()),
            asst_with_calls,
            Message::tool_result("x", "ok"),
            Message::user("continue".to_string()),
            Message::assistant("done".to_string()),
        ];
        let split = Compactor::safe_split_point(&msgs, 2);
        assert_eq!(split, 3, "split at a User message should not retreat");
    }

    // ========================================================================
    // estimate_bytes — tool_calls and reasoning_content coverage
    // ========================================================================

    #[test]
    fn estimate_bytes_includes_tool_calls() {
        use crate::llm::ToolCall;
        // An assistant message with one tool call.
        // name = "Read" (4 chars), arguments = "\"path\"" (6 chars), overhead = 32
        let tool_call = ToolCall {
            id: "c1".into(),
            name: "Read".into(),
            arguments: serde_json::json!("path"),
        };
        let msg = Message::assistant_with_tool_calls("think".to_string(), vec![tool_call]);
        // content=5, name=4, args=serde representation len, overhead=32
        let args_len = msg.tool_calls[0].arguments.to_string().len();
        let expected = 5 + (4 + args_len + 32);
        assert_eq!(
            Compactor::estimate_bytes(&[msg]),
            expected,
            "tool_calls contribution must be counted"
        );
    }

    #[test]
    fn estimate_bytes_includes_reasoning_content() {
        let mut msg = Message::assistant("hello".to_string());
        msg.reasoning_content = Some("some reasoning".to_string());
        // content=5, reasoning=14
        assert_eq!(
            Compactor::estimate_bytes(&[msg]),
            5 + 14,
            "reasoning_content must be counted"
        );
    }

    #[test]
    fn estimate_bytes_tool_calls_plus_reasoning_combined() {
        use crate::llm::ToolCall;
        let tool_call = ToolCall {
            id: "c2".into(),
            name: "Write".into(),
            arguments: serde_json::json!({"path": "x"}),
        };
        let mut msg = Message::assistant_with_tool_calls("".to_string(), vec![tool_call]);
        msg.reasoning_content = Some("reason".to_string());
        let args_len = msg.tool_calls[0].arguments.to_string().len();
        // content=0, tool_call: name=5, args=..., +32; reasoning=6
        let expected = (5 + args_len + 32) + 6;
        assert_eq!(Compactor::estimate_bytes(&[msg]), expected);
    }

    // ========================================================================
    // apply_to_transcript — splice and return-value coverage
    // ========================================================================

    #[tokio::test]
    async fn apply_to_transcript_splices_and_returns_counts() {
        let provider = MockProvider::new(vec![Completion {
            content: "summary of first three".to_string(),
            tool_calls: vec![],
            finish_reason: Some("stop".to_string()),
            usage: None,
            reasoning_content: None,
        }]);

        let mut transcript = vec![
            Message::system("sys".to_string()),
            Message::user("msg1".to_string()),
            Message::assistant("rep1".to_string()),
            Message::user("msg2".to_string()),
            Message::assistant("rep2".to_string()),
            Message::user("msg3".to_string()),
        ];

        // keep_recent_n=2 → split=4, compact first 4 msgs → removed=4
        let compactor = Compactor::new(50).keep_recent_n(2);
        let result = compactor
            .apply_to_transcript(&provider, &mut transcript, 5)
            .await
            .unwrap();

        // Should have returned Some((removed, summary_chars))
        let (removed, summary_chars) =
            result.expect("should compact when transcript is long enough");
        assert!(removed > 0, "removed must be > 0 when compaction ran");
        assert!(summary_chars > 0, "summary_chars must be > 0");

        // Transcript must start with the compaction summary
        assert_eq!(transcript[0].role, crate::message::Role::System);
        assert!(transcript[0].is_compaction_summary);
        // The 2 recent messages are preserved after the summary
        assert!(transcript.len() >= 3, "summary + at least 2 recent");
    }

    #[tokio::test]
    async fn apply_to_transcript_too_short_returns_none() {
        // A provider that should never be called
        let provider = MockProvider::new(vec![]);

        let mut transcript = vec![
            Message::user("only one".to_string()),
            Message::assistant("reply".to_string()),
        ];

        // keep_recent_n=8 + 2 = 10 required, but transcript has only 2
        let compactor = Compactor::new(50).keep_recent_n(8);
        let result = compactor
            .apply_to_transcript(&provider, &mut transcript, 0)
            .await
            .unwrap();

        assert!(result.is_none(), "transcript too short must return None");
        // Transcript must not be modified
        assert_eq!(transcript.len(), 2);
    }

    #[tokio::test]
    async fn compact_includes_tool_calls_in_older_text() {
        // Verify that assistant messages carrying tool_calls contribute their
        // call summaries to the text sent to the LLM for summarization.
        // If the `!m.tool_calls.is_empty()` guard is inverted, the calls
        // are omitted (or spurious empty `[tool_calls: ]` is appended to
        // messages that have no calls), which breaks the compactor's ability
        // to preserve tool-invocation context across compaction boundaries.
        use crate::llm::{Completion, MockProvider, ToolCall};
        let provider = MockProvider::new(vec![Completion {
            content: "summary".to_string(),
            tool_calls: vec![],
            finish_reason: Some("stop".to_string()),
            usage: None,
            reasoning_content: None,
        }]);

        let tool_call = ToolCall {
            id: "c1".into(),
            name: "Read".into(),
            arguments: serde_json::json!({"path": "src/lib.rs"}),
        };
        let asst_with_calls =
            Message::assistant_with_tool_calls("thinking".to_string(), vec![tool_call]);
        // Build transcript long enough that the tool-call pair ends up in
        // the "older" portion: [user, asst+tc, tool_result, user] are older,
        // [asst, user] are "recent" (keep_recent_n=2).
        // safe_split_point with keep_n=2 on 6 msgs: initial split=4
        // (transcript[4] = "asst" — no tool_calls, no Tool role) → no retreat.
        let transcript = vec![
            Message::user("start".to_string()),          // [0] older
            asst_with_calls,                             // [1] older  ← has tool_calls
            Message::tool_result("c1", "file contents"), // [2] older
            Message::user("continue".to_string()),       // [3] older
            Message::assistant("done".to_string()),      // [4] recent
            Message::user("next".to_string()),           // [5] recent
        ];

        let compactor = Compactor::new(0).keep_recent_n(2);
        let _ = compactor.compact(&provider, &transcript, 0).await.unwrap();

        let calls = provider.calls();
        assert!(!calls.is_empty(), "provider must have been called");
        // The first (and only) call is the summarization request.
        let prompt_msg = &calls[0][0];
        assert!(
            prompt_msg.content.contains("Read"),
            "older_text must include the tool call name 'Read'; got: {}",
            prompt_msg.content
        );
        assert!(
            prompt_msg.content.contains("[tool_calls:"),
            "older_text must include the [tool_calls: ...] marker; got: {}",
            prompt_msg.content
        );
    }

    #[tokio::test]
    async fn apply_to_transcript_minimum_length_boundary() {
        // Goal 345: older must have ≥2 conversational messages.
        // At keep_n=1 with 4 messages, older = [sys, user, asst] → 2 conv.
        let provider = MockProvider::new(vec![Completion {
            content: "boundary summary".to_string(),
            tool_calls: vec![],
            finish_reason: Some("stop".to_string()),
            usage: None,
            reasoning_content: None,
        }]);

        let keep_n = 1_usize;
        let mut transcript = vec![
            Message::system("s".to_string()),
            Message::user("u".to_string()),
            Message::assistant("a".to_string()),
            Message::user("u2".to_string()),
        ];

        let compactor = Compactor::new(0).keep_recent_n(keep_n);
        let result = compactor
            .apply_to_transcript(&provider, &mut transcript, 1)
            .await
            .unwrap();

        assert!(
            result.is_some(),
            "should compact when older has ≥2 conversational msgs"
        );
    }

    // ========================================================================
    // render_structured — direct unit tests
    // ========================================================================

    #[test]
    fn render_structured_with_next_steps_includes_todos_section() {
        let rendered = Compactor::render_structured(
            "summary text",
            &["fact A".into(), "fact B".into()],
            &["step 1".into()],
            5,
        );
        assert!(rendered.contains("[compacted: structured at step 5]"));
        assert!(rendered.contains("Summary: summary text"));
        assert!(rendered.contains("- fact A"));
        assert!(rendered.contains("- fact B"));
        assert!(
            rendered.contains("Outstanding TODOs:"),
            "non-empty next_steps must show TODOs section"
        );
        assert!(rendered.contains("- step 1"));
    }

    #[test]
    fn render_structured_without_next_steps_omits_todos_section() {
        // Kills `delete !` at line 85: if next_steps.is_empty() (mutated from !next_steps.is_empty())
        // then the TODO block would appear even for empty next_steps.
        let rendered = Compactor::render_structured(
            "summary",
            &["fact".into()],
            &[], // empty next_steps
            1,
        );
        assert!(
            !rendered.contains("Outstanding TODOs:"),
            "empty next_steps must NOT include TODOs section (kills delete-! mutation)"
        );
    }

    #[tokio::test]
    async fn apply_to_transcript_boundary_plus_not_times() {
        // Kills: `replace + with * in Compactor::apply_to_transcript`.
        // The threshold is `keep_recent_n + 2`, NOT `keep_recent_n * 2`.
        // With keep_recent_n=2, older=[sys, user, asst] → 2 conv.
        // Original `+`: 5 >= 3 → compaction fires
        // Mutant `*`:   5 >= 4 → early return misuse
        let provider = MockProvider::new(vec![Completion {
            content: "summary".to_string(),
            tool_calls: vec![],
            finish_reason: Some("stop".to_string()),
            usage: None,
            reasoning_content: None,
        }]);

        let keep_n = 2_usize;
        // 5 messages: sys + 2 conv older + 2 kept
        let mut transcript = vec![
            Message::system("s0".to_string()),
            Message::user("u1".to_string()),
            Message::assistant("a1".to_string()),
            Message::user("u2".to_string()),
            Message::assistant("a2".to_string()),
        ];

        let compactor = Compactor::new(0).keep_recent_n(keep_n);
        let result = compactor
            .apply_to_transcript(&provider, &mut transcript, 1)
            .await
            .unwrap();

        assert!(
            result.is_some(),
            "must compact when older has ≥2 conversational msgs"
        );
    }

    // ========================================================================
    // Goal 345: degenerate slice rejection and system-message preservation
    // ========================================================================

    #[test]
    fn safe_split_point_keeps_system_message_in_kept() {
        // Scenario: a transcript where keep_recent_n would put the
        // system prompt into the older portion.  After the fix, the
        // split must advance past the system message so it stays
        // verbatim in `kept` — never summarised.
        let msgs = vec![
            Message::system("You are a coding agent. Be thorough.".to_string()),
            Message::user("Do task A".to_string()),
            Message::assistant("Working on A".to_string()),
            Message::user("Now do task B".to_string()),
            Message::assistant("Working on B".to_string()),
            Message::user("Now do task C".to_string()),
            Message::assistant("Working on C".to_string()),
        ];
        // keep_n=4: initial split = 7-4 = 3 (msgs[3] = "Now do task B")
        // That's a User message — no backup, no forward advance.
        let split = Compactor::safe_split_point(&msgs, 4);
        assert_eq!(split, 3, "valid split must not be disturbed");
    }

    #[test]
    fn safe_split_point_advances_past_system_at_boundary() {
        // Scenario: keep_recent_n is large enough that the initial
        // split lands on a System message.  The forward pass must
        // advance past it so the system prompt stays in `kept`.
        let msgs = vec![
            Message::system("SYSTEM PROMPT — keep me verbatim".to_string()),
            Message::user("Hello".to_string()),
            Message::assistant("Hi!".to_string()),
        ];
        // keep_n=3: initial split = 3-3 = 0
        // split=0: backup loop breaks immediately.
        // Forward pass: msgs[0] is System → advance to 1.
        let split = Compactor::safe_split_point(&msgs, 3);
        assert_eq!(
            split, 1,
            "system message at split boundary must be advanced past"
        );
    }

    #[tokio::test]
    async fn apply_to_transcript_rejects_system_only_older() {
        // The g331 failure mode: a long system prompt followed by a
        // few real messages, keep_recent_n covers almost everything,
        // older = [system] → summary says "No conversation found to
        // summarize".
        let provider = MockProvider::new(vec![]); // should never be called

        let mut transcript = vec![
            Message::system(
                "VERY LONG SYSTEM PROMPT — AGENTS.md, CLAUDE.md, journal, skills...".to_string(),
            ),
            Message::user("Add a feature".to_string()),
            Message::assistant("I'll work on it.".to_string()),
            Message::user("Status?".to_string()),
            Message::assistant("Almost done.".to_string()),
            Message::user("Now test it.".to_string()),
            Message::assistant("Tests pass.".to_string()),
            Message::user("Commit.".to_string()),
            Message::assistant("Done.".to_string()),
        ];
        // 9 messages total, keep_recent_n=8
        // safe_split_point: initial split = 9-8 = 1 (msgs[1]="Add a feature")
        // That's User → no backup. Forward: not System → no advance.
        // older = transcript[..1] = [system]
        // conversational_count = 0 < MIN(2) → return None
        let compactor = Compactor::new(0).keep_recent_n(8);
        let result = compactor
            .apply_to_transcript(&provider, &mut transcript, 0)
            .await
            .unwrap();

        assert!(
            result.is_none(),
            "system-only older (0 conversational msgs < 2 min) must be rejected"
        );
        // Transcript must not be modified
        assert_eq!(transcript.len(), 9, "transcript must be unchanged");
    }

    #[tokio::test]
    async fn apply_to_transcript_compacts_normally_with_real_conversation() {
        // When older has ≥2 conversational messages, compaction runs
        // normally.
        let provider = MockProvider::new(vec![Completion {
            content: "We added a feature and tested it.".to_string(),
            tool_calls: vec![],
            finish_reason: Some("stop".to_string()),
            usage: None,
            reasoning_content: None,
        }]);

        let mut transcript = vec![
            Message::system("System prompt".to_string()),
            Message::user("Add feature X".to_string()),
            Message::assistant("Working on X".to_string()),
            Message::user("Add feature Y".to_string()),
            Message::assistant("Working on Y".to_string()),
            Message::user("Test everything".to_string()),
            Message::assistant("All tests pass".to_string()),
        ];
        // keep_recent_n=2: older = transcript[..5] = [sys, user, asst, user, asst]
        // conversational_count = 4 (≥2) → compaction proceeds
        let compactor = Compactor::new(0).keep_recent_n(2);
        let result = compactor
            .apply_to_transcript(&provider, &mut transcript, 1)
            .await
            .unwrap();

        let (removed, summary_chars) =
            result.expect("should compact when older has real conversation");
        assert!(removed > 0);
        assert!(summary_chars > 0);
        assert_eq!(transcript[0].role, crate::message::Role::System);
        assert!(transcript[0].is_compaction_summary);
    }

    #[tokio::test]
    async fn apply_to_transcript_rejects_below_min_conversational() {
        // 1 conversational message in older (< MIN=2) → reject.
        let provider = MockProvider::new(vec![]); // should never be called

        let mut transcript = vec![
            Message::system("System prompt".to_string()),
            Message::user("The only user message".to_string()),
            Message::assistant("Reply".to_string()),
            Message::user("Another user message".to_string()),
            Message::assistant("Another reply".to_string()),
            Message::user("Yet another".to_string()),
            Message::assistant("Final reply".to_string()),
        ];
        // 7 messages, keep_recent_n=5
        // safe_split_point: initial split = 7-5 = 2 (msgs[2]="Reply")
        // That's Asst w/o tool_calls → no backup. Forward: not System.
        // older = transcript[..2] = [system, user]
        // conversational_count = 1 < 2 → reject
        let compactor = Compactor::new(0).keep_recent_n(5);
        let result = compactor
            .apply_to_transcript(&provider, &mut transcript, 0)
            .await
            .unwrap();

        assert!(
            result.is_none(),
            "older with 1 conversational msg (< 2 min) must be rejected"
        );
    }

    #[tokio::test]
    async fn apply_to_transcript_accepts_exactly_min_conversational() {
        // Exactly 2 conversational messages in older (= MIN=2) → accept.
        let provider = MockProvider::new(vec![Completion {
            content: "Conversation about two messages.".to_string(),
            tool_calls: vec![],
            finish_reason: Some("stop".to_string()),
            usage: None,
            reasoning_content: None,
        }]);

        let mut transcript = vec![
            Message::system("System prompt".to_string()),
            Message::user("Hello".to_string()),
            Message::assistant("Hi there!".to_string()),
            Message::user("How are you?".to_string()),
            Message::assistant("I'm well, thanks.".to_string()),
        ];
        // 5 messages, keep_recent_n=2
        // safe_split_point: initial split = 5-2 = 3 (msgs[3]="How are you?")
        // That's User → no backup. Forward: not System.
        // older = transcript[..3] = [sys, user("Hello"), asst("Hi there!")]
        // conversational_count = 2 = MIN → accepted
        let compactor = Compactor::new(0).keep_recent_n(2);
        let result = compactor
            .apply_to_transcript(&provider, &mut transcript, 1)
            .await
            .unwrap();

        let (removed, _summary_chars) =
            result.expect("should compact when older has exactly 2 conversational msgs");
        assert!(removed > 0);
    }

    // ========================================================================
    // Goal 337: PTL retry loop tests
    // ========================================================================

    #[tokio::test]
    async fn compact_retries_on_ptl_then_succeeds() {
        // First summarization call returns PTL error, second succeeds.
        let provider = MockProvider::new(vec![Completion {
            content: "Second attempt summary.".to_string(),
            tool_calls: vec![],
            finish_reason: Some("stop".to_string()),
            usage: None,
            reasoning_content: None,
        }])
        .with_errors(vec![crate::error::Error::Llm {
            provider: "mock".into(),
            message: "HTTP 400: context_length_exceeded, prompt is too long".into(),
        }]);

        let transcript = vec![
            Message::system("sys".to_string()),
            Message::user("msg1".to_string()),
            Message::assistant("rep1".to_string()),
            Message::user("msg2".to_string()),
            Message::assistant("rep2".to_string()),
            Message::user("msg3".to_string()),
            Message::assistant("rep3".to_string()),
            Message::user("msg4".to_string()),
            Message::assistant("rep4".to_string()),
            Message::user("msg5".to_string()),
            Message::assistant("rep5".to_string()),
        ];

        let compactor = Compactor::new(0).keep_recent_n(2);
        let summary_msg = compactor.compact(&provider, &transcript, 0).await.unwrap();

        assert_eq!(summary_msg.role, crate::message::Role::System);
        assert!(
            summary_msg.content.contains("Second attempt summary."),
            "should contain the retry's summary, got: {}",
            summary_msg.content
        );

        // The provider should have been called twice (first error → retry).
        let calls = provider.calls();
        assert_eq!(
            calls.len(),
            2,
            "provider must be called twice (first error, then retry)"
        );
    }

    #[tokio::test]
    async fn compact_gives_up_after_max_retries() {
        // Provider returns PTL error every time.
        let mut errors = Vec::new();
        for _ in 0..=MAX_PTL_RETRIES {
            errors.push(crate::error::Error::Llm {
                provider: "mock".into(),
                message: "HTTP 400: context_length_exceeded, prompt is too long".into(),
            });
        }
        // Add one more completion so total calls = MAX_PTL_RETRIES + 1.
        // The loop tries once + 3 retries = 4 total calls, all errors.
        let provider = MockProvider::new(vec![]).with_errors(errors);

        let transcript = vec![
            Message::system("sys".to_string()),
            Message::user("long msg1".to_string()),
            Message::assistant("long rep1".to_string()),
            Message::user("long msg2".to_string()),
            Message::assistant("long rep2".to_string()),
        ];

        let compactor = Compactor::new(0).keep_recent_n(1);
        let result = compactor.compact(&provider, &transcript, 0).await;
        assert!(
            result.is_err(),
            "should return error after exhausting retries"
        );
        let err = result.unwrap_err();
        assert!(
            err.to_string().contains("context_length_exceeded"),
            "error should contain the PTL signal"
        );
    }

    #[tokio::test]
    async fn compact_non_ptl_error_propagates_immediately() {
        // Non-PTL error — should NOT retry.
        let provider = MockProvider::new(vec![]).with_errors(vec![crate::error::Error::Llm {
            provider: "mock".into(),
            message: "HTTP 400: invalid request".into(),
        }]);

        let transcript = vec![
            Message::system("sys".to_string()),
            Message::user("msg1".to_string()),
            Message::assistant("rep1".to_string()),
        ];

        let compactor = Compactor::new(0).keep_recent_n(1);
        let result = compactor.compact(&provider, &transcript, 0).await;
        assert!(
            result.is_err(),
            "non-PTL error should propagate immediately"
        );

        // Provider should have been called only once (no retry).
        let calls = provider.calls();
        assert_eq!(calls.len(), 1, "non-PTL error must not trigger retry");
    }

    // ========================================================================
    // Goal 341: boundary-preserving compaction validation
    //
    // Validates that compaction never modifies the recent tail messages —
    // they pass through byte-identical. Proves that splice reordering alone
    // cannot preserve the prefix cache (the summary is new content preceding
    // the recent region either way). The real cache-preserving lever is
    // microcompact (goals 332/333).
    // ========================================================================

    /// Validation helper: verify the recent-region messages are byte-identical
    /// after applying the compaction splice.
    ///
    /// Computes the split point, serialises the expected recent messages (the
    /// transcript slice from `split..`), applies `drain(..split)` +
    /// `insert(0, summary)` (the same splice `apply_to_transcript` uses),
    /// then asserts the messages at indices `1..` are byte-identical (via
    /// `serde_json` serialisation) to the pre-compaction recent region.
    ///
    /// This proves compaction never touches the recent tail — the only thing
    /// that changes is what precedes the recent region (the summary vs. the
    /// original older messages).
    async fn validate_boundary_preserving(
        transcript: &[Message],
        keep_n: usize,
        summary_content: &str,
    ) {
        let split = Compactor::safe_split_point(transcript, keep_n);

        // Serialize expected recent messages before any mutation.
        let expected_recent_json: Vec<String> = transcript[split..]
            .iter()
            .map(|m| serde_json::to_string(m).unwrap())
            .collect();

        // Apply the same splice that `apply_to_transcript` uses:
        // drain older, insert summary at head.
        let mut compacted = transcript.to_vec();
        let summary_msg = Message::system(format!(
            "[compacted: test summary at step 1]\n{}",
            summary_content
        ))
        .with_compaction_summary();
        compacted.drain(..split);
        compacted.insert(0, summary_msg);

        // After splice, recent messages are at compacted[1..] (after the summary).
        let recent_after_json: Vec<String> = compacted[1..]
            .iter()
            .map(|m| serde_json::to_string(m).unwrap())
            .collect();

        assert_eq!(
            recent_after_json, expected_recent_json,
            "recent region must be byte-identical after compaction splice"
        );
    }

    #[tokio::test]
    async fn boundary_preserving_recent_region_is_byte_identical() {
        // Build a transcript with a clear split: 4 older messages + 2 recent.
        let transcript = vec![
            Message::system("System prompt.".to_string()),
            Message::user("Old goal: add feature X".to_string()),
            Message::assistant("Working on feature X.".to_string()),
            Message::user("Old goal: test it.".to_string()),
            // ── split point (keep_n=2) ──
            Message::assistant("Recent: I fixed a test.".to_string()),
            Message::user("Recent: Let's commit.".to_string()),
        ];

        validate_boundary_preserving(
            &transcript,
            2, // keep_recent_n
            "Summary of the old goals: added feature X, tests pass.",
        )
        .await;
    }

    #[tokio::test]
    async fn boundary_preserving_with_tool_calls_in_older() {
        // Scenario: older portion includes tool-call pairs. The split point
        // backs up past Tool/Assistant+tool_calls, so the recent region
        // boundary may shift. Verify the recent region is still byte-identical.
        use crate::llm::ToolCall;

        let tool_call = ToolCall {
            id: "c1".into(),
            name: "Read".into(),
            arguments: serde_json::json!({"path": "src/lib.rs"}),
        };
        let transcript = vec![
            Message::system("sys".to_string()),
            Message::user("Read the file".to_string()),
            Message::assistant_with_tool_calls("reading".to_string(), vec![tool_call]),
            Message::tool_result("c1", "file contents here"),
            // ── split point (keep_n=2, after backup) ──
            Message::user("Recent: now edit".to_string()),
            Message::assistant("Recent: done".to_string()),
        ];

        validate_boundary_preserving(
            &transcript,
            2, // keep_recent_n
            "Summary of read-file steps.",
        )
        .await;
    }

    #[tokio::test]
    async fn mock_provider_accepts_post_compact_transcript() {
        // Feed a post-compaction transcript through MockProvider and verify
        // the first non-system message is Role::User (provider ordering
        // invariant). This confirms the current splice yields a valid
        // sequence — no mid-transcript System-message rejection risk.
        //
        // Two completions are needed: the first is consumed by
        // `apply_to_transcript`'s internal summarization call; the second
        // by the explicit `complete()` below.
        let provider = MockProvider::new(vec![
            Completion {
                content: "Summary: added feature, tests pass.".to_string(),
                tool_calls: vec![],
                finish_reason: Some("stop".to_string()),
                usage: None,
                reasoning_content: None,
            },
            Completion {
                content: "This is a response to the post-compact transcript.".to_string(),
                tool_calls: vec![],
                finish_reason: Some("stop".to_string()),
                usage: None,
                reasoning_content: None,
            },
        ]);

        let mut transcript = vec![
            Message::system("System prompt.".to_string()),
            Message::user("Goal: add feature".to_string()),
            Message::assistant("Working on it.".to_string()),
            Message::user("Test it.".to_string()),
            Message::assistant("Tests pass.".to_string()),
            Message::user("Recent: commit.".to_string()),
            Message::assistant("Recent: done.".to_string()),
        ];

        // Apply compaction: this produces transcript = [summary, recent...]
        let compactor = Compactor::new(0).keep_recent_n(2);
        let result = compactor
            .apply_to_transcript(&provider, &mut transcript, 5)
            .await
            .unwrap();
        assert!(
            result.is_some(),
            "compaction must succeed for the test transcript"
        );

        // The post-compact transcript should have: [System(summary), User("Recent: commit."), Assistant("Recent: done.")]
        // Feed it through the mock provider and verify:
        // 1. The provider accepts it (no error).
        // 2. The first non-system message is Role::User.
        let completion = provider.complete(&transcript, &[]).await;
        assert!(
            completion.is_ok(),
            "mock provider must accept post-compaction transcript"
        );

        // Check the messages that were sent to the provider:
        let calls = provider.calls();
        assert!(
            !calls.is_empty(),
            "provider should have been called via complete()"
        );
        let sent_messages = &calls[calls.len() - 1]; // Last call = our complete() above.

        // Find first non-system message and verify it's User.
        let first_non_system = sent_messages
            .iter()
            .find(|m| m.role != crate::message::Role::System);
        assert!(
            first_non_system.is_some(),
            "post-compact transcript must contain at least one non-System message"
        );
        assert_eq!(
            first_non_system.unwrap().role,
            crate::message::Role::User,
            "first non-system message must be User (provider ordering invariant)"
        );

        // Verify no message has role `Tool` without a preceding
        // Assistant-with-tool_calls (basic transcript validity).
        for (i, msg) in sent_messages.iter().enumerate() {
            if msg.role == crate::message::Role::Tool {
                // Ensure preceding message is an Assistant with matching tool_calls.
                if i > 0 {
                    let prev = &sent_messages[i - 1];
                    assert_eq!(
                        prev.role,
                        crate::message::Role::Assistant,
                        "Tool message at index {i} must be preceded by Assistant"
                    );
                }
            }
        }
    }

    #[tokio::test]
    async fn mock_provider_accepts_compact_with_system_summary_user_ordering() {
        // Edge case: compaction just happened, the transcript is:
        // [System(summary), System(original prompt), User(msg), ...]
        // Verify that after skipping all leading System messages, the first
        // non-System message is still User — no mid-stream System insertion
        // breaks the provider's ordering invariant.
        let provider = MockProvider::new(vec![Completion {
            content: "provider accepted the sequence".to_string(),
            tool_calls: vec![],
            finish_reason: Some("stop".to_string()),
            usage: None,
            reasoning_content: None,
        }]);

        // Simulate a post-compact transcript with multiple leading System msgs.
        let transcript = vec![
            Message::system("[compacted: summary at step 5] Summary text.")
                .with_compaction_summary(),
            Message::system("Original system prompt.".to_string()),
            Message::user("Post-compact user message.".to_string()),
            Message::assistant("Post-compact assistant response.".to_string()),
        ];

        let completion = provider.complete(&transcript, &[]).await;
        assert!(
            completion.is_ok(),
            "mock provider must accept transcript with multiple leading System messages"
        );

        let calls = provider.calls();
        let sent = calls.last().expect("provider must have been called");
        let first_non_system = sent.iter().find(|m| m.role != crate::message::Role::System);
        assert!(
            first_non_system.is_some(),
            "transcript must have a non-System message"
        );
        assert_eq!(
            first_non_system.unwrap().role,
            crate::message::Role::User,
            "first non-System must be User even with multiple leading System messages"
        );
    }
}