Skip to content

fix: stop appended messages from taking prompt cache breakpoints - #43510

Open
NamedIdentity wants to merge 7 commits into
anomalyco:devfrom
NamedIdentity:cache-breakpoint-selection
Open

fix: stop appended messages from taking prompt cache breakpoints#43510
NamedIdentity wants to merge 7 commits into
anomalyco:devfrom
NamedIdentity:cache-breakpoint-selection

Conversation

@NamedIdentity

Copy link
Copy Markdown

Issue for this PR

Closes #43507

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

Prompt caching stops working, without any error, when something appends a message to the end of a request. This change keeps appended messages out of the array that cache breakpoints are placed on, so the breakpoints land on durable conversation instead.

The mechanism is described in the issue. In short: applyCaching (packages/opencode/src/provider/transform.ts:361) selects its two conversation breakpoints as the last two non-system messages. Anthropic caching matches on a prefix, so a breakpoint placed on a status notice that is rebuilt on every request can never produce a cache hit — new conversation is inserted ahead of that notice each turn, and the cached prefix stops being a prefix.

The fix is to keep those messages out of the array breakpoints are selected from. They are passed separately, in a new messageSuffix field alongside messages, and concatenated back after selection. The messages delivered to the model are unchanged in content and order; only which message carries a cache marker changes.

Scope limit. Appended messages rejoin after the AI SDK has lowered messages into provider format, and only non-empty text parts cross that boundary structurally identical. This change therefore separates text-shaped appends only. A plugin appending a file, tool content, reasoning, or an empty message leaves that message on the ordinary conversion path, where it can still consume a breakpoint. This was deliberate: hand-lowering those shapes would mean reproducing the SDK's own conversion, and an incorrect copy of it would be worse than leaving them unchanged. Every append observed in practice has been status text, but the gap is real and is stated here rather than left to be discovered.

Inside ProviderTransform.message(): the conversation and the appended messages are normalized separately, applyCaching runs over the conversation only, the two are concatenated, and the existing provider-key remapping — the pass that rewrites providerOptions keys into the form each SDK expects — then runs over the whole array. applyCaching itself is unchanged. MAX_STEPS_PROMPT moves into the appended array as well, which fixes the case that involves no plugin at all.

One complication. Normalizing the two arrays separately breaks a rule in normalizeMessages — the only rule there that examines both sides of a message boundary. For Mistral, a tool message followed by a user message has an assistant "Done." inserted between them. When the split falls exactly at that boundary, neither array can see the pair. That rule was extracted into a helper and is re-applied at the join before concatenation. Any future rule that reads across a message boundary will need the same treatment.

On the size of this change. dev does not currently have the pieces this requires, so the diff is larger than the description above suggests. The underlying problem is that nothing in the tree records which messages a plugin appended: experimental.chat.messages.transform hands the plugin a mutable array and takes it back, with no record of what changed. This change therefore snapshots message IDs across the hook and classifies what returns as appended, at session/prompt.ts:1255.

That is the only site that needs it. Compaction triggers the same hook, but the following line serializes the transformed history into a single string and embeds it in the compaction prompt, so anything a plugin appends there becomes text inside one message rather than a trailing message that could hold a breakpoint. No change is required on that path.

The classification has to be applied before conversion, because conversion does not preserve message count — one stored message can become several, and some are dropped entirely — so the converted array cannot be split by counting from the end. Conversion returns the two halves instead, and the second half needs a route down to ProviderTransform.message, which currently accepts a single array.

Two alternative shapes were considered and not taken: marking each message at the point a plugin adds it, which requires a plugin-facing API change; and changing the hook to return an appendix rather than mutating in place, which changes the contract for existing plugins. The approach here was chosen because it requires neither.

Not included: packages/llm/src/cache-policy.ts has the same defect on the native path. Fixing it requires a new field on the Message schema — public surface, and better raised separately than added inside a bug fix.

How did you verify your code works?

Production measurement. Before the fix, across four consecutive requests in one session, cache reads stayed pinned at exactly 300,150 while cache writes climbed 9,796 → 15,382 → 18,805 → 25,074. All 69,057 written tokens were paid for and never read back. After the fix, reads tracked writes exactly: 471,966 + 8,521 = 480,487, then + 2,502 = 482,989, then + 3,788 = 486,777.

Caveat on those "after" figures. They were measured on an earlier build of this fix, which placed breakpoints in the same position by a more complicated route. What is submitted here is a simplification of it.

The simplified version has since run live across three sessions. Over the span attributable to it by process evidence: 28 consecutive request pairs, 23 of which show the cache read advancing by exactly the previous request's cache write, and no instance of the failure this change fixes — a read holding flat at a non-zero value while writes accumulate. A message was appended on 28 of those 31 requests, so the triggering condition was present throughout. The remaining pairs are ordinary cold starts, where the read drops toward zero rather than holding.

A matched before-and-after has not been run on the simplified version, so the saving quoted above remains the predecessor's figure. Breakpoint placement is verified; the magnitude of the saving is carried over from the earlier build.

Tests. These drive the real call site — streamText with a fake fetch — and assert against the captured request body rather than calling applyCaching directly. A direct call passes whether or not the route that actually builds the request is correct, so it proves nothing about this defect.

Coverage:

  • Breakpoints land on the conversation and none on the appended messages. As a negative control, returning the appended messages to the main array causes one to take a breakpoint again, confirming the assertion can fail.
  • An empty appended array produces a byte-identical request body to passing none at all, on Anthropic, DeepSeek and Mistral. For anyone not appending messages, the request is unchanged.
  • Normalizing the two arrays separately produces a byte-identical result to normalizing them together, on the same three providers. This is the case that failed until the Mistral boundary was handled.
  • Breakpoints stay within Anthropic's four-cache_control limit; an empty message that normalization deletes cannot consume one; and running selection twice over the same input does not accumulate markers.

That file passes. On the rest of the packages/opencode suite there are no failures unique to this branch. The base run had 17 failing tests and the branch had 16, and those 16 are the same tests, all already failing before any change — mostly Windows symlink cases and timeouts. The additional failure on the base run was a snapshot test that timed out under a loaded full-suite run; it passed when run on its own against both revisions. Typecheck is clean on both affected packages.

Screenshots / recordings

N/A — not a UI change.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

Sean Smith added 7 commits August 19, 2026 02:08
Normalize appended messages separately so cache breakpoints only see the conversation, then join both arrays before provider option remapping. Restore Mistral's tool-to-user bridge when the split falls on that boundary.
Classify safely appended message IDs before model conversion and preserve the split when one stored message expands into multiple provider messages. Keep the existing conversion API unchanged for callers that do not request a split.
Snapshot message IDs across the plugin hook, split safely appended messages before conversion, and transport them separately through request preparation. Append the suffix in both AI SDK and native consumers so no request-only messages are dropped.
Exercise the real AI SDK middleware boundary, fail-closed append classification, conversion expansion, provider-specific normalization, and cache selection invariants. Include a single-generation outer loop and a flattening negative control.
Assert that the prompt loop separates plugin and max-step messages, and that request preparation preserves their order through both AI SDK and native consumers.
Represent the max-step prompt as a text part because appended messages bypass the AI SDK's ordinary message conversion before the provider transform.
Request-only suffixes join after AI SDK prompt lowering. Keep appends with non-text parts in the normal conversion path so files and tool content retain upstream lowering semantics.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Prompt caching breaks when a plugin appends a message

1 participant