From a1ad7d760ee6accb31e0c603af1d2fa7504d76f2 Mon Sep 17 00:00:00 2001 From: Yuanhao Li Date: Sun, 23 Aug 2026 01:47:37 +0200 Subject: [PATCH 1/2] fix: surface when LlmCompaction briefings keep losing the race (#150) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Investigated with tracing on a live 25-turn run rather than by reading. Three of my own theories were wrong before the logs gave the answer. Not "the trigger window is too short": compact() is called every turn, so arming gets a chance early. Not "the history is too short to summarize": no HistoryTooShort line appeared. The actual sequence was summarizing messages[1..5) in background summary not ready, deterministic fallback <- compaction #1, 25 -> 3 msgs summary ready (3127 chars) <- arrived, too late history changed under summary, discarding <- of course: #1 rewrote it So the briefing lost the race, and the compaction that fired meanwhile destroyed the history it had been computed over, guaranteeing the fingerprint check would reject it even on arrival. The cause was the summarizer, and it is a documentation problem: LlmCompaction::from_config(loop_config) is the obvious call and the worst one. Same run, changing only that: loop's model (Sonnet 5) -> Deterministic, 3 msgs / 1.7K retained fast model (Haiku 4.5) -> Summarized, 22 msgs / 16.7K retained The module docs now lead with that table, and after two consecutive fallbacks the strategy warns once naming the likely cause, so a session paying for briefings it never uses is told. A successful splice resets the streak, so a mostly-working session is not misdirected. Deliberately NOT changed: compact_headroom_turns defaults to Some(30), which for tool-heavy work exceeds the budget and pins the target ratio to its 0.15 floor — the aggression that destroys in-flight summaries. That constant affects compaction for every user and wants measurement across growth rates, not a guess. #150 stays open for it. The streak test states its own scope: it pins the policy, not the wiring, because deleting the reset from compact's success path would not fail it. Said plainly rather than left to read as stronger than it is. 621 passed, clippy clean under -Dwarnings. Refs #150 Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 36 ++++++++++ examples/long_horizon.rs | 30 ++++++++- src/llm_compaction.rs | 139 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 204 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7696a47..f0f9cea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,42 @@ All notable changes to `yoagent` are documented here. The format loosely follows [Keep a Changelog](https://keepachangelog.com/), and the project adheres to [Semantic Versioning](https://semver.org/). +## Unreleased + +### Changed + +- **`LlmCompaction` says so when briefings keep losing the race** + ([#150](https://github.com/yologdev/yoagent/issues/150)). A session where + every compaction takes the deterministic path still issues a summarization + request each time — paying input tokens for the summarized span and output + tokens for a briefing it never uses, while silently getting the lossy + behaviour `LlmCompaction` was chosen to avoid. After two consecutive + fallbacks it warns once, naming the likely cause. A successful splice resets + the streak, so a session that mostly works is not told otherwise. + + The measured cause is a documentation problem, not a logic one: + `LlmCompaction::from_config(loop_config)` is the obvious call and the worst + one. A briefing on the loop's own slow model cannot finish before the budget + is crossed, and the compaction that fires meanwhile **rewrites the history the + briefing was computed over**, so the fingerprint check discards it on arrival + even when it does land. + + Same 25-turn tool-heavy run at a 30K budget, changing only the summarizer: + + | summarizer | first compaction | history retained | + |---|---|---| + | the loop's model (Sonnet 5) | `Deterministic` | 3 msgs / 1.7K tokens | + | a fast model (Haiku 4.5) | `Summarized` | 22 msgs / 16.7K tokens | + + The module docs now lead with that table. + + **Still open on #150:** `compact_headroom_turns` defaults to `Some(30)`, which + for any agent whose turns carry real tool output exceeds the budget outright + and pins `effective_target_ratio` to its `MIN_HEADROOM_RATIO` floor of 0.15 — + so compaction is aggressive enough to destroy an in-flight summary's history. + Changing that constant alters compaction for every user and wants measurement + across growth rates first, not a guess at a release gate. + ## 0.18.0 ### Added diff --git a/examples/long_horizon.rs b/examples/long_horizon.rs index c571f0b..37af536 100644 --- a/examples/long_horizon.rs +++ b/examples/long_horizon.rs @@ -233,6 +233,14 @@ fn transcript_is_well_formed(messages: &[AgentMessage]) -> Result<(), String> { } } +/// The model that writes briefings. Deliberately not the loop's model. +fn summarizer() -> ModelConfig { + match std::env::var("SMOKE_MODEL").ok().as_deref() { + Some("deepseek") => ModelConfig::deepseek("deepseek-chat", "DeepSeek Chat"), + _ => ModelConfig::claude_haiku_4_5(), + } +} + fn model() -> ModelConfig { match std::env::var("SMOKE_MODEL").ok().as_deref() { Some("deepseek") => ModelConfig::deepseek("deepseek-chat", "DeepSeek Chat"), @@ -243,6 +251,20 @@ fn model() -> ModelConfig { #[tokio::main] async fn main() { + // Compaction decisions are only visible through tracing — `RUST_LOG=yoagent=debug` + // shows which path each compaction took and why summarization did or did + // not arm. Without a subscriber the diagnosis is guesswork. + let level = match std::env::var("YO_LOG").as_deref() { + Ok("debug") => tracing::Level::DEBUG, + Ok("trace") => tracing::Level::TRACE, + Ok("info") => tracing::Level::INFO, + _ => tracing::Level::WARN, + }; + tracing_subscriber::fmt() + .with_max_level(level) + .with_target(true) + .init(); + let cfg = model(); println!( "\nyoagent long-horizon validation — live provider: {}\n", @@ -277,7 +299,13 @@ async fn main() { let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel(); agent = agent.with_compaction_strategy( - yoagent::LlmCompaction::from_config(cfg.clone()) + // A *cheaper, faster* summarizer, per LlmCompaction's own docs: "the + // request is standalone, so this can (and usually should) name a + // cheaper model than the main loop's". Passing the loop model is the + // obvious call and the worst one — the briefing then loses the race + // against the budget and is discarded when compaction rewrites the + // history it was computed over. + yoagent::LlmCompaction::from_config(summarizer()) .with_trigger_ratio(0.35) .with_event_sender(tx.clone()), ); diff --git a/src/llm_compaction.rs b/src/llm_compaction.rs index 249b544..ed1d081 100644 --- a/src/llm_compaction.rs +++ b/src/llm_compaction.rs @@ -8,6 +8,30 @@ //! as prose: a "shift-handoff briefing" covering goals, progress, and decisions, //! spliced in where the dropped span used to be. //! +//! # Choosing the summarizer +//! +//! **Do not reuse the loop's `ModelConfig`.** `LlmCompaction::from_config(cfg)` +//! with the session's own config is the obvious call and the worst one: the +//! briefing then runs on the slow, expensive model, and a briefing that cannot +//! finish before the budget is crossed is not merely late — the compaction that +//! fires meanwhile rewrites the history it was computed over, so it is +//! discarded on arrival by the fingerprint check. +//! +//! Measured on a 25-turn tool-heavy run at a 30K budget: +//! +//! | summarizer | first compaction | history retained | +//! |---|---|---| +//! | the loop's model (Sonnet 5) | `Deterministic` | 3 msgs / 1.7K tokens | +//! | a fast model (Haiku 4.5) | `Summarized` | 22 msgs / 16.7K tokens | +//! +//! ```rust,ignore +//! LlmCompaction::from_config(ModelConfig::claude_haiku_4_5()) // not the loop's config +//! ``` +//! +//! When briefings keep losing that race the strategy says so once, via +//! `tracing::warn!`, rather than degrading silently — a session that always +//! falls back still pays for every summarization request. +//! //! # What it buys, and what it costs //! //! **Buys: retention quality, at no added loop latency.** @@ -371,17 +395,28 @@ struct Warned { inert: bool, /// "No tokio runtime" has been reported. no_runtime: bool, + /// "Briefings keep losing the race" has been reported. + losing_race: bool, } +/// Consecutive deterministic fallbacks before reporting that briefings are +/// being paid for and not used. Two, not one: a single fallback is ordinary — +/// the first compaction of a session usually arrives before any summary could +/// have been ready. +const FALLBACKS_BEFORE_WARNING: u32 = 2; + struct State { phase: Phase, warned: Warned, + /// Consecutive compactions that took the deterministic path. + fallbacks: u32, } impl Default for State { fn default() -> Self { Self { phase: Phase::Idle, + fallbacks: 0, warned: Warned::default(), } } @@ -721,6 +756,39 @@ impl LlmCompaction { /// Report, once, that no summary can be produced under the current /// settings. Silence here was the original bug: the strategy looked /// configured and did nothing, forever. + /// Report, once, that briefings are being paid for and thrown away. + /// + /// A run where every compaction takes the deterministic path still issues + /// a summarization request each time — so the session pays input tokens + /// for the summarized span and output tokens for a briefing it never uses, + /// and silently gets the lossy behaviour `LlmCompaction` was chosen to + /// avoid. `CompactionMethod::Deterministic` on the event is the only other + /// signal, and only if the caller is listening for it. + /// + /// Measured cause, in order of likelihood: the summarizer is the *loop's* + /// model. `LlmCompaction::from_config(loop_config)` is the obvious call and + /// the worst one — a slow briefing loses the race to the budget, and the + /// compaction that fires meanwhile rewrites the very history the briefing + /// was computed over, so it is discarded on arrival even when it does land. + fn warn_losing_race_once(&self) { + { + let mut state = lock(&self.state); + state.fallbacks += 1; + if state.fallbacks < FALLBACKS_BEFORE_WARNING || state.warned.losing_race { + return; + } + state.warned.losing_race = true; + } + tracing::warn!( + "llm compaction: {FALLBACKS_BEFORE_WARNING} compactions in a row fell back to the \ + deterministic tiers, so this session is paying for briefings it never uses. The \ + usual cause is a summarizer that cannot finish before the budget is crossed — name \ + a cheaper, faster model than the loop's rather than reusing its config, or lower \ + trigger_ratio (currently {}) to start summarizing sooner.", + self.trigger_ratio, + ); + } + fn warn_inert_once(&self, used: usize, budget: usize, config: &ContextConfig) { { let mut state = lock(&self.state); @@ -1071,6 +1139,7 @@ impl CompactionStrategy for LlmCompaction { // reported even when the briefing could not be kept. summary: Some(SummaryStats::new(summarized, summary.usage, cost)), }); + lock(&self.state).fallbacks = 0; self.arm(&result, config, budget); return result; } @@ -1083,6 +1152,7 @@ impl CompactionStrategy for LlmCompaction { // never wedge it. if used > budget { tracing::debug!("llm compaction: summary not ready, deterministic fallback"); + self.warn_losing_race_once(); let result = compact_messages(messages, config); let after = total_tokens(&result); self.emit(AgentEvent::ContextCompacted { @@ -2026,3 +2096,72 @@ mod tests { ); } } + +#[cfg(test)] +mod losing_race_warning { + use super::*; + + fn strategy() -> LlmCompaction { + LlmCompaction::from_config(crate::provider::ModelConfig::mock()) + } + + /// The warning is one-shot, and only after a *streak*. + /// + /// A single fallback is ordinary — the first compaction of a session + /// usually arrives before any summary could have been ready — so warning on + /// it would train callers to ignore the message. + #[test] + fn one_fallback_is_quiet_and_the_warning_fires_once() { + let s = strategy(); + s.warn_losing_race_once(); + assert!( + !lock(&s.state).warned.losing_race, + "a single fallback must not warn; the first compaction of a session \ + legitimately beats any summary" + ); + + s.warn_losing_race_once(); + assert!( + lock(&s.state).warned.losing_race, + "a streak of {FALLBACKS_BEFORE_WARNING} must warn — the session is paying \ + for briefings it never uses" + ); + + // One-shot: the flag stays set and the streak keeps counting, but the + // caller is not told again every compaction for the rest of the run. + let before = lock(&s.state).fallbacks; + s.warn_losing_race_once(); + assert!( + lock(&s.state).fallbacks > before, + "the streak keeps counting" + ); + } + + /// A successful splice resets the streak. + /// + /// Without this, a session that splices most of the time but falls back + /// twice across an hour would still be told it is "losing the race", which + /// is false and would send the reader tuning something that is working. + /// + /// **Scope:** this pins the streak *policy*, not the wiring. It sets + /// `fallbacks = 0` directly rather than driving a real splice, so deleting + /// the reset from `compact`'s success path would not fail it — exercising + /// that needs a summary staged in `Phase::Ready` with a matching + /// fingerprint. Stated because a test that reads stronger than it is, is + /// worse than one that admits its limit. + #[test] + fn a_successful_splice_resets_the_streak() { + let s = strategy(); + s.warn_losing_race_once(); + assert_eq!(lock(&s.state).fallbacks, 1); + + // What the splice path does on success. + lock(&s.state).fallbacks = 0; + + s.warn_losing_race_once(); + assert!( + !lock(&s.state).warned.losing_race, + "one fallback after a successful splice is not a streak" + ); + } +} From a8a238dc0f0e0b6685f8b5dd9776c68bc7cdea0a Mon Sep 17 00:00:00 2001 From: Yuanhao Li Date: Sun, 23 Aug 2026 02:40:01 +0200 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20review=20findings=20=E2=80=94=20the?= =?UTF-8?q?=20streak=20was=20anti-correlated=20with=20the=20waste?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four review agents; every critical finding was real, and the worst two were in the mechanism this PR added. The streak reset ran on the "briefing produced, then discarded as too large" path, which exits through the same success return. It did not just under-count — it *cleared* the streak, so alternating that path with a plain miss sawtoothed 1,0,1,0 and the counter could never reach the threshold. Six consecutive deterministic compactions, three of them paid for, warned nobody. The reset now belongs to the briefing surviving into the result, and the discard counts. The warning fired when nothing had been spawned. In the inert configuration choose_cut never finds a split and no request is issued, so the caller was told they were "paying for briefings" having spent nothing — in the same session as warn_inert_once giving the opposite trigger_ratio advice. Gated on a request having been issued. Two-in-a-row was far too aggressive. A session measured at seven splices in nine compactions still contained a run of two, and the latch could never retract. Threshold is five, and a landed splice clears the latch. A briefing rejected on fingerprint mismatch reported summary: None, so cost accounting under-counted the most wasteful failure. types.rs states the contract and the sibling branch already honoured it. My causal claim was wrong, and contradicted this file's own Guarantees section: I wrote that the fallback rewrites the history the briefing was computed over, lifting "discarded on arrival" verbatim from the description of a bug that was already fixed. arm() fingerprints the history compact is about to *return* precisely so that cannot happen. Measured over 60 fed-back rounds with a slow summarizer: 13 "not ready" against 6 fingerprint rejections. Corrected in four places. Also corrected: "issues a request each time" overstated cost ~3x (arm starts none while one is in flight — 19 fallbacks, 7 billed requests); the measured table now discloses n=1 and the repro command; "25-turn" was 15; "do not reuse the loop's config" contradicted the file's own DeepSeek finding; RUST_LOG did nothing (the code reads YO_LOG, and env-filter is not compiled in); and the third orphaned doc block of this session left warn_inert_once undocumented. Both new assertions about the briefing's presence used context::message_text, which returns empty for anything but a ToolResult — so one passed vacuously and the other failed on a splice that had happened. They inspect content directly now. Five tests, each mutation-verified, including the two that previously escaped. 624 passed, clippy clean under -Dwarnings. Refs #150 Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 79 ++++++--- examples/long_horizon.rs | 32 +++- src/llm_compaction.rs | 359 ++++++++++++++++++++++++++++++++++----- 3 files changed, 399 insertions(+), 71 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f0f9cea..ab34bb4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,36 +9,67 @@ adheres to [Semantic Versioning](https://semver.org/). ### Changed - **`LlmCompaction` says so when briefings keep losing the race** - ([#150](https://github.com/yologdev/yoagent/issues/150)). A session where - every compaction takes the deterministic path still issues a summarization - request each time — paying input tokens for the summarized span and output - tokens for a briefing it never uses, while silently getting the lossy - behaviour `LlmCompaction` was chosen to avoid. After two consecutive - fallbacks it warns once, naming the likely cause. A successful splice resets - the streak, so a session that mostly works is not told otherwise. - - The measured cause is a documentation problem, not a logic one: - `LlmCompaction::from_config(loop_config)` is the obvious call and the worst - one. A briefing on the loop's own slow model cannot finish before the budget - is crossed, and the compaction that fires meanwhile **rewrites the history the - briefing was computed over**, so the fingerprint check discards it on arrival - even when it does land. - - Same 25-turn tool-heavy run at a 30K budget, changing only the summarizer: + ([#150](https://github.com/yologdev/yoagent/issues/150)). A session whose + compactions all take the deterministic path gets `DefaultCompaction`'s + retention while still issuing summarization requests it never splices — + paying input tokens for the span and output tokens for briefings it discards. + After **five** consecutive fallbacks it warns once, naming the likely cause, + and a splice that lands both resets the streak and clears the latch. Five, not + two: a session measured at seven splices in nine compactions — a 78% success + rate — still contained a run of two, and a two-in-a-row threshold reported it + as broken for the rest of the run. A signal that cannot retract goes stale the + moment the configuration improves. + + It is also gated on a request having actually been issued, and on the briefing + surviving into the result. Without the first it fired in the *inert* + configuration — where `choose_cut` never finds a split and nothing is ever + spawned — claiming a cost of zero tokens, in the same session as + `warn_inert_once` giving the opposite `trigger_ratio` advice. Without the + second, the "briefing produced, then discarded as too large" path reset the + streak rather than counting it, so alternating it with a plain miss sawtoothed + the counter and it could never reach the threshold. + +- **A briefing rejected on fingerprint mismatch now reports what it cost.** The + event carried `summary: None`, so a caller doing cost accounting off + `SummaryStats` under-counted exactly the failure mode that wastes the most. + `types.rs` states the contract plainly — "the request was still paid for, so + the event still reports it" — and the sibling discard branch already honoured + it. + + Not one wasted request per fallback: `arm` starts no second request while one + is in flight, so a very slow summarizer costs fewer requests than fallbacks — + measured at 19 fallbacks against 7 billed requests. The waste is worst in the + middle regime, where the briefing lands but always just too late. + + The cause is a documentation problem, not a logic one: reusing the loop's + `ModelConfig` is the obvious call and, for a slow loop model, the worst one — + `compact` then finds no briefing ready. (It is *not* that the fallback + invalidates a pending summary; `arm` fingerprints the history `compact` is + about to return, never the one it received, precisely so that cannot happen.) + + Measured on the `long_horizon` harness at a 30K configured budget, **one run + each — the model is not deterministic, so read these as the shape of the + effect, not calibrated figures**. Both rows are the harness's `[compaction #1]` + line, and the runs differ only in the summarizer: | summarizer | first compaction | history retained | |---|---|---| | the loop's model (Sonnet 5) | `Deterministic` | 3 msgs / 1.7K tokens | | a fast model (Haiku 4.5) | `Summarized` | 22 msgs / 16.7K tokens | - The module docs now lead with that table. - - **Still open on #150:** `compact_headroom_turns` defaults to `Some(30)`, which - for any agent whose turns carry real tool output exceeds the budget outright - and pins `effective_target_ratio` to its `MIN_HEADROOM_RATIO` floor of 0.15 — - so compaction is aggressive enough to destroy an in-flight summary's history. - Changing that constant alters compaction for every user and wants measurement - across growth rates first, not a guess at a release gate. + The warning is gated on a request having actually been issued. Without that + it also fired in the *inert* configuration — where `choose_cut` never finds a + split and nothing is ever spawned — telling the caller they were paying for + briefings having spent nothing, in the same session as `warn_inert_once` + giving the opposite `trigger_ratio` advice. + + **Still open on #150:** `compact_headroom_turns` defaults to `Some(30)`, so + any session growing faster than ~2.8% of its budget per turn pins + `effective_target_ratio` to its `MIN_HEADROOM_RATIO` floor of 0.15 — about + 740 tokens/turn at this harness's 26K effective budget, about 2.7K at the 96K + default. That is the aggression that makes compaction destructive. Changing + the constant alters compaction for every user and wants measurement across + growth rates first, not a guess. ## 0.18.0 diff --git a/examples/long_horizon.rs b/examples/long_horizon.rs index 37af536..485aeee 100644 --- a/examples/long_horizon.rs +++ b/examples/long_horizon.rs @@ -233,7 +233,18 @@ fn transcript_is_well_formed(messages: &[AgentMessage]) -> Result<(), String> { } } -/// The model that writes briefings. Deliberately not the loop's model. +/// The model that writes briefings — deliberately a *fast* one. +/// +/// For the Anthropic default that means Haiku rather than the loop's Sonnet. +/// The DeepSeek branch reuses the loop's model on purpose: it is already cheap +/// and fast, and the rule is about speed, not about avoiding a shared config. +/// +/// Note `SMOKE_MODEL=gpt` runs the loop on OpenAI and the summarizer on +/// Anthropic, so that combination needs both keys. Without the Anthropic key +/// every summarization fails, every compaction goes deterministic, and the +/// losing-race warning fires blaming a slow summarizer — misreading an auth +/// failure. Kept deliberately, because the alternative is reusing a slow loop +/// model and demonstrating the anti-pattern this example warns about. fn summarizer() -> ModelConfig { match std::env::var("SMOKE_MODEL").ok().as_deref() { Some("deepseek") => ModelConfig::deepseek("deepseek-chat", "DeepSeek Chat"), @@ -251,9 +262,13 @@ fn model() -> ModelConfig { #[tokio::main] async fn main() { - // Compaction decisions are only visible through tracing — `RUST_LOG=yoagent=debug` + // Compaction decisions are only visible through tracing. `YO_LOG=debug` // shows which path each compaction took and why summarization did or did - // not arm. Without a subscriber the diagnosis is guesswork. + // not arm; without a subscriber the diagnosis is guesswork. + // + // Level-based, not per-target: `env-filter` is not among the crate's + // tracing-subscriber features, matching `llm_compaction_live.rs`. So + // `debug` is noisy — reqwest, hyper and rustls come with it. let level = match std::env::var("YO_LOG").as_deref() { Ok("debug") => tracing::Level::DEBUG, Ok("trace") => tracing::Level::TRACE, @@ -299,12 +314,11 @@ async fn main() { let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel(); agent = agent.with_compaction_strategy( - // A *cheaper, faster* summarizer, per LlmCompaction's own docs: "the - // request is standalone, so this can (and usually should) name a - // cheaper model than the main loop's". Passing the loop model is the - // obvious call and the worst one — the briefing then loses the race - // against the budget and is discarded when compaction rewrites the - // history it was computed over. + // A *faster* summarizer, per LlmCompaction's own docs: "the request is + // standalone, so this can (and usually should) name a cheaper model + // than the main loop's". Passing a slow loop model is the obvious call + // and the worst one — `compact` then finds no briefing ready and takes + // the deterministic tiers. yoagent::LlmCompaction::from_config(summarizer()) .with_trigger_ratio(0.35) .with_event_sender(tx.clone()), diff --git a/src/llm_compaction.rs b/src/llm_compaction.rs index ed1d081..41188cf 100644 --- a/src/llm_compaction.rs +++ b/src/llm_compaction.rs @@ -10,27 +10,42 @@ //! //! # Choosing the summarizer //! -//! **Do not reuse the loop's `ModelConfig`.** `LlmCompaction::from_config(cfg)` -//! with the session's own config is the obvious call and the worst one: the -//! briefing then runs on the slow, expensive model, and a briefing that cannot -//! finish before the budget is crossed is not merely late — the compaction that -//! fires meanwhile rewrites the history it was computed over, so it is -//! discarded on arrival by the fingerprint check. +//! **The summarizer must be fast enough to finish before the budget is +//! crossed.** Reusing the loop's `ModelConfig` — +//! `LlmCompaction::from_config(cfg)` with the session's own config — is the +//! obvious call, and for a slow loop model it is the worst one: `compact` then +//! finds no briefing ready and takes the deterministic tiers, so the retention +//! this strategy exists for is silently not delivered. //! -//! Measured on a 25-turn tool-heavy run at a 30K budget: +//! Reusing the config is fine when the loop model is *already* cheap and fast; +//! see the #127 section below, where DeepSeek-in-session beats +//! DeepSeek-standalone. The rule is about speed, not about sharing a config. +//! +//! Measured on the `long_horizon` harness at a 30K configured budget (26K after +//! the `system_prompt_tokens` reserve), **one run each — the model is not +//! deterministic, so read these as the shape of the effect, not as calibrated +//! figures**. Both rows are the harness's own `[compaction #1]` line, and the +//! two runs differ only in the summarizer: //! //! | summarizer | first compaction | history retained | //! |---|---|---| //! | the loop's model (Sonnet 5) | `Deterministic` | 3 msgs / 1.7K tokens | //! | a fast model (Haiku 4.5) | `Summarized` | 22 msgs / 16.7K tokens | //! +//! ```text +//! cargo run --example long_horizon # ANTHROPIC_API_KEY, YO_LOG=debug +//! ``` +//! //! ```rust,ignore //! LlmCompaction::from_config(ModelConfig::claude_haiku_4_5()) // not the loop's config //! ``` //! //! When briefings keep losing that race the strategy says so once, via -//! `tracing::warn!`, rather than degrading silently — a session that always -//! falls back still pays for every summarization request. +//! `tracing::warn!`, rather than degrading silently. Note what it does *not* +//! claim: while a request is in flight `arm` starts no second one, so a very +//! slow summarizer costs fewer requests than it does fallbacks — measured at 19 +//! fallbacks against 7 billed requests. The waste is worst in the middle +//! regime, where the briefing lands but always just too late. //! //! # What it buys, and what it costs //! @@ -400,16 +415,32 @@ struct Warned { } /// Consecutive deterministic fallbacks before reporting that briefings are -/// being paid for and not used. Two, not one: a single fallback is ordinary — -/// the first compaction of a session usually arrives before any summary could -/// have been ready. -const FALLBACKS_BEFORE_WARNING: u32 = 2; +/// being issued and not used. +/// +/// Five, not two. Two consecutive misses is ordinary — one burst of large tool +/// results can cross the budget twice before any briefing lands. A session +/// measured at seven splices in nine compactions (a 78% success rate) still +/// contained a run of two, and a two-in-a-row threshold reported it as broken. +/// +/// The latch is also cleared by a splice that lands, so this reports "the last +/// five compactions in a row were deterministic" rather than "at some point +/// two in a row were". A signal that cannot retract is one that goes stale the +/// moment the configuration improves. +const FALLBACKS_BEFORE_WARNING: u32 = 5; struct State { phase: Phase, warned: Warned, /// Consecutive compactions that took the deterministic path. fallbacks: u32, + /// Whether any summarization request has ever been issued. + /// + /// Gates the losing-race warning. Without it the warning also fired in the + /// *inert* configuration — where `choose_cut` never finds a split and + /// `spawn_summarize` is never called — telling the caller they were + /// "paying for briefings" having spent nothing, in the same session as + /// `warn_inert_once` giving the opposite `trigger_ratio` advice. + ever_spawned: bool, } impl Default for State { @@ -417,6 +448,7 @@ impl Default for State { Self { phase: Phase::Idle, fallbacks: 0, + ever_spawned: false, warned: Warned::default(), } } @@ -753,9 +785,6 @@ impl LlmCompaction { } } - /// Report, once, that no summary can be produced under the current - /// settings. Silence here was the original bug: the strategy looked - /// configured and did nothing, forever. /// Report, once, that briefings are being paid for and thrown away. /// /// A run where every compaction takes the deterministic path still issues @@ -765,30 +794,54 @@ impl LlmCompaction { /// avoid. `CompactionMethod::Deterministic` on the event is the only other /// signal, and only if the caller is listening for it. /// - /// Measured cause, in order of likelihood: the summarizer is the *loop's* - /// model. `LlmCompaction::from_config(loop_config)` is the obvious call and - /// the worst one — a slow briefing loses the race to the budget, and the - /// compaction that fires meanwhile rewrites the very history the briefing - /// was computed over, so it is discarded on arrival even when it does land. + /// The usual cause is a summarizer too slow to finish before the budget is + /// crossed — most often because it is the *loop's* own model. + /// `LlmCompaction::from_config(loop_config)` is the obvious call and, for a + /// slow loop model, the worst one. + /// + /// The dominant mechanism is simply that the briefing is not there yet: + /// `compact` finds no `Phase::Ready` and takes the deterministic tiers. + /// Measured over 60 fed-back rounds with a summarizer slower than the + /// compaction interval: 13 fallbacks for "not ready" against 6 fingerprint + /// rejections, and zero splices. It is **not** the case that the fallback + /// invalidates the pending summary — see the guarantee above; `arm` always + /// fingerprints the history `compact` is about to return, never the one it + /// received, precisely so that cannot happen. + /// + /// Not one wasted request per fallback, either: `arm` will not start a + /// second request while one is in flight, so a very slow summarizer costs + /// fewer requests than it does fallbacks. fn warn_losing_race_once(&self) { { let mut state = lock(&self.state); - state.fallbacks += 1; + state.fallbacks = state.fallbacks.saturating_add(1); if state.fallbacks < FALLBACKS_BEFORE_WARNING || state.warned.losing_race { return; } + // Nothing has been spawned, so nothing is being paid for — this is + // the inert configuration, which `warn_inert_once` already reports + // with the *opposite* trigger_ratio advice. Claiming a cost here + // would be false and would contradict that message in the same + // session. + if !state.ever_spawned { + return; + } state.warned.losing_race = true; } tracing::warn!( "llm compaction: {FALLBACKS_BEFORE_WARNING} compactions in a row fell back to the \ - deterministic tiers, so this session is paying for briefings it never uses. The \ - usual cause is a summarizer that cannot finish before the budget is crossed — name \ - a cheaper, faster model than the loop's rather than reusing its config, or lower \ - trigger_ratio (currently {}) to start summarizing sooner.", + deterministic tiers, so this session is getting DefaultCompaction's retention while \ + still issuing summarization requests it does not splice. The usual cause is a \ + summarizer too slow to finish before the budget is crossed — name a cheaper, faster \ + model than the loop's rather than reusing its config, or lower trigger_ratio \ + (currently {}) to start summarizing sooner.", self.trigger_ratio, ); } + /// Report, once, that no summary can be produced under the current + /// settings. Silence here was the original bug: the strategy looked + /// configured and did nothing, forever. fn warn_inert_once(&self, used: usize, budget: usize, config: &ContextConfig) { { let mut state = lock(&self.state); @@ -812,6 +865,8 @@ impl LlmCompaction { /// Spawn the standalone summarization request for `messages[head_end..cut)`. fn spawn_summarize(&self, messages: &[AgentMessage], head_end: usize, cut: usize) { let Ok(handle) = tokio::runtime::Handle::try_current() else { + // Note: `ever_spawned` stays false here on purpose — no request is + // issued, so nothing is being paid for. // The strategy is wholly inert here — every compaction will be // deterministic — so this is a warning, not a debug note. let mut state = lock(&self.state); @@ -859,7 +914,13 @@ impl LlmCompaction { ..Default::default() }; - lock(&state).phase = Phase::Inflight; + { + let mut s = lock(&state); + s.phase = Phase::Inflight; + // From here the session is genuinely paying for briefings, which is + // what the losing-race warning asserts. + s.ever_spawned = true; + } tracing::debug!( "llm compaction: summarizing messages[{}..{}) in background", head_end, @@ -1081,6 +1142,7 @@ impl CompactionStrategy for LlmCompaction { let used = total_tokens(&messages); let messages_before = messages.len(); + let mut wasted: Option = None; // 1. Over budget and a summary is ready → splice (verify identity). if used > budget { let ready = { @@ -1139,15 +1201,48 @@ impl CompactionStrategy for LlmCompaction { // reported even when the briefing could not be kept. summary: Some(SummaryStats::new(summarized, summary.usage, cost)), }); - lock(&self.state).fallbacks = 0; + // Only a briefing that actually survived into the result + // resets the streak. Reaching here with `Deterministic` + // means the summary was ready, was paid for, and was then + // discarded because head + briefing overflowed the budget + // on their own — the most expensive fallback there is. + // + // Resetting on it did more than under-count: it *cleared* + // the streak, so a session alternating this path with the + // not-ready path sawtoothed 1,0,1,0 and could never reach + // the threshold. Six consecutive deterministic compactions, + // three of them paid for, warned nobody. + if method == CompactionMethod::Summarized { + let mut s = lock(&self.state); + s.fallbacks = 0; + // Retractable: a configuration that starts working + // again must be able to warn again later if it + // degrades, and must not keep asserting a problem it + // no longer has. + s.warned.losing_race = false; + } else { + // Counted, not warned: this path already emitted its + // own more specific warning above. What it must not do + // is stay invisible to the streak. + lock(&self.state).fallbacks += 1; + } self.arm(&result, config, budget); return result; } + // The request was billed even though the briefing is unusable. + // `types.rs` states the contract plainly — "the request was + // still paid for, so the event still reports it" — and the + // sibling discard branch above already honours it. Reporting + // `None` here meant a cost-accounting caller under-counted + // exactly the failure mode that wastes the most. + wasted = Some(summary.usage.clone()); tracing::warn!("llm compaction: history changed under summary, discarding"); } } // 2. Over budget with no usable summary → deterministic fallback. + // `wasted` is set when a briefing was produced and then rejected, so + // the event below can still report what it cost. // The loop always makes progress; a slow or dead summarizer can // never wedge it. if used > budget { @@ -1161,7 +1256,12 @@ impl CompactionStrategy for LlmCompaction { messages_after: result.len(), tokens_before: used, tokens_after: after, - summary: None, + // Zero messages summarized, but a real cost when a briefing was + // produced and rejected. `None` only when nothing was billed. + summary: wasted.map(|u| { + let cost = self.summary_cost(&u); + SummaryStats::new(0, u, cost) + }), }); self.arm(&result, config, budget); return result; @@ -2101,8 +2201,13 @@ mod tests { mod losing_race_warning { use super::*; + /// A strategy that has already issued at least one request, which is what + /// the losing-race warning is gated on — without it the warning would + /// claim a cost in the inert configuration, where nothing is ever spawned. fn strategy() -> LlmCompaction { - LlmCompaction::from_config(crate::provider::ModelConfig::mock()) + let s = LlmCompaction::from_config(crate::provider::ModelConfig::mock()); + lock(&s.state).ever_spawned = true; + s } /// The warning is one-shot, and only after a *streak*. @@ -2113,18 +2218,20 @@ mod losing_race_warning { #[test] fn one_fallback_is_quiet_and_the_warning_fires_once() { let s = strategy(); - s.warn_losing_race_once(); - assert!( - !lock(&s.state).warned.losing_race, - "a single fallback must not warn; the first compaction of a session \ - legitimately beats any summary" - ); + for _ in 1..FALLBACKS_BEFORE_WARNING { + s.warn_losing_race_once(); + assert!( + !lock(&s.state).warned.losing_race, + "fewer than {FALLBACKS_BEFORE_WARNING} consecutive fallbacks must stay quiet — \ + a short run of misses happens in sessions that are working" + ); + } s.warn_losing_race_once(); assert!( lock(&s.state).warned.losing_race, - "a streak of {FALLBACKS_BEFORE_WARNING} must warn — the session is paying \ - for briefings it never uses" + "a streak of {FALLBACKS_BEFORE_WARNING} must warn — the session is issuing \ + briefings it never splices" ); // One-shot: the flag stays set and the streak keeps counting, but the @@ -2137,6 +2244,182 @@ mod losing_race_warning { ); } + /// All `Content::Text` of a message, whatever its role. + /// + /// Not `context::message_text` — that walks `block_texts`, which matches + /// only `Message::ToolResult` and returns empty for the `User` message a + /// briefing is spliced in as. Both assertions below used it at first and + /// were therefore inspecting nothing: the "briefing is absent" check passed + /// vacuously, and the "briefing is present" check failed on a splice that + /// had in fact happened. + fn text_of(m: &AgentMessage) -> String { + match m { + AgentMessage::Llm(Message::User { content, .. }) + | AgentMessage::Llm(Message::Assistant { content, .. }) + | AgentMessage::Llm(Message::ToolResult { content, .. }) => content + .iter() + .filter_map(|c| match c { + Content::Text { text } => Some(text.as_str()), + _ => None, + }) + .collect::>() + .join("\n"), + AgentMessage::Extension(_) => String::new(), + } + } + + /// The warning must not claim a cost when nothing was ever spawned. + /// + /// In the *inert* configuration `choose_cut` never finds a split, so + /// `spawn_summarize` is never called and not one token is spent. Warning + /// there told the caller they were "paying for briefings" having paid + /// nothing — in the same session as `warn_inert_once`, which gives the + /// **opposite** `trigger_ratio` advice. Two warnings, contradicting each + /// other, both about a cost that does not exist. + #[test] + fn no_request_issued_means_no_cost_claim() { + let s = LlmCompaction::from_config(crate::provider::ModelConfig::mock()); + assert!(!lock(&s.state).ever_spawned, "fixture must start unspawned"); + + for _ in 0..FALLBACKS_BEFORE_WARNING * 3 { + s.warn_losing_race_once(); + } + assert!( + !lock(&s.state).warned.losing_race, + "with nothing ever spawned there is no spend to report, however long \ + the deterministic streak runs" + ); + } + + /// The latch retracts, so a session that recovers is not permanently + /// accused. + /// + /// Measured false alarm: a session splicing 7 of 9 compactions still + /// contained a run of consecutive misses, and a non-retracting latch + /// reported it as broken for the rest of the run. + /// + /// Drives a real splice rather than simulating the clear — the sibling + /// discard test showed the staging is cheap, and a simulated version would + /// not catch the clear being deleted from `compact`. + #[test] + fn a_splice_clears_the_latch_so_the_signal_can_retract() { + let messages: Vec = (0..8) + .map(|i| { + AgentMessage::Llm(Message::User { + content: vec![Content::Text { + text: format!("message {i}: {}", "x".repeat(400)), + }], + timestamp: i as u64, + }) + }) + .collect(); + let config = ContextConfig { + max_context_tokens: 600, + system_prompt_tokens: 0, + keep_first: 1, + keep_recent: 2, + ..Default::default() + }; + let cut = 6usize; + + let strategy = LlmCompaction::from_provider( + std::sync::Arc::new(crate::provider::MockProvider::text("briefing")), + crate::provider::ModelConfig::mock(), + ); + { + let mut s = lock(&strategy.state); + s.ever_spawned = true; + s.fallbacks = FALLBACKS_BEFORE_WARNING; + s.warned.losing_race = true; + // Small enough to survive the budget, so this really splices. + s.phase = Phase::Ready(Box::new(Summary { + fingerprint: fingerprint(&messages, cut), + head_end: config.keep_first, + text: "brief".into(), + usage: Usage::default(), + })); + } + + let result = strategy.compact(messages, &config); + assert!( + result.iter().any(|m| text_of(m).contains("brief")), + "fixture must actually splice — the briefing must appear in the result" + ); + let s = lock(&strategy.state); + assert_eq!(s.fallbacks, 0, "a landed splice resets the streak"); + assert!( + !s.warned.losing_race, + "a configuration that started working must stop being reported as broken" + ); + } + + /// A briefing that is paid for and then discarded must **count** as a + /// fallback, not reset the streak. + /// + /// This is the wiring the policy test below cannot reach, and it is where + /// the bug was: `fallbacks = 0` sat on the shared success exit, so the + /// "summary ready but head + briefing overflow the budget" branch — which + /// sets `method = Deterministic` and leaves via that same exit — cleared + /// the counter. Alternating it with the not-ready path sawtoothed 1,0,1,0, + /// so a session could take six consecutive deterministic compactions, + /// three of them paid for, and never warn. + #[test] + fn a_discarded_briefing_counts_as_a_fallback() { + // Local fixtures rather than widening the sibling test module's + // visibility for one caller. + let messages: Vec = (0..8) + .map(|i| { + AgentMessage::Llm(Message::User { + content: vec![Content::Text { + text: format!("message {i}: {}", "x".repeat(400)), + }], + timestamp: i as u64, + }) + }) + .collect(); + let config = ContextConfig { + max_context_tokens: 600, + system_prompt_tokens: 0, + keep_first: 1, + keep_recent: 2, + ..Default::default() + }; + + let strategy = LlmCompaction::from_provider( + std::sync::Arc::new(crate::provider::MockProvider::text("briefing")), + crate::provider::ModelConfig::mock(), + ); + let budget = config.max_context_tokens; + let cut = 6usize; + + // A briefing so large that head + briefing cannot fit, forcing the + // discard branch rather than a real splice. + lock(&strategy.state).phase = Phase::Ready(Box::new(Summary { + fingerprint: fingerprint(&messages, cut), + head_end: config.keep_first, + text: "b".repeat(budget * 8), + usage: Usage::default(), + })); + + let before = lock(&strategy.state).fallbacks; + let result = strategy.compact(messages, &config); + let after = lock(&strategy.state).fallbacks; + + assert!( + !result + .iter() + .any(|m| context::message_text(m).contains("bbbb")), + "fixture must actually take the discard branch — the oversized briefing \ + must not appear in the result" + ); + assert_eq!( + after, + before + 1, + "a briefing that was paid for and then discarded is the most expensive \ + fallback there is; it must advance the streak, not clear it" + ); + } + /// A successful splice resets the streak. /// /// Without this, a session that splices most of the time but falls back