From a22be62eee2e4545c92f272d4779073f38fdcc55 Mon Sep 17 00:00:00 2001 From: Osher-Elhadad Date: Mon, 10 Aug 2026 13:23:57 +0000 Subject: [PATCH] fix(cache): delete both breakpoint spellings from the split's volatile half splitVolatileTail COPIES the system block it splits, then deletes cache_control from the churning half so the breakpoint covers only the stable text. It did not delete cachePoint -- the Bedrock/Vertex spelling of the same thing, which wireBreakpoints counts alongside cache_control. So on those providers a splittable system block carrying an inline cachePoint had its breakpoint DUPLICATED: one on each half, 1 -> 2. That silently spends a slot from the provider's cap of four, and from a request already at the cap it puts five on the wire and takes a 400. Found by an integration review of the five cache/filter/observe merges: neither per-PR review could see it, because the split and the wire-breakpoint counter landed in the same PR but in different files, and the Anthropic path -- the only one the benchmarks exercise -- has no cachePoint to duplicate. The regression test asserts the wire count is unchanged across the split and that the volatile half keeps no breakpoint in either spelling. It fails without the fix with the exact symptom (1 -> 2). Signed-off-by: Osher-Elhadad --- apply/prefixsplit.go | 8 +++++++- apply/prefixsplit_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/apply/prefixsplit.go b/apply/prefixsplit.go index 4b1142b..c650437 100644 --- a/apply/prefixsplit.go +++ b/apply/prefixsplit.go @@ -114,8 +114,14 @@ func splitVolatileTail(body []byte, provider bschemas.ModelProvider) ([]byte, bo stable["text"] = txt[:at] volatile["text"] = txt[at:] // The breakpoint belongs on the stable half only; leaving one on the volatile - // half would put the churn back inside a hashed prefix. + // half would put the churn back inside a hashed prefix. BOTH spellings must go: + // Bedrock/Vertex write `cachePoint` where Anthropic writes `cache_control`, and + // wireBreakpoints counts both (metawrite.go). Deleting only one DUPLICATES the + // other — the split copies the block, so a system block carrying an inline + // cachePoint turns 1 breakpoint into 2 and can push the wire past the provider's + // cap of four (measured: 4 inbound -> 5 on the wire -> 400). delete(volatile, "cache_control") + delete(volatile, "cachePoint") sb, err1 := json.Marshal(stable) vb, err2 := json.Marshal(volatile) if err1 != nil || err2 != nil { diff --git a/apply/prefixsplit_test.go b/apply/prefixsplit_test.go index 80072c6..d658d55 100644 --- a/apply/prefixsplit_test.go +++ b/apply/prefixsplit_test.go @@ -333,3 +333,31 @@ func TestUnknownFieldsNotDropped(t *testing.T) { gjson.GetBytes(got, "system").Raw[:200]) } } + +// A Bedrock/Vertex system block can carry `cachePoint` where Anthropic writes +// `cache_control`. The split COPIES the block, so deleting only one spelling leaves the +// other on BOTH halves and turns one breakpoint into two. wireBreakpoints counts both +// spellings, so that silently eats a slot from the provider's cap of four — and from a +// request already at the cap it pushes the wire to five and takes a 400. +func TestSplitDoesNotDuplicateBedrockCachePoint(t *testing.T) { + full, _, _ := blockWithGitTail(6000) + block := map[string]any{"type": "text", "text": full, "cachePoint": map[string]any{"type": "default"}} + in := sysBody(block) + + before := wireBreakpoints(in) + got, split := splitVolatileTail(in, bschemas.Bedrock) + if !split { + t.Fatal("expected the git-tail block to split") + } + if after := wireBreakpoints(got); after != before { + t.Fatalf("split changed the wire breakpoint count %d -> %d; a copied cachePoint "+ + "burns a slot from the provider's cap of four", before, after) + } + // and the churning half must not carry a breakpoint at all + for _, b := range gjson.GetBytes(got, "system").Array() { + if strings.Contains(b.Get("text").String(), "Recent commits:") && + (b.Get("cachePoint").Exists() || b.Get("cache_control").Exists()) { + t.Fatal("the volatile half kept a breakpoint — the churn is back inside a hashed prefix") + } + } +}