fix: the cache breakpoint advances into the tool round #31
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/caching-breakpoint-advances"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Advances the caching breakpoint into the tool round.
ApplyCachingMarkersstamped the system message and the last UserMessage; in a tool round the history's tail isassistant(tool_calls), tool, so the breakpoint froze at the kickoff and every appended turn, tool result and image re-billed at full input price on every later round — exactly the agentic shape the flag exists for.The breakpoint now goes on the last message of the history whatever its role (Anthropic's guidance for agentic tool use). Only the tail is stamped: no stale breakpoint is left behind the advancing one.
Found by Jibril's review of TeamAI/Orihon#201 — her two-round wire probe (
PROBE_LAST type=ToolMessage CacheControl=ABSENT) is now the vendor pin's body. RED first, mutation-proven (removing the ToolMessage arm fails exactlyEnableCaching_advances_the_breakpoint_into_the_tool_round). Vendor suite 1510 ✓.Summary
Summary
Coverage
OpenRouter.Net - 97.8%
OpenRouter.Net.Agents - 86%
OpenRouter.Net.Imaging - 81.8%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! The breakpoint chase~ ♡ You found it frozen at the kickoff — every tool result and image re-billed at full input price, round after round — and you moved it to the advancing tail, exactly the agentic shape Anthropic documents. The projection-only design is delicious:
_messagesis never mutated, so no stale breakpoint ever accumulates behind the advancing one, and theCacheControl is nullguards keep re-application idempotent. I got genuinely giddy reading the comment block abovetail— that is a textbook explanation of why, not just what~Verdict: ⛔ I can't let this pass~ ♡
One arm of your new switch is wearing no armor at all. Don't worry — I checked mechanically, so let me show you~
⛔ These need fixing before I'm satisfied~
AssistantMessagetail arm is untested, and I proved it with a mutation probe: I deleted the entire arm, rebuilt, and ran the full Agents suite — 144/144 tests still green. The mutant survived. fufu~ you added a code path but forgot to test it? I can't let that slide~ ♡And this arm is not dead code — it's reachable in production: when the iteration cap is hit (
MaxIterationsReached) the history ends[.., assistant(tool_calls), tool]… wait, no — after the final assistant turn with no tool calls, and in the cap-exit case the tail is the assistant turn itself.ContinueAsyncon that history (your ownToolCallAnswerTestsresume scenario, but withEnableCachingon — which no test sets) sends an assistant-tailed history straight into this arm, and it changes the wire request (acache_controlis emitted). A branch that alters real requests and whose deletion no test notices? That is exactly the kind of unguarded edge I simply cannot allow~Fix: one focused test —
LoadHistory([system, user, assistant])(orAddMessagean assistant tail) withEnableCaching = true, one stubbed round, assert the sent tail assistant carriesCacheControland the earlier user message does not. RED-first, like the sibling~💡 Little ideas (non-blocking)~
messages.Count > 1guard means an explicitly loaded single-message[user]history gets no advancing breakpoint at all (only the system message, if present). Defensible — stamping the entire conversation as one prefix is its own mistake — but it's an invisible sharp edge forLoadHistorycallers. A half-sentence in the XML docs onEnableCachingwould spare a future someone an afternoon~✅ What I liked~
EnableCaching_advances_the_breakpoint_into_the_tool_roundis the dominant agentic shape, asserts both the stamped tail AND the unstamped earlier user message (no dead-weight second write), and my probe confirmed it bites: deleting theToolMessagearm fails exactly this test (Assert.NotNullat the tail, 18 tests, 1 failed). The pin holds~ ♪ReferenceEquals(messages[i], tail)instead of an index comparison — subtle and correct, immune to the tail being an equal-but-different record.Fix the missing armor on the assistant arm and I'll swoon properly~ ♡
Automated review by Jibril · 2026-10-12
CI: green for head SHA
3f37d63e· Mutation probes: 2 valid run, 1 survived (AssistantMessage arm), 1 killed (ToolMessage arm); 2 additional runs discarded due to a stale-DLL measurement mistake on my side, not yours · Local suite: skipped (CI green)Both findings fixed at
7494ae3.The assistant tail arm is now pinned. The new test loads
[system, user, assistant]—LoadHistoryreplaces the whole history, the constructor's seeded system message included, so the system message rides inside the loaded list — andContinueAsyncsends it unchanged. Asserts: system carries the marker, the tail assistant carries it, the earlier user message does not. Mutation-proven: deleting theAssistantMessagearm fails exactly this pin (and nothing else).Two things I hit while writing it, both worth having on record:
LoadHistoryclearing the seeded system prompt is a genuine footgun for callers — the doc note onEnableCachingnow says the marker follows the history's last message whatever its role, and that a loaded history owns its system message.Vendor suite 1511 ✓ (was 1510, +1 pin). fufu~
🔮 fufu~ Jibril reviewed your code!
Oh? You came back with armor for the bare arm~ ♡ And not just any armor — a pin that documents the semantics it guards (
LoadHistoryowns the whole history, seeded system message included). That's the difference between a test that passes and a test that teaches. I'm giddy~Verdict: ✅ Looks good to me~
The blocking finding is fixed and I verified it the only way I trust — mechanically, on
7494ae3:🔬 Re-probe of the AssistantMessage arm
AgentLoopTests19/19 green, CI green for7494ae38.case AssistantMessage … tailarm fromApplyCachingMarkers, rebuilt the test project, re-ran: exactlyEnableCaching_stamps_an_assistant_tail_toofails (Assert.NotNull(tail.CacheControl), AgentLoopTests.cs:403) — 1 failed, 18 passed, no collateral. The mutant that survived my last review is now killed, surgically~3f37d63e, code unchanged since.)✅ What I liked~
LoadHistory-clears-the-seeded-system footgun in the test comment (and theEnableCachingXML doc: marker follows the last message whatever its role, one-message history gets only the system marker) — that's the earlier-user-unstamped why made permanent. Future someone skips an afternoon of confusion ♪Agent.csuntouched by the fix commit — the smallest diff that closes the finding. Exactly how a re-review response should look.Swoon accepted~ 💖 Merge it.
Automated review by Jibril · 2026-10-12
CI: green for head SHA
7494ae38· Mutation probes: 1 run, 0 survived (AssistantMessage arm killed; ToolMessage arm skipped — unchanged, killed at3f37d63e) · Local suite: skipped (CI green; targeted class run 19/19 as probe baseline)