fix: a turn is not a unit of size, and a summary of a summary is not a compaction

CHANGELOG.md

@@ -15,6 +15,8 @@ All notable changes to smith. The format follows [Keep a Changelog](https://keep
 
 ### Fixed
 
+- **Compaction no longer summarizes its own summary while the turn that is actually growing sits untouched**: the recency window counts *turns*, and a long agentic run — one prompt, then a hundred tool calls — is a single turn, so the window that exists to protect the last three turns covered the entire history instead. All four cheap stages became no-ops (`window_start` returns 0 once there are no more real turns than the window is wide, and every stage breaks on the first message), `safe_cut_index` found no boundary whose tail fit and fell back to the newest one, and the prefix left to summarize was — from the second compaction onward — the summary the first one had written, and nothing else. The reported result was `~105421 → ~105341 tokens, 0% of the budget reclaimed · summarize`, every turn, each one paying for a provider call and a full prompt-cache invalidation to swap one summary for another while the 76k of tool results behind the cut were never a candidate. The escape hatch that ignores the recency window already existed but was wired to `cut.zero?` — the one shape this case never takes. It now runs whenever the tail that would survive the cut is itself over target, which is the same dead end reached by a different road: `cut.zero?` is the case where there is no boundary, this is the case where no boundary helps. Afterwards the boundary is re-chosen against the shortened history, and a history that now fits is returned as it is rather than summarized for nothing. Two guards stand behind that: a prefix that is only a previous summary is refused before the call is made, recognised by the `SUMMARY_PREFIX` the writer and the reader now share rather than by a flag, which a resumed session's JSON round trip would not carry; and a summary that comes back longer than the turns it replaced is discarded, because paying for the call, losing the detail and growing the request is the one outcome with nothing to recommend it. On the reproduction — a short turn, then thirty 8 KB tool results — the same history now goes `60212 → 34249`, 74% of the budget reclaimed, target reached, and no provider call at all (#118).
+
 - **A resumed session no longer forgets what earlier runs cost**: `Agent` counts tokens for a *run* — its counter is built at zero and nothing seeds it — while the session record holds a *lifetime*, and the loop's write-back assigned the one straight onto the other. So the first turn back overwrote everything a session had ever spent with the few hundred tokens of the turn just taken, and `smith sessions`, `smith stats` and `smith sessions export` reported the last run as though it were the whole session. The record now keeps the total it stood at when its agent was built and writes that plus the run's counter, so a session resumed twice shows all three runs in the index. A sum against a baseline rather than an addition, deliberately: the write-back runs after *every* turn and the counter is the run's running total, not the turn's increment, so `+=` would have counted turn one again on turn two, and again on turn three. The baseline is taken where the counter is born — in `build_agent`, the one place all five routes to a new agent pass through, `/resume` inside either running loop among them, which is the route a baseline taken at startup misses and the one that silently wiped the session being switched to. It rides on the session rather than on the CLI because such a switch rebuilds the agent before it re-points the interrupt handler and can yield in between, so a `^C` dispatched in that window can still reach a handler holding the session being *left* — which has to be written against its own baseline rather than the incoming one's. Nothing the agent counts changed, and neither did the two things that read it: `--max-budget-usd` is still a per-run limit through `spent_usd`, and the token line at the end of a run still reports that run rather than quietly becoming a lifetime figure (#102).
 - **`/clear` followed by `/resume` no longer destroys the session you left**: `/clear` empties the agent's conversation, and the loop's write-back copies that conversation onto the session record — so switching away straight after a clear wrote an emptied `session.json` and a `0` message count into the index, without asking and without a word. The `transcript.jsonl` beside it survived, being append-only, but nothing else did. `/clear` now writes nothing at all: it starts the running context over, and the record keeps what is on disk until a real turn adds something — so `/clear` plus a turn is still a fresh start, while `/clear` plus a switch loses nothing. The todo list and the calibration ratio are kept with the transcript they belong to, rather than being replaced by the empty list and the neutral ratio a clear leaves behind. The guard sits in the one method every persisting path calls, rather than at the call site the report happened to find — `/resume` in the plain loop and in the fullscreen one, the `^C` handler, the second `^C` in the fullscreen UI, that loop's exit and a prompt a `user_prompt_submit` hook blocks are each such a path, and that list is illustrative rather than exhaustive. It also covers a case the report thought was already safe: `/clear` then `/quit` costs nothing in the plain loop, which does not persist on the way out, but the fullscreen loop persists after `app.run` returns and lost the session the same way. Marked on the agent rather than inferred from an empty message list, which cannot tell a session that was cleared from one that was never used — and those two have to be written differently (#105).
 - **A `mode:` smith cannot read now costs a subagent its tools, not the reader the promise**: `mode: inspekt`, `mode: read-only` — anything that was not `work` or `inspect` — resolved to `work`, so a definition whose author was asking for a read-only agent got `bash`, `write_file` and `edit_file` instead, and nothing said so. `smith agents list` printed `work` as though the file had asked for it, which made the one view that exists to show configuration the view that hid the typo. An unreadable value falls back to `inspect` now — the conservative direction, since a mode is a security statement and a typo in one must not be able to grant more than the file asked for — and says so by path and by the value it could not read, on the `warn_io` the catalog already uses. The definition still loads: a typo should make an agent careful, not unusable. The listing quotes what was declared next to what took effect, so `inspect (fallback; 'inspekt' is not a mode)` reads as smith's own choice rather than as configuration. `mode: Inspect` was never a typo and still is not — case is folded before the value is read. The same string arriving from a *model*, through the `agent` tool's `mode` parameter, is refused outright with the two valid names, exactly as an unknown `agent_type` already was: the schema declares an enum but not every provider enforces one, and a value the model invented must not become `work` (#106).

README.md

@@ -1014,9 +1014,9 @@ Below the trigger nothing happens at all and the transcript is left byte-identic
 1. **Drop stale thinking.** Bulky, worthless once its turn is over, and free in fidelity terms. The current turn is preserved byte for byte, because Anthropic validates the signature on a thinking block.
 2. **Supersede duplicate reads.** Read the same file five times and only the newest is kept in full; the earlier ones become a note saying why they are not there. Only genuinely read-only tools take part — `read_file`, `grep`, `glob`. `bash` is deliberately excluded: running `make test` twice is not a duplicate, it is a before and an after.
 3. **Truncate stale tool results.** Largest first, so reclaiming a given number of tokens mangles as few results as possible. A result reached once keeps a 512-byte head; one already carrying a truncation marker collapses to a single line.
-4. **Summarize the oldest turns** via a separate, tool-free provider call, replacing them with a single summary message. If that call fails, the prefix is dropped outright rather than failing the turn.
+4. **Summarize the oldest turns** via a separate, tool-free provider call, replacing them with a single summary message. If that call fails, the prefix is dropped outright rather than failing the turn. Two things it will not do: summarize a prefix that is only the summary the last compaction wrote, and keep a summary that came back longer than the turns it replaced. Both are calls that cost money and reclaim nothing.
 
-The **last three real turns are left alone** by stages 2 and 3, so compaction cannot truncate the file being edited out from under the model. Real turns: the agent injects user messages of its own when a stop hook asks it to keep going or a response hits the output limit, and counting those would let three of them inside a single turn consume the whole window. The window is given up only as a last resort — one oversized `cat` inside the only turn there is gets cut anyway, because protecting the work in hand is worth less than a request the provider will accept at all.
+The **last three real turns are left alone** by stages 2 and 3, so compaction cannot truncate the file being edited out from under the model. Real turns: the agent injects user messages of its own when a stop hook asks it to keep going or a response hits the output limit, and counting those would let three of them inside a single turn consume the whole window. The window is given up when no turn boundary leaves a tail that fits: stages 2 and 3 then run again over everything, the current turn included, because protecting the work in hand is worth less than a request the provider will accept at all. That is not an exotic case. A turn is a unit of *conversation*, not of size, and a long agentic run — one prompt, then a hundred tool calls — is a single turn, so a coding session reaches it routinely. Largest first still applies, and the pass stops the moment the target is met, so small recent results survive it.
 
 Every stage preserves the pairing between `tool_use` and `tool_result` blocks — providers reject a request where one half is missing, so compaction only ever shortens content or removes whole turns. Checkpoints name the message they were taken at rather than a position in the transcript, so summarizing a prefix cannot move them: a rewind still lands on the turn you picked, and a checkpoint whose message was summarized away restores its files and says the transcript was left alone.
 

spec/smith/context_spec.cr

@@ -742,3 +742,96 @@ describe "the cost of compacting a long history" do
     assert_tool_pairing(result.messages)
   end
 end
+
+# One prompt, then a hundred tool calls. The recency window counts turns, so a
+# run of this shape is a *single* turn and the window covers all of it — which
+# is how compaction came to summarize its own summary every turn while the
+# thing that was actually growing sat behind the cut, untouched.
+private def one_long_turn(steps : Int32, result_bytes : Int32 = 8_000)
+  messages = [] of Smith::LLM::Message
+
+  messages << user_msg("the first question")
+  messages << assistant_tool_call("call-first")
+  messages << tool_result("call-first", "y" * 200)
+
+  messages << user_msg("yes, go ahead")
+  steps.times do |i|
+    messages << assistant_tool_call("call-#{i}")
+    messages << tool_result("call-#{i}", "x" * result_bytes)
+  end
+
+  messages
+end
+
+private def tool_result_bytes(messages : Array(Smith::LLM::Message))
+  messages.sum do |message|
+    message.content.sum { |block| block.type.tool_result? ? (block.text.try(&.bytesize) || 0) : 0 }
+  end
+end
+
+describe "a long agentic run inside one turn" do
+  it "shortens the turn that is the whole context instead of summarizing the prefix" do
+    messages = one_long_turn(30)
+    result = compact_with_summary(messages, budget(70_000))
+
+    # It used to report `["summarize"]` here, having replaced three messages
+    # with a summary and left 240 KB of tool results in the turn behind the cut.
+    result.stages.should eq(["truncate"])
+    result.reached_target?.should be_true
+    tool_result_bytes(result.messages).should be < tool_result_bytes(messages)
+    assert_tool_pairing(result.messages)
+  end
+
+  it "shortens the turn rather than summarizing again on the next compaction" do
+    # The second compaction of the same history is where the old behaviour
+    # settled: the prefix is the summary the first one wrote, and nothing else.
+    # Shortening the turn is what makes the call unnecessary in the first place.
+    messages = [Smith::LLM::Message.user("#{Smith::Context::SUMMARY_PREFIX}what came before")] +
+               one_long_turn(30)[3..]
+
+    calls = 0
+    result = Smith::Context.compact(messages, budget(70_000)) do |_|
+      calls += 1
+      "what came before"
+    end
+
+    calls.should eq(0)
+    result.stages.should eq(["truncate"])
+    result.reached_target?.should be_true
+  end
+
+  it "refuses to summarize a prefix that is only the last summary" do
+    # Nothing here is truncatable, so the desperate pass cannot help and the
+    # cut lands back on the same boundary. What is left to summarize is the
+    # summary itself — a call that buys the difference between one summary and
+    # the next, every turn, forever.
+    messages = [
+      Smith::LLM::Message.user("#{Smith::Context::SUMMARY_PREFIX}what came before"),
+      user_msg("yes, go ahead"),
+      Smith::LLM::Message.assistant("x" * 300_000),
+    ]
+
+    calls = 0
+    result = Smith::Context.compact(messages, budget(70_000)) do |_|
+      calls += 1
+      "what came before"
+    end
+
+    calls.should eq(0)
+    result.strategy.none?.should be_true
+    result.compacted?.should be_false
+    result.messages.should eq(messages)
+  end
+
+  it "keeps the staged history when the summary comes back longer than the turns it replaces" do
+    # Nothing bounds what a provider answers with. Paying for the call, losing
+    # the detail and growing the request is the one outcome worth refusing.
+    messages = conversation(10, result_bytes: 60_000)
+    result = Smith::Context.compact(messages, budget(1_000)) { |_| "z" * 4_000_000 }
+
+    result.strategy.summarized?.should be_false
+    result.stages.should_not contain("summarize")
+    result.after_tokens.should be <= result.before_tokens
+    assert_tool_pairing(result.messages)
+  end
+end