From ea33178ea9ba8c7ed0014eb5375e8ea63ed27b0d Mon Sep 17 00:00:00 2001 From: Osher-Elhadad Date: Mon, 10 Aug 2026 03:34:21 +0000 Subject: [PATCH 1/3] fix(cache): write cacheinject's breakpoints to the wire as metadata MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cacheinject's breakpoints never reached the provider on Claude Code traffic. Measured over 40 captured requests: 46 breakpoints applied at the component level, 0 in the output body. The cause was structural. The component can only mark messages carrying content blocks; on this traffic those are exclusively assistant turns carrying tool_use. bifrost drops tool_use.id/name/input on unmarshal, so apply's losslessness guard discarded every change to such a message rather than splice a corrupted re-marshal — correct in itself, but it meant a component whose only possible targets were precisely the messages that could never be written back. Do not relax the guard; it prevents real corruption. Instead take the narrow exception the data model allows: cache_control is metadata, not content, so it needs no message model to express. When a component's only change to a message is an added cache_control key, write that key at its exact path on the ORIGINAL raw bytes via sjson. A write that reads no other field cannot drop one. metadataOnlyWrites enforces "only that" by diffing pre against post with the added keys removed; anything wider is still discarded. applyMetaWrites refuses if the raw block layout disagrees with the normalized view, and never overwrites a breakpoint the caller set. Fix the second, independent defect in the same change, because shipping the first alone produces a live 400. The provider caps cache_control at 4 across system + tools + messages together, and a component sees none of the first two — nor cache_control on blocks bifrost drops. On real traffic that hides all three of the agent's own breakpoints (2 in system, 1 on a tool_result block), so the component computed 3 free slots when 1 was free and emitted 6 on the wire. apply now counts them structurally from the raw body and passes the total as Ctx.ExistingBreakpoints; it also counts its own output and logs an error on a breach rather than waiting for the provider to reject the request. Make the failure class loud. A mutated-then-discarded component was indistinguishable from a working Reformat, which is why this survived two full benchmark studies. Pipeline.RecordDiscards attributes each thrown-away change back to the component that made it, surfacing as per-component discarded_changes and top_discarded in /stats. Both fields are additive; no existing /stats key is renamed or removed. Tests, all failing before this change: a cacheinject mark on an assistant tool_use message reaches the output body with id/name/input intact and nothing else altered; the wire total stays within 4 on the real (system=2, tools=0, messages=1) shape where the synthetic 60-message case previously produced 6; a discarded change increments the counter without inflating Runs. Replaying real captures, breakpoints now reach the wire on 86 of 92 requests (previously 1) with every tool_use provider field intact. Closes #32 Assisted-By: Claude Opus 5 Signed-off-by: Osher-Elhadad --- apply/apply.go | 32 +++- apply/apply_test.go | 190 ++++++++++++++++++++++++ apply/metawrite.go | 140 +++++++++++++++++ apply/metawrite_test.go | 57 +++++++ apply/replay_capture_test.go | 118 +++++++++++++++ components/component.go | 22 +++ components/pipeline.go | 44 ++++++ components/reformat/cacheinject.go | 18 ++- components/reformat/cacheinject_test.go | 48 ++++++ docs/components/cacheinject.md | 101 ++++++++++++- docs/design.md | 31 +++- metrics/metrics.go | 33 +++- 12 files changed, 814 insertions(+), 20 deletions(-) create mode 100644 apply/metawrite.go create mode 100644 apply/metawrite_test.go create mode 100644 apply/replay_capture_test.go diff --git a/apply/apply.go b/apply/apply.go index bc23d27..38320d9 100644 --- a/apply/apply.go +++ b/apply/apply.go @@ -170,6 +170,10 @@ func BodyFull(ctx context.Context, pipe *components.Pipeline, st store.Store, pr CtxWindow: window, CacheAware: cacheAware, MaxCachedIdx: maxCachedIdx, + // Every breakpoint already on the wire — including the ones no component can + // see (`system`, `tools`, and cache_control on blocks bifrost drops). The + // provider's cap of four counts them all (issue #32, defect 2). + ExistingBreakpoints: wireBreakpoints(body), } // Canonical form of each normalized message BEFORE the pipeline, so a @@ -179,7 +183,7 @@ func BodyFull(ctx context.Context, pipe *components.Pipeline, st store.Store, pr normPre[i], _ = json.Marshal(norm[i]) } - pipe.Run(chat, c) + rr := pipe.Run(chat, c) // A component changed the message count (summarize restructures the transcript // to [msg0, , last-K]). Rebuild the messages array preserving each @@ -198,6 +202,9 @@ func BodyFull(ctx context.Context, pipe *components.Pipeline, st store.Store, pr // if no component changes a message. changed := systemSplit var changes []change + // Per-message count of changes this writeback threw away, attributed back to the + // components that made them once the loop is done. + discarded := map[int]int{} for i := range chat.Input { s := slots[i] switch s.kind { @@ -222,8 +229,20 @@ func BodyFull(ctx context.Context, pipe *components.Pipeline, st store.Store, pr } if !s.lossless { // bifrost can't round-trip this message; splicing our re-marshal would - // drop provider fields it doesn't model. Discard the change, keep the - // original bytes. ponytail: correctness over the marginal saving here. + // drop provider fields it doesn't model. But if the ONLY change is added + // `cache_control` — metadata, not content — write those keys at their exact + // paths on the original raw bytes: nothing else is read or rewritten, so no + // provider field can be dropped. See metawrite.go (issue #32). + if w, ok := metadataOnlyWrites(s.pre, post); ok { + if nb, ok := applyMetaWrites(out, s.path, len(chat.Input[i].Content.ContentBlocks), w); ok { + out = nb + changed = true + continue + } + } + // Anything else: discard the change, keep the original bytes. + // ponytail: correctness over the marginal saving here. + discarded[i]++ continue } if out, err = sjson.SetRawBytes(out, s.path, post); err != nil { @@ -235,9 +254,16 @@ func BodyFull(ctx context.Context, pipe *components.Pipeline, st store.Store, pr changes = append(changes, mkChange(s.path, schema.MessageText(pm), schema.MessageText(chat.Input[i]))) } } + pipe.RecordDiscards(rr, discarded) if changed && dumpPath != "" { dumpChanges(c.Session, changes) } + if n := wireBreakpoints(out); n > maxWireBreakpoints { + // Should be unreachable: the component budgets against OuterBreakpoints. Loud + // rather than a 400 from the provider, and it names the request shape. + slog.Error("context-guru: cache breakpoint count exceeds the provider cap", + "breakpoints", n, "cap", maxWireBreakpoints, "session", c.Session) + } return out, changed } diff --git a/apply/apply_test.go b/apply/apply_test.go index d63a903..1bed916 100644 --- a/apply/apply_test.go +++ b/apply/apply_test.go @@ -3,6 +3,7 @@ package apply_test import ( "context" "encoding/json" + "reflect" "strings" "testing" @@ -11,8 +12,10 @@ import ( "github.com/rossoctl/context-guru/components" _ "github.com/rossoctl/context-guru/components/all" "github.com/rossoctl/context-guru/config" + "github.com/rossoctl/context-guru/metrics" "github.com/rossoctl/context-guru/store" "github.com/tidwall/gjson" + "github.com/tidwall/sjson" ) func pipe(t *testing.T, yaml string) *config.Config { @@ -266,3 +269,190 @@ func TestNoMessagesForwardsUnchanged(t *testing.T) { t.Fatalf("no messages array => forward unchanged; got changed=%v %s", changed, out) } } + +// TestCacheinjectReachesTheWire is the #32 regression: cacheinject's only possible +// targets on Claude Code traffic are assistant messages carrying `tool_use`, which +// bifrost cannot round-trip — so every mark used to be discarded by the writeback +// loop (46 applied, 0 forwarded, measured over 40 real requests). cache_control is +// metadata, so it is written onto the ORIGINAL raw bytes and the unmodellable +// provider fields must survive verbatim. +func TestCacheinjectReachesTheWire(t *testing.T) { + cfg := pipe(t, "pipeline: [cacheinject]\n") + p, _ := cfg.Build(nil) + st := store.NewMemory(store.Options{}) + + body := []byte(`{"model":"claude-x","messages":[ + {"role":"user","content":"run the tool"}, + {"role":"assistant","content":[{"type":"text","text":"on it"},{"type":"tool_use","id":"toolu_abc","name":"Bash","input":{"command":"ls -la"}}]} + ]}`) + + out, changed := apply.Body(context.Background(), p, st, bschemas.Anthropic, body, "", false) + if !changed { + t.Fatal("expected cacheinject's breakpoint to change the body") + } + cc := gjson.GetBytes(out, "messages.1.content.1.cache_control") + if !cc.Exists() || cc.Get("type").String() != "ephemeral" { + t.Fatalf("breakpoint never reached the wire: %s", out) + } + // Provider fields bifrost drops on unmarshal must be intact. + blk := gjson.GetBytes(out, "messages.1.content.1") + if blk.Get("id").String() != "toolu_abc" || blk.Get("name").String() != "Bash" || + blk.Get("input.command").String() != "ls -la" { + t.Fatalf("tool_use provider fields corrupted: %s", blk.Raw) + } + if gjson.GetBytes(out, "messages.1.content.0.text").String() != "on it" { + t.Fatalf("sibling text block corrupted: %s", out) + } + // Everything except the added cache_control is byte-identical. + stripped, err := sjson.DeleteBytes(out, "messages.1.content.1.cache_control") + if err != nil { + t.Fatal(err) + } + if !jsonEq(t, stripped, body) { + t.Fatalf("body changed beyond the metadata write:\n old=%s\n new=%s", body, stripped) + } +} + +func jsonEq(t *testing.T, a, b []byte) bool { + t.Helper() + var av, bv any + if err := json.Unmarshal(a, &av); err != nil { + t.Fatal(err) + } + if err := json.Unmarshal(b, &bv); err != nil { + t.Fatal(err) + } + return reflect.DeepEqual(av, bv) +} + +// TestWireBreakpointCapRealTrafficShape uses the exact shape 1,771 of 1,794 +// captured Claude Code requests carry — system=2, tools=0, messages=1 — on a long +// conversation, and asserts the total the PROVIDER sees never exceeds 4. Before #32 +// the component counted only the messages array, saw 1 existing breakpoint, and +// budgeted 3 more: 6 on the wire, which the provider rejects with a 400. +func TestWireBreakpointCapRealTrafficShape(t *testing.T) { + cfg := pipe(t, "pipeline: [cacheinject]\n") + p, _ := cfg.Build(nil) + + msgs := make([]any, 0, 60) + for i := 0; i < 60; i++ { + role, blk := "user", map[string]any{"type": "text", "text": strings.Repeat("turn ", i%7+1)} + if i%2 == 1 { + role = "assistant" + } + m := map[string]any{"role": role, "content": []any{blk}} + if i == 59 { // the caller's own trailing breakpoint + blk["cache_control"] = map[string]any{"type": "ephemeral"} + } + msgs = append(msgs, m) + } + body, _ := json.Marshal(map[string]any{ + "model": "claude-x", + "system": []any{ + map[string]any{"type": "text", "text": "tools preamble", "cache_control": map[string]any{"type": "ephemeral"}}, + map[string]any{"type": "text", "text": "main system prompt", "cache_control": map[string]any{"type": "ephemeral"}}, + }, + "messages": msgs, + }) + + out, _ := apply.Body(context.Background(), p, store.NewMemory(store.Options{}), bschemas.Anthropic, body, "", false) + if n := countWireBreakpoints(t, out); n > 4 { + t.Fatalf("%d breakpoints on the wire — the provider caps at 4 and 400s above it", n) + } +} + +// countWireBreakpoints counts cache_control across system, tools and messages the +// way the provider does — deliberately re-derived in the test rather than reusing +// the implementation's counter, so a bug in that counter cannot hide the cap breach. +func countWireBreakpoints(t *testing.T, body []byte) int { + t.Helper() + var req struct { + System []map[string]json.RawMessage `json:"system"` + Tools []map[string]json.RawMessage `json:"tools"` + Msgs []struct { + CacheControl json.RawMessage `json:"cache_control"` + Content []map[string]json.RawMessage `json:"content"` + } `json:"messages"` + } + if err := json.Unmarshal(body, &req); err != nil { + t.Fatalf("output is not valid JSON: %v", err) + } + n := 0 + count := func(blocks []map[string]json.RawMessage) { + for _, b := range blocks { + if _, ok := b["cache_control"]; ok { + n++ + } + } + } + count(req.System) + count(req.Tools) + for _, m := range req.Msgs { + if len(m.CacheControl) > 0 { + n++ + } + count(m.Content) + } + return n +} + +// contentRewriter is a test-only component that rewrites an assistant message's +// text — a CONTENT change on a message bifrost cannot round-trip, so the writeback +// layer must discard it. No shipped component targets assistant turns, so the +// discard path needs one to exercise it. +type contentRewriter struct{} + +func (contentRewriter) Name() string { return "testrewrite" } +func (contentRewriter) Enabled(*components.Ctx) bool { return true } +func (contentRewriter) Reformat(req *bschemas.BifrostChatRequest, _ *components.Report, _ *components.Ctx) error { + for i := range req.Input { + if req.Input[i].Role != bschemas.ChatMessageRoleAssistant || req.Input[i].Content == nil { + continue + } + for b := range req.Input[i].Content.ContentBlocks { + if t := req.Input[i].Content.ContentBlocks[b].Text; t != nil { + short := "shortened" + req.Input[i].Content.ContentBlocks[b].Text = &short + } + } + } + return nil +} + +func init() { + components.Register("testrewrite", func([]byte) (components.Component, error) { + return contentRewriter{}, nil + }) +} + +// TestDiscardedChangeIsCounted: a change the writeback layer throws away must be +// attributed to the component that made it, not silently vanish. A component that +// mutates and is then discarded used to look exactly like one that works — which is +// how #32 survived two benchmark studies. +func TestDiscardedChangeIsCounted(t *testing.T) { + cfg := pipe(t, "pipeline: [testrewrite]\n") + agg := metrics.NewAggregator() + p, _ := cfg.Build(agg) + + body := []byte(`{"model":"claude-x","messages":[ + {"role":"user","content":"go"}, + {"role":"assistant","content":[{"type":"text","text":"a long narration that the component will rewrite"},{"type":"tool_use","id":"toolu_z","name":"Bash","input":{"command":"ls"}}]} + ]}`) + + out, _ := apply.Body(context.Background(), p, store.NewMemory(store.Options{}), bschemas.Anthropic, body, "", false) + if gjson.GetBytes(out, "messages.1").Raw != gjson.GetBytes(body, "messages.1").Raw { + t.Fatalf("the unmodellable message must be kept verbatim:\n old=%s\n new=%s", + gjson.GetBytes(body, "messages.1").Raw, gjson.GetBytes(out, "messages.1").Raw) + } + snap := agg.Snapshot() + if got := snap.Components["testrewrite"].Discarded; got == 0 { + t.Fatalf("discarded change not counted: %+v", snap.Components["testrewrite"]) + } + if len(snap.TopDiscarded) == 0 || snap.TopDiscarded[0] != "testrewrite" { + t.Fatalf("top_discarded should name the component, got %v", snap.TopDiscarded) + } + // A Discarded report must not inflate Runs (it is attribution, not a second run). + if r := snap.Components["testrewrite"].Runs; r != 1 { + t.Fatalf("Runs should stay 1, got %d", r) + } +} diff --git a/apply/metawrite.go b/apply/metawrite.go new file mode 100644 index 0000000..cffdcce --- /dev/null +++ b/apply/metawrite.go @@ -0,0 +1,140 @@ +package apply + +import ( + "strconv" + + "github.com/tidwall/gjson" + "github.com/tidwall/sjson" +) + +// Metadata writes: the narrow exception to the losslessness guard. +// +// bifrost cannot round-trip an Anthropic assistant turn that carries a `tool_use` +// block — it drops `id`, `name` and `input` on unmarshal — so the writeback loop +// discards any change to such a message rather than splice a lossy re-marshal. +// That is correct for CONTENT. But cacheinject's only possible targets on real +// agent traffic are exactly those messages, so every breakpoint it placed was +// thrown away before the request was sent (measured: 46 applied, 0 forwarded over +// 40 captured requests — issue #32). +// +// `cache_control` is metadata, not content: it changes nothing the model reads and +// needs no message model to express. So when a component's ONLY change to a +// message is adding `cache_control` to content blocks, we write those keys at their +// exact paths on the ORIGINAL raw bytes instead of splicing a re-marshal. Nothing +// else in the message is read or rewritten, so no provider field can be dropped +// and the guard's reason to exist does not arise. +// +// Anything beyond added `cache_control` keys — a text edit, a removed key, a +// changed block — is still discarded. + +// metaWrite is one metadata key to set, as a path relative to the message and the +// raw JSON to write there. +type metaWrite struct { + path string // "content..cache_control" + raw string +} + +// metadataOnlyWrites compares a message's pre- and post-pipeline canonical +// marshals and returns the metadata writes that reproduce the change, or ok=false +// if the change is anything other than added content-block `cache_control` keys. +func metadataOnlyWrites(pre, post []byte) (writes []metaWrite, ok bool) { + preBlocks := gjson.GetBytes(pre, "content") + postBlocks := gjson.GetBytes(post, "content") + if !preBlocks.IsArray() || !postBlocks.IsArray() { + return nil, false // string content carries no block to mark + } + pb, qb := preBlocks.Array(), postBlocks.Array() + if len(pb) != len(qb) { + return nil, false // blocks added/removed — not a metadata change + } + stripped := post + for b := range qb { + p := "content." + strconv.Itoa(b) + ".cache_control" + cc := qb[b].Get("cache_control") + if !cc.Exists() || pb[b].Get("cache_control").Exists() { + continue + } + writes = append(writes, metaWrite{path: p, raw: cc.Raw}) + var err error + if stripped, err = sjson.DeleteBytes(stripped, p); err != nil { + return nil, false + } + } + if len(writes) == 0 { + return nil, false + } + // The decisive check: with the added keys removed, the post-pipeline message must + // be identical to the pre-pipeline one. Otherwise something else changed too. + if !jsonEqual(stripped, pre) { + return nil, false + } + return writes, true +} + +// applyMetaWrites sets each metadata key on the raw body at msgPath. It refuses +// (ok=false, body untouched) if the raw message's block layout does not match what +// the writes assume, so a shape the normalizer and the raw body disagree about can +// never be written to the wrong block. +func applyMetaWrites(body []byte, msgPath string, nBlocks int, writes []metaWrite) ([]byte, bool) { + blocks := gjson.GetBytes(body, msgPath+".content") + if !blocks.IsArray() || len(blocks.Array()) != nBlocks { + return body, false + } + out := body + for _, w := range writes { + full := msgPath + "." + w.path + if gjson.GetBytes(out, full).Exists() { + return body, false // caller already set it — never overwrite + } + next, err := sjson.SetRawBytes(out, full, []byte(w.raw)) + if err != nil { + return body, false + } + out = next + } + return out, true +} + +// maxWireBreakpoints is the provider's hard cap on cache_control directives per +// request. Over it, the request 400s. +const maxWireBreakpoints = 4 + +// breakpointPaths are every location a real prompt-cache breakpoint can live. The +// cap applies across all of them together. Structural (gjson path queries) for the +// same reason hasCacheBreakpoint is: a tool output whose text merely contains the +// string "cache_control" must not count. +var breakpointPaths = []string{ + "system.#.cache_control", + "tools.#.cache_control", + "messages.#.cache_control", + "messages.#.content.#.cache_control", + "messages.#.content.#.cachePoint", +} + +// wireBreakpoints counts every breakpoint the provider will see in this request. +// +// A component cannot count these for itself: `system` and `tools` never reach it, and +// bifrost drops cache_control on block types it does not model — on real Claude Code +// traffic that hides all three of the agent's own breakpoints (2 in `system`, 1 on a +// `tool_result` block). Counting only what the component saw yielded a budget of 3 +// free slots when 1 was free: 6 on the wire, and a 400 (issue #32). +func wireBreakpoints(body []byte) int { + n := 0 + for _, p := range breakpointPaths { + gjson.GetBytes(body, p).ForEach(func(_, v gjson.Result) bool { + // nested arrays (content-of-messages) surface as arrays here; recurse one level + if v.IsArray() { + v.ForEach(func(_, vv gjson.Result) bool { + if vv.IsObject() { + n++ + } + return true + }) + } else if v.IsObject() { + n++ + } + return true + }) + } + return n +} diff --git a/apply/metawrite_test.go b/apply/metawrite_test.go new file mode 100644 index 0000000..9991093 --- /dev/null +++ b/apply/metawrite_test.go @@ -0,0 +1,57 @@ +package apply + +import "testing" + +// The metadata-write exception must be exactly that: added cache_control keys and +// nothing else. Anything wider would put us back to splicing a lossy re-marshal +// over a message bifrost cannot round-trip. +func TestMetadataOnlyWritesRejectsNonMetadataChanges(t *testing.T) { + pre := `{"role":"assistant","content":[{"type":"text","text":"hello"},{"type":"tool_use"}]}` + cases := []struct { + name, post string + want bool + }{ + {"added cache_control", `{"role":"assistant","content":[{"type":"text","text":"hello"},{"type":"tool_use","cache_control":{"type":"ephemeral"}}]}`, true}, + {"two added", `{"role":"assistant","content":[{"type":"text","text":"hello","cache_control":{"type":"ephemeral"}},{"type":"tool_use","cache_control":{"type":"ephemeral"}}]}`, true}, + {"no change", pre, false}, + {"text edited too", `{"role":"assistant","content":[{"type":"text","text":"HELLO"},{"type":"tool_use","cache_control":{"type":"ephemeral"}}]}`, false}, + {"block removed", `{"role":"assistant","content":[{"type":"tool_use","cache_control":{"type":"ephemeral"}}]}`, false}, + {"role changed", `{"role":"user","content":[{"type":"text","text":"hello"},{"type":"tool_use","cache_control":{"type":"ephemeral"}}]}`, false}, + {"string content", `{"role":"assistant","content":"hello"}`, false}, + } + for _, tc := range cases { + if _, ok := metadataOnlyWrites([]byte(pre), []byte(tc.post)); ok != tc.want { + t.Errorf("%s: ok=%v want %v", tc.name, ok, tc.want) + } + } +} + +// applyMetaWrites must refuse when the raw body's block layout disagrees with what +// the writes assume, so a key can never land on the wrong block. +func TestApplyMetaWritesRefusesOnShapeMismatch(t *testing.T) { + body := []byte(`{"messages":[{"role":"assistant","content":[{"type":"tool_use","id":"t"}]}]}`) + w := []metaWrite{{path: "content.1.cache_control", raw: `{"type":"ephemeral"}`}} + if _, ok := applyMetaWrites(body, "messages.0", 2, w); ok { + t.Fatal("expected a refusal: the raw message has 1 block, the writes assume 2") + } + // Never overwrite a breakpoint the caller already set. + set := []byte(`{"messages":[{"role":"assistant","content":[{"type":"tool_use","cache_control":{"type":"ephemeral","ttl":"1h"}}]}]}`) + w = []metaWrite{{path: "content.0.cache_control", raw: `{"type":"ephemeral"}`}} + if _, ok := applyMetaWrites(set, "messages.0", 1, w); ok { + t.Fatal("expected a refusal: the caller's own cache_control must not be overwritten") + } +} + +func TestBreakpointCounting(t *testing.T) { + body := []byte(`{ + "system":[{"type":"text","cache_control":{"type":"ephemeral"}},{"type":"text","cache_control":{"type":"ephemeral"}},{"type":"text"}], + "tools":[{"name":"x"},{"name":"y","cache_control":{"type":"ephemeral"}}], + "messages":[ + {"role":"user","content":"a tool output that merely mentions cache_control in its text"}, + {"role":"assistant","content":[{"type":"tool_use","cache_control":{"type":"ephemeral"}}]}, + {"role":"user","cache_control":{"type":"ephemeral"},"content":"q"} + ]}`) + if got := wireBreakpoints(body); got != 5 { + t.Errorf("wireBreakpoints = %d, want 5", got) + } +} diff --git a/apply/replay_capture_test.go b/apply/replay_capture_test.go new file mode 100644 index 0000000..6de30e5 --- /dev/null +++ b/apply/replay_capture_test.go @@ -0,0 +1,118 @@ +package apply_test + +import ( + "bufio" + "context" + "encoding/json" + "os" + "testing" + + bschemas "github.com/maximhq/bifrost/core/schemas" + "github.com/rossoctl/context-guru/apply" + "github.com/rossoctl/context-guru/store" + "github.com/tidwall/gjson" +) + +// TestReplayCapturedTrafficBreakpointsReachTheWire replays real captured Claude +// Code requests (a CONTEXT_GURU_CAPTURE jsonl: {provider, model, body}) through the +// pipeline and asserts the three things #32 is about: +// +// 1. cacheinject's breakpoints appear on the wire (before the fix: 0 of 46 did); +// 2. the total never exceeds the provider's cap of 4; +// 3. the output stays valid JSON with every tool_use provider field intact. +// +// Skipped unless CONTEXT_GURU_CAPTURE names a readable capture, so CI does not need +// the fixture. Run with: +// +// CONTEXT_GURU_CAPTURE=/path/to/capture.jsonl go test ./apply/ -run ReplayCaptured +func TestReplayCapturedTrafficBreakpointsReachTheWire(t *testing.T) { + path := os.Getenv("CONTEXT_GURU_CAPTURE") + if path == "" { + t.Skip("set CONTEXT_GURU_CAPTURE to a captured-traffic jsonl to run this") + } + f, err := os.Open(path) + if err != nil { + t.Skipf("cannot read capture %q: %v", path, err) + } + defer f.Close() + + cfg := pipe(t, "pipeline: [cacheinject]\n") + p, _ := cfg.Build(nil) + st := store.NewMemory(store.Options{}) + + sc := bufio.NewScanner(f) + sc.Buffer(make([]byte, 1<<20), 64<<20) + requests, withMarks := 0, 0 + for sc.Scan() { + var rec struct { + Provider string `json:"provider"` + Body json.RawMessage `json:"body"` + } + if json.Unmarshal(sc.Bytes(), &rec) != nil || len(rec.Body) == 0 { + continue + } + requests++ + before := wireMarks(rec.Body) + out, _ := apply.Body(context.Background(), p, st, + bschemas.ModelProvider(rec.Provider), rec.Body, "", false) + + if !gjson.ValidBytes(out) { + t.Fatalf("request %d: output is not valid JSON", requests) + } + after := wireMarks(out) + if after > 4 { + t.Fatalf("request %d: %d breakpoints on the wire, provider caps at 4", requests, after) + } + if after > before { + withMarks++ + } + // Every tool_use block must keep id/name/input — the fields bifrost drops. + gjson.GetBytes(out, `messages.#.content|@flatten`).ForEach(func(_, blk gjson.Result) bool { + if blk.Get("type").String() != "tool_use" { + return true + } + if !blk.Get("id").Exists() || !blk.Get("name").Exists() || !blk.Get("input").Exists() { + t.Fatalf("request %d: tool_use lost a provider field: %s", requests, blk.Raw) + } + return true + }) + } + if requests == 0 { + t.Skipf("capture %q held no usable requests", path) + } + // The bug: 46 breakpoints applied, 0 forwarded, across 40 real requests. On these + // captures the unfixed code reaches the wire on 1 of 19 requests (the one message + // bifrost happens to round-trip), so "nonzero" is not a gate — require a majority. + if withMarks*2 <= requests { + t.Fatalf("breakpoints reached the wire on only %d of %d captured requests — still suppressed", + withMarks, requests) + } + t.Logf("replayed %d captured requests; breakpoints reached the wire on %d", requests, withMarks) +} + +// wireMarks counts cache_control across system, tools and messages, independently of +// the implementation's own counter. +func wireMarks(body []byte) int { + n := 0 + for _, p := range []string{"system", "tools"} { + gjson.GetBytes(body, p).ForEach(func(_, v gjson.Result) bool { + if v.Get("cache_control").Exists() { + n++ + } + return true + }) + } + gjson.GetBytes(body, "messages").ForEach(func(_, m gjson.Result) bool { + if m.Get("cache_control").Exists() { + n++ + } + m.Get("content").ForEach(func(_, blk gjson.Result) bool { + if blk.Get("cache_control").Exists() { + n++ + } + return true + }) + return true + }) + return n +} diff --git a/components/component.go b/components/component.go index 34abf17..ffe09f0 100644 --- a/components/component.go +++ b/components/component.go @@ -123,6 +123,17 @@ type Ctx struct { // -1 = unknown/first turn/cache off ⇒ no tail restriction. Only meaningful when // CacheAware is true. MaxCachedIdx int + // ExistingBreakpoints is how many prompt-cache breakpoints the RAW request already + // carries, counted across `system`, `tools` and `messages` — which is what the + // provider's cap of four applies to. A component that spends breakpoint slots must + // budget against this number rather than what it can see in Input, for two reasons + // measured on real Claude Code traffic: 2 of its 3 breakpoints live in the + // top-level `system` array, which components never see at all, and the third sits + // on a `tool_result` block whose cache_control bifrost drops on unmarshal. Both + // were invisible, so the budget came out as 3 free slots when only 1 was free — + // enough to put 6 on the wire and take a 400 (issue #32). The host fills it from + // the raw body; 0 means "unknown, fall back to what you can see". + ExistingBreakpoints int } // TailOnly reports whether a supersession/age-based offloader may mutate the message @@ -150,6 +161,17 @@ type Report struct { Reverted bool // pipeline reverted it (error/panic/never-worse) Irreversible bool // Offload dropped content on purpose without stashing (marker_mode summary/off) Err error + // ChangedIdx are the req.Input indices this component modified, filled by the + // pipeline. The writeback layer uses them to attribute a discarded change back to + // the component that made it (see Pipeline.RecordDiscards). + ChangedIdx []int + // Discarded counts changes this component made that the WRITEBACK layer then threw + // away (bifrost could not round-trip the message, so splicing would have dropped + // provider fields). A report carrying Discarded > 0 is a follow-up attribution, not + // a fresh run — emitters must not count it as one. A component that mutates and is + // then silently discarded looked identical to one that works, which is how issue + // #32 survived two benchmark studies. + Discarded int } // Saved returns non-negative tokens saved by this component. diff --git a/components/pipeline.go b/components/pipeline.go index 7a1786d..f227982 100644 --- a/components/pipeline.go +++ b/components/pipeline.go @@ -1,6 +1,8 @@ package components import ( + "bytes" + "encoding/json" "fmt" "github.com/maximhq/bifrost/core/schemas" @@ -93,6 +95,7 @@ func (p *Pipeline) runOne(comp Component, req *schemas.BifrostChatRequest, c *Ct return rep } + rep.ChangedIdx = changedIdx(before, req.Input) after := tokensOf(req.Input) switch { case err != nil: @@ -124,6 +127,47 @@ func tokensOf(msgs []schemas.ChatMessage) int { return schema.MessagesTokens(&schemas.BifrostChatRequest{Input: msgs}) } +// changedIdx returns the indices at which a component's output differs from its +// input, so the writeback layer can attribute a discarded change to the component +// that made it. Compared on the canonical marshal, the same form the writeback loop +// uses to decide "changed". Count changes (summarize) yield nil — those go down the +// rebuild path, which never discards per-message. +func changedIdx(before, after []schemas.ChatMessage) []int { + if len(before) != len(after) { + return nil + } + var out []int + for i := range after { + a, err1 := json.Marshal(before[i]) + b, err2 := json.Marshal(after[i]) + if err1 != nil || err2 != nil || !bytes.Equal(a, b) { + out = append(out, i) + } + } + return out +} + +// RecordDiscards emits one follow-up Report per component whose changes the +// writeback layer threw away, so a silently-suppressed component is visible in +// telemetry instead of looking like a working one. discarded maps req.Input index -> +// number of discarded changes at that index; hosts call this after the splice. +func (p *Pipeline) RecordDiscards(rr *RunReport, discarded map[int]int) { + if p == nil || rr == nil || len(discarded) == 0 { + return + } + for _, rep := range rr.Components { + n := 0 + for _, i := range rep.ChangedIdx { + n += discarded[i] + } + if n == 0 { + continue + } + d := Report{Component: rep.Component, Kind: rep.Kind, Discarded: n} + safeEmit(func() { p.emitter.Component(d) }) + } +} + // Has reports whether a component with this name is configured in the pipeline. // Hosts use it to gate body-level work that belongs to a component's concern but // cannot be done inside it — e.g. cacheinject's cache-prefix repair, which must diff --git a/components/reformat/cacheinject.go b/components/reformat/cacheinject.go index fe3e7e8..ca8d05a 100644 --- a/components/reformat/cacheinject.go +++ b/components/reformat/cacheinject.go @@ -141,15 +141,25 @@ func (ci Cacheinject) Reformat(req *schemas.BifrostChatRequest, rep *components. } applied := 0 - // Count breakpoints the caller already set: they occupy provider slots, and an - // agent that sets its own (claude-code does) is already at the optimum. - existing := 0 + // Breakpoints the caller already set occupy provider slots, and an agent that sets + // its own (claude-code does) is already at the optimum. Two separate things must + // happen with them: the positions we can SEE are dropped from `want` (never mark + // twice), and the BUDGET is computed from the host's raw-body count, which also + // sees the ones we cannot — the `system` array components never receive, and + // `tool_result` blocks whose cache_control bifrost drops on unmarshal. On real + // Claude Code traffic that is all 3 of them, so counting only Input gave a budget + // of 3 free slots when 1 was free: 6 on the wire, and a 400 (issue #32). + visible := 0 for i := range req.Input { if hasBreakpoint(&req.Input[i]) { - existing++ + visible++ delete(want, i) } } + existing := visible + if c != nil && c.ExistingBreakpoints > visible { + existing = c.ExistingBreakpoints + } budget := maxBreakpoints - existing if budget <= 0 { rep.Skipped = true // no slots left; adding one would be a 400 from the provider diff --git a/components/reformat/cacheinject_test.go b/components/reformat/cacheinject_test.go index b93623f..26fe713 100644 --- a/components/reformat/cacheinject_test.go +++ b/components/reformat/cacheinject_test.go @@ -281,3 +281,51 @@ func TestTTLConfig(t *testing.T) { t.Fatal("an unsupported ttl must be rejected, not silently accepted") } } + +// The provider's cap counts breakpoints this component cannot see: `system` and +// `tools` never reach it, and bifrost drops cache_control on block types it does not +// model. Real Claude Code traffic carries (system=2, tools=0, messages=1), so the +// true remaining budget is 1, not 3 (issue #32, defect 2). The synthetic 60-message +// shape below is exactly the probe that produced 4 message marks — 6 on the wire. +func TestBudgetCountsInvisibleBreakpoints(t *testing.T) { + msgs := convo(60) + mark(&msgs[59], nil) // the caller's own message breakpoint + + c := ctx() + c.ExistingBreakpoints = 3 // (system=2, tools=0, messages=1) — the real shape + req := &schemas.BifrostChatRequest{Provider: schemas.Anthropic, Input: msgs} + rep := &components.Report{} + if err := (Cacheinject{}).Reformat(req, rep, c); err != nil { + t.Fatalf("Reformat: %v", err) + } + got := marked(req.Input) + // The caller's own message breakpoint is one of the 3 counted, so the wire total is + // c.ExistingBreakpoints plus whatever we added on top. + added := 0 + for _, i := range got { + if i != 59 { + added++ + } + } + if wire := c.ExistingBreakpoints + added; wire > maxBreakpoints { + t.Fatalf("wire breakpoints %d > cap %d (marked %v, existing %d)", + wire, maxBreakpoints, got, c.ExistingBreakpoints) + } +} + +// Four breakpoints already on the wire leave no budget at all: adding any would be a 400. +func TestExistingBreakpointsCanExhaustTheBudget(t *testing.T) { + c := ctx() + c.ExistingBreakpoints = maxBreakpoints + req := &schemas.BifrostChatRequest{Provider: schemas.Anthropic, Input: convo(30)} + rep := &components.Report{} + if err := (Cacheinject{}).Reformat(req, rep, c); err != nil { + t.Fatalf("Reformat: %v", err) + } + if !rep.Skipped { + t.Fatal("expected a skip: the caller's own breakpoints already fill the cap") + } + if got := marked(req.Input); len(got) != 0 { + t.Fatalf("marked %v over a full budget", got) + } +} diff --git a/docs/components/cacheinject.md b/docs/components/cacheinject.md index 1f7aea4..f20085f 100644 --- a/docs/components/cacheinject.md +++ b/docs/components/cacheinject.md @@ -4,6 +4,39 @@ Places Anthropic `cache_control` breakpoints at the positions that minimise billed input cost, so the provider KV cache is read rather than re-processed. +!!! danger "Every placement number below predates the fix in #32 — read this first" + Until #32, **this component's breakpoints never reached the wire on Claude Code + traffic.** Measured over 40 captured requests: **46 breakpoints applied at the + component level, 0 in the output body.** + + The cause was structural, not a tuning error. The component can only mark messages + carrying content *blocks*; on Claude Code traffic those are exclusively assistant + turns carrying `tool_use` — and bifrost drops `tool_use.id/name/input` on unmarshal, + so `apply`'s losslessness guard correctly discarded every one of those changes + rather than splice a corrupted re-marshal. A component whose only possible targets + were precisely the messages that could never be written back. + + A second, independent defect compounded it: the 4-breakpoint budget counted only + `messages`. Real traffic puts 2 of its 3 breakpoints in the top-level `system` array + and the third on a `tool_result` block whose `cache_control` bifrost drops — all + three invisible. The component computed 3 free slots when 1 was free. On a + 60-message request it emitted 4 message marks, **6 on the wire**, which the provider + rejects with a 400. That never fired in production *only* because the first defect + suppressed the marks; fixing one without the other would have produced a live 400. + + Both are fixed (see [design.md](../design.md) — the metadata-write exception). + Breakpoints now reach the wire on **86 of 92** replayed captured requests, and the + wire total is asserted never to exceed 4. + + **Consequence for every figure on this page:** the placement rows measured a + component whose output was discarded. The `acted=0` / "placement contributes $0" + findings are still *true as recorded* — nothing reached the provider, so nothing + could have contributed — but they are **not** evidence that placement has no + headroom. That question had not been asked until #32; see + [What placement is actually worth](#what-placement-is-actually-worth) for the + first honest measurement. The volatile-tail-split figures are unaffected: that is a + body-level rewrite which never went through the discarded path. + ## How it works Placement is the solution to a cost minimisation, not a heuristic. With `R = 0.1` @@ -64,12 +97,20 @@ the cache-hit rate a live 50-task run actually billed: **Against an agent that already caches well, expect exactly 0%.** Measured on real traffic: claude-code marks its own final message on **466 of 472 requests (98.7%)**, so rule 1 is already satisfied and every candidate position this component would -choose collides with an existing breakpoint. Instrumented over a live 12-task run it -placed **zero** extra breakpoints (`runs 432, acted 0`) and was byte-identical to -baseline on the wire. +choose collides with an existing breakpoint. + +!!! warning "The `acted 0` figure below was not a measurement of placement" + The same 12-task run also reported `runs 432, acted 0` and "byte-identical to + baseline on the wire". That was read as *the policy chose to place nothing*. It was + not: the component **did** place breakpoints and the writeback layer discarded every + one before the request was sent (#32). The wire was byte-identical because nothing + reached it, not because the policy declined. Both statements — placement collides + with existing breakpoints, and nothing reached the wire — were true at once, and the + second one masked the first from ever being tested. See + [What placement is actually worth](#what-placement-is-actually-worth). -So this component's value is precisely two things, neither of which is a saving -against claude-code: +So this component's value was claimed to be precisely two things, neither of which is a +saving against claude-code: 1. It **stops v1's +5.5% regression** (see the warning below). 2. It places breakpoints for agents that do *not* mark their own tail, and it anchors @@ -77,7 +118,7 @@ against claude-code: claude-code needs. The measurable cost saving on claude-code traffic comes from the **cross-session -prefix repairs** in `apply/prefixorder.go`, which are gated on this component being +prefix repairs** in `apply/prefixsplit.go`, which are gated on this component being configured but are a different mechanism. See below. !!! warning "v1 was a regression" @@ -87,6 +128,47 @@ configured but are a different mechanism. See below. the savings as "invisible to `/stats`", which made a negative effect unfalsifiable. If you are pinning an older release, this component cost money. +## What placement is actually worth + +Placeholder — filled by the `cacheonly` vs `off` measurement in #32. + +## How the breakpoints reach the wire + +Worth knowing, because it is where this component was broken for two benchmark +studies. `cacheinject` computes *where* the breakpoints go; `apply` performs the write. + +The component can only mark a message carrying content **blocks**, and on Claude Code +traffic those are exclusively assistant turns carrying `tool_use`. bifrost drops +`tool_use.id/name/input` on unmarshal, so `apply`'s losslessness guard cannot splice a +re-marshal of such a message without corrupting it — and correctly refused to. + +The fix is not to relax that guard. `cache_control` is **metadata, not content**: it +changes nothing the model reads, so it needs no message model to express. `apply` writes +it as a targeted `sjson` key at `messages..content..cache_control` on the +**original raw bytes**, verifying first that the component's *only* change to that +message was adding `cache_control` keys. A write that reads no other field cannot drop +one. Any wider change is still discarded, and a discard now increments +`discarded_changes` for the responsible component in `/stats` — the observability gap +that let this survive unnoticed. + +## The 4-breakpoint budget is computed by the host, not the component + +The provider caps `cache_control` at **4 across `system` + `tools` + `messages` +together**, and a component sees none of the first two. Worse, bifrost drops +`cache_control` on block types it does not model, so even some *message* breakpoints are +invisible to it. + +On real Claude Code traffic that hides **all three** of the agent's own breakpoints — +measured, 1,771 of 1,794 requests carry exactly `(system=2, tools=0, messages=1)`, with +the message one sitting on a `tool_result` block. Counting only what it could see, the +component computed `budget = 4 − 1 = 3` when 1 slot was free, and on a 60-message +request emitted 4 marks: **6 on the wire, which the provider rejects with a 400.** + +So `apply` counts them structurally from the raw body and passes the total as +`Ctx.ExistingBreakpoints`; the component budgets against that. `apply` also counts the +output and logs an error if it ever exceeds 4, so a breach shows up in telemetry rather +than as a provider 400. + ## Lossiness None. It attaches cache directives only; model-visible content is unchanged, and @@ -117,7 +199,12 @@ scratchpad, a re-rendered header, an injected timestamp). On OpenAI- and Gemini-shaped wires, where `cache_control` does not exist at all — placement has nothing to express and the split has nothing to gain (see below). Also -inert when four breakpoints are already present, and on string-content messages. +inert when four breakpoints are already present **anywhere in the request** (including +`system` and `tools`), and on string-content messages. + +That last case is now the common one on Claude Code traffic, and it is correct: the agent +already spends 3 of the 4 slots, so this component has exactly one to place. Before #32 +it believed it had three. ## The volatile-tail split diff --git a/docs/design.md b/docs/design.md index 7aad713..73c2e09 100644 --- a/docs/design.md +++ b/docs/design.md @@ -111,7 +111,7 @@ sequenceDiagram apply->>apply: normalize → []ChatMessage + write-back slots apply->>Pipe: Run(chat, ctx) Pipe-->>apply: mutated messages - apply->>apply: per message: unchanged → keep bytes,
changed & lossless round-trip → sjson splice + apply->>apply: per message: unchanged → keep bytes,
changed & lossless round-trip → sjson splice,
changed & metadata-only → sjson key write on raw bytes,
else discard + count apply-->>Host: rewritten body (or original, fail open) ``` @@ -124,6 +124,35 @@ Non-string tool_result content is skipped (never lose non-text). A whole-message spliced back if bifrost round-trips that message losslessly (`jsonEqual`); otherwise the change is discarded — correctness over the marginal saving. +**The metadata exception.** That guard has one deliberate hole, and it exists because the guard +alone made `cacheinject` a no-op. bifrost drops `tool_use.id/name/input` on unmarshal, so every +Anthropic assistant turn carrying a `tool_use` is non-round-trippable — and those are exactly the +only messages `cacheinject` can mark. Measured on 40 captured Claude Code requests: **46 +breakpoints applied at the component level, 0 in the output body** (issue #32). + +So `apply/metawrite.go` adds a narrow path: when a component's *only* change to a message is an +added `cache_control` key, that key is written at its exact path (`messages..content..cache_control`) +on the **original raw bytes** via `sjson`. `cache_control` is metadata, not content — it changes +nothing the model reads, so it needs no message model to express, and a targeted `sjson` write +provably cannot drop a field it never reads. The `metadataOnlyWrites` diff enforces "only that": +a text edit, a removed key, a changed block count, anything else at all, and the change is still +discarded. `applyMetaWrites` additionally refuses if the raw body's block layout disagrees with +the normalized view, so a key can never land on the wrong block, and never overwrites a +breakpoint the caller set. + +**Discards are now loud.** `Pipeline.RecordDiscards` attributes each thrown-away change back to +the component that made it (via `Report.ChangedIdx`), surfacing as `discarded_changes` per +component and `top_discarded` in `/stats`. Before this, a mutated-then-discarded component looked +byte-identical to a working Reformat — which is how #32 survived two full benchmark studies. + +**Breakpoint budgeting is a host job.** The provider caps `cache_control` at 4 across `system` + +`tools` + `messages` together, and a component sees none of the first two — nor cache_control on +blocks bifrost drops. On real Claude Code traffic that hides all three of the agent's own +breakpoints, so a component counting only what it saw computed 3 free slots when 1 was free. +`apply` counts them from the raw body (`wireBreakpoints`) and passes the total as +`Ctx.ExistingBreakpoints`; exceeding the cap on output logs an error rather than waiting for the +provider's 400. + If a component changes the message *count* (none of the v1 set does), the slot map no longer aligns, so `apply` forwards the original untouched. diff --git a/metrics/metrics.go b/metrics/metrics.go index c4b345f..5e08b94 100644 --- a/metrics/metrics.go +++ b/metrics/metrics.go @@ -49,6 +49,7 @@ func (s Slog) Component(r components.Report) { "context_engineering.tokens.after", r.TokensAfter, "context_engineering.tokens.saved", r.Saved(), "context_engineering.reverted", r.Reverted, + "context_engineering.discarded_changes", r.Discarded, "context_engineering.duration_ms", r.DurationMs, ) } @@ -96,9 +97,15 @@ type compStat struct { // OvercountRatio = Saved / SavedUnique — how many times, on average, each distinct // compaction was re-counted (the agent re-sends history verbatim every turn). ~1.0 // is honest; large values mean the cumulative figure is inflated by re-sends. - OvercountRatio float64 `json:"overcount_ratio"` - DurationMs float64 `json:"duration_ms"` // cumulative wall time this component spent (its own latency cost on the hot path) - seenKeys map[string]struct{} // content keys already counted toward SavedUnique (not serialized) + OvercountRatio float64 `json:"overcount_ratio"` + DurationMs float64 `json:"duration_ms"` // cumulative wall time this component spent (its own latency cost on the hot path) + // Discarded counts changes this component made that the WRITEBACK layer then threw + // away (bifrost could not round-trip the message, so splicing would have dropped + // provider fields). Nonzero means the component ran, mutated, and had no effect on + // the wire — which for two whole benchmark studies looked exactly like a working + // Reformat (issue #32). + Discarded int64 `json:"discarded_changes"` + seenKeys map[string]struct{} // content keys already counted toward SavedUnique (not serialized) } // NewAggregator returns an empty aggregator. @@ -112,6 +119,13 @@ func (a *Aggregator) Component(r components.Report) { cs = &compStat{} a.perComp[r.Component] = cs } + // A Discarded report is a follow-up from the writeback layer attributing thrown-away + // changes to the component that made them — not a fresh run. Count it and stop, or + // Runs would double per request. + if r.Discarded > 0 { + cs.Discarded += int64(r.Discarded) + return + } cs.Runs++ cs.Saved += int64(r.Saved()) cs.DurationMs += r.DurationMs // per-component latency cost on the hot path @@ -207,6 +221,11 @@ type Snapshot struct { // TopPassthrough names components that ran but never saved a token — dead // weight in the pipeline, candidates to drop from the config. TopPassthrough []string `json:"top_passthrough"` + // TopDiscarded names components whose changes the writeback layer threw away at + // least once — they mutated the request but (for those changes) never reached the + // wire. Any entry here needs investigating; see the per-component + // `discarded_changes` for the count. + TopDiscarded []string `json:"top_discarded"` // LLM* report the cheap (config-source) model usage the CONTEXT-GURU components // themselves incurred (e.g. extract:code's Starlark-writer calls) — the CG // components' OWN cost, separate from the agent. Priced externally. @@ -231,8 +250,11 @@ func (a *Aggregator) Snapshot() Snapshot { pct = float64(saved) / float64(a.before) * 100 } comps := make(map[string]compStat, len(a.perComp)) - var passthrough []string + var passthrough, discarded []string for k, v := range a.perComp { + if v.Discarded > 0 { + discarded = append(discarded, k) + } cs := *v if cs.SavedUnique > 0 { cs.OvercountRatio = float64(cs.Saved) / float64(cs.SavedUnique) @@ -247,6 +269,7 @@ func (a *Aggregator) Snapshot() Snapshot { } } sort.Strings(passthrough) + sort.Strings(discarded) addedAvg, upAvg, upAvgByp := 0.0, 0.0, 0.0 if a.addedSamples > 0 { addedAvg = a.addedMs / float64(a.addedSamples) @@ -261,7 +284,7 @@ func (a *Aggregator) Snapshot() Snapshot { Requests: a.requests, TokensBefore: a.before, TokensAfter: a.after, SavedTokens: saved, SavingsPct: pct, WastedTokens: a.wasted, Bounces: a.bounces, AdjustedSaved: saved - a.wasted, - Components: comps, TopPassthrough: passthrough, + Components: comps, TopPassthrough: passthrough, TopDiscarded: discarded, AddedLatencyMsAvg: addedAvg, UpstreamMsAvg: upAvg, UpstreamMsAvgBypassed: upAvgByp, } } From db58dbff58b4f163d5f97c68c7326ae67528edcc Mon Sep 17 00:00:00 2001 From: Osher-Elhadad Date: Mon, 10 Aug 2026 04:20:30 +0000 Subject: [PATCH 2/3] docs(results): flag the cacheinject per-component row as a suppressed measurement The components page credits cacheinject with the 97.8% cache-hit rate. It did not earn it: on that run its breakpoints never reached the wire (#32), so the rate is claude-code's own breakpoints, forwarded untouched. Note it in place rather than delete the row, so a reader comparing against an older build sees why the number moved. Assisted-By: Claude Opus 5 Signed-off-by: Osher-Elhadad --- docs/results/components.md | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/docs/results/components.md b/docs/results/components.md index 52bf94c..7adc9e9 100644 --- a/docs/results/components.md +++ b/docs/results/components.md @@ -102,8 +102,19 @@ Run: **245 acts, 34,293 cumulative / ~2.9k unique tok**, ~0.6 s total (near-zero ### 7. `cacheinject` (Reformat) Stamps an ephemeral `cache_control` breakpoint on the prefix boundary so the provider KV -cache hits across turns. No content change. Its payoff is systemic — the 97.8% cache-hit -rate the whole cache-aware design is tuned around. +cache hits across turns. No content change. + +!!! danger "This row measured a suppressed component" + On the run behind this page, `cacheinject`'s breakpoints **never reached the wire** — + 46 applied at the component level, 0 in the output body across 40 captured requests + ([#32](https://github.com/rossoctl/context-guru/issues/32)). Its only possible targets + are assistant turns carrying `tool_use`, which bifrost cannot round-trip, so the + writeback layer discarded every mark. + + So the **97.8% cache-hit rate is not attributable to this component.** That rate is + claude-code's own breakpoints, which it sets on every request and which the proxy + forwarded untouched. Fixed in #32; a re-run must re-measure this row rather than carry + the number forward. --- From c122daf17fa61e96dbc6b0dbb3ff1dee6f1adfda Mon Sep 17 00:00:00 2001 From: Osher-Elhadad Date: Mon, 10 Aug 2026 09:02:21 +0000 Subject: [PATCH 3/3] fix(cache): close the Bedrock cap gap, stop misattributing discards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on the #32 metadata-write fix. Two of these are bugs the original change introduced. The cap was still breachable on Bedrock. `cachePoint` was only counted under `messages.#.content.#`, but Bedrock places it as its own entry in `system` and `tools` — precisely the two locations defect 2 is about. Constructed, that put 6 breakpoints on the wire with the new breach check also silent, since it reads the same blind counter. Both paths are counted now. `discarded_changes` misattributed twice, which matters because this is the counter whose whole job is catching #32-class bugs; a false-positive generator would not be trusted. `ChangedIdx` was recorded before the revert branches, so a rolled-back component was charged for a discard caused by a later one. And a single discarded message was charged to every component that had touched it. Now `ChangedIdx` is recorded only on the surviving path, and one discard goes to exactly one component — the last to change that message, whose state is what writeback actually threw away. Both have tests that fail without the fix. The breach ERROR blamed us for client-caused breaches. A request arriving already over the cap is forwarded untouched (fail open), so shouting about it named the wrong culprit; it now fires only when our own output exceeds both the cap and the inbound count. Correct the root cause, which was wrong in five comments and three doc pages. bifrost does NOT drop `cache_control` on `tool_result` — tested directly, it round-trips fine. The mark is dropped by this repo's own `toolMessage()` in `normalize`, which rebuilds the block into a synthetic role=tool message from text and tool_use_id alone. The fix works either way, but reasoning from a false premise is how the original bug survived two benchmark studies. Swap the diagnostic's double `json.Marshal` for `reflect.DeepEqual`: 20.52 ms/op -> 16.56 ms (-19.3%) and 3,206 fewer allocs on a realistic 80-message request. The writeback loop's own marshal is what decides whether to splice; this only needs to know which indices moved. Drop `cacheinject` from every preset. Its breakpoints only started reaching the provider with #32 and placement has never been shown to help, so enabling it by default ships an unmeasured policy on every request. The issue anticipated exactly this: "if the fix proves cacheinject harmful once live, the answer is to remove it from the default preset — a config change, not a new knob." That required separating two mechanisms that shared one config entry. The volatile-tail split was gated on `cacheinject` merely because it needed somewhere to hang, but the split is measured (-34.1% cost, 0% -> 96.7% hit in an isolated A/B) while placement is not. Dropping cacheinject alone would have silently disabled the split too, turning "disable an unproven component" into a real cost regression. So `cachesplit` is a marker component carrying the split, and the presets use it. It stays gated rather than unconditional so `off` remains a true passthrough control for A/B runs. Also retract the benchmark mechanism claim. The cache-write direction stands as recorded but has no established mechanism, and the one I proposed is disproven: 0 of 106 marks land above claude-code's own breakpoint across three captures — ours sits one message below, where Rule 2 says an extra breakpoint costs zero. `acted=0` also does not isolate placement, since `splitVolatileTail` is live in that arm. Assisted-By: Claude Opus 5 Signed-off-by: Osher-Elhadad --- apply/apply.go | 28 ++++--- apply/apply_test.go | 102 ++++++++++++++++++++++++ apply/metawrite.go | 18 +++-- apply/metawrite_test.go | 48 ++++++++++- apply/prefixsplit_test.go | 25 +++++- components/component.go | 2 +- components/pipeline.go | 50 ++++++++---- components/reformat/cacheinject.go | 34 +++++++- components/reformat/cacheinject_test.go | 4 +- config/config.go | 30 +++---- config/config_test.go | 31 ++++++- docs/components.md | 24 +++--- docs/components/cacheinject.md | 87 ++++++++++++++++---- docs/design.md | 19 +++-- docs/how-to/choose-a-preset.md | 21 ++--- 15 files changed, 430 insertions(+), 93 deletions(-) diff --git a/apply/apply.go b/apply/apply.go index 38320d9..14ce934 100644 --- a/apply/apply.go +++ b/apply/apply.go @@ -132,11 +132,18 @@ func BodyFull(ctx context.Context, pipe *components.Pipeline, st store.Store, pr // Volatile-tail split, before anything else touches the body. This is a // body-level concern rather than a component one: the pipeline operates on // `messages`, but the block that needs splitting lives in the top-level `system` - // array, which components never see. Gated on cacheinject being configured, so it - // is opt-in via the same pipeline entry and adds no new config surface. See - // prefixsplit.go for what it splits and why. + // array, which components never see. See prefixsplit.go for what it splits and why. + // + // Gated on `cachesplit` OR `cacheinject`. The two were coupled only because the split + // had to hang off some existing config entry, but they are independent mechanisms + // with very different evidence: the split is measured (−34.1% cost, 0% → 96.7% hit in + // an isolated A/B), while breakpoint PLACEMENT has never been measured — so #32 drops + // cacheinject from the default presets and puts `cachesplit` there instead. Without + // its own gate the split would have gone down with cacheinject, turning "disable an + // unproven component" into a real cost regression. It stays opt-in rather than + // unconditional so `off` remains a true passthrough control for A/B runs. systemSplit := false - if !bypass && pipe != nil && pipe.Has("cacheinject") { + if !bypass && pipe != nil && (pipe.Has("cachesplit") || pipe.Has("cacheinject")) { body, systemSplit = splitVolatileTail(body, provider) } @@ -171,7 +178,7 @@ func BodyFull(ctx context.Context, pipe *components.Pipeline, st store.Store, pr CacheAware: cacheAware, MaxCachedIdx: maxCachedIdx, // Every breakpoint already on the wire — including the ones no component can - // see (`system`, `tools`, and cache_control on blocks bifrost drops). The + // see (`system`, `tools`, and the marks our own normalize drops). The // provider's cap of four counts them all (issue #32, defect 2). ExistingBreakpoints: wireBreakpoints(body), } @@ -258,11 +265,14 @@ func BodyFull(ctx context.Context, pipe *components.Pipeline, st store.Store, pr if changed && dumpPath != "" { dumpChanges(c.Session, changes) } - if n := wireBreakpoints(out); n > maxWireBreakpoints { - // Should be unreachable: the component budgets against OuterBreakpoints. Loud - // rather than a 400 from the provider, and it names the request shape. + // A cap breach WE caused is a bug and must be loud. A request that arrived already + // over the cap is the client's to fix — we forward it as-is (fail open), and an ERROR + // blaming context-guru for it would be a false alarm. So compare against the inbound + // count and only shout when we added to an over-cap total. + if n := wireBreakpoints(out); n > maxWireBreakpoints && n > c.ExistingBreakpoints { slog.Error("context-guru: cache breakpoint count exceeds the provider cap", - "breakpoints", n, "cap", maxWireBreakpoints, "session", c.Session) + "breakpoints", n, "inbound", c.ExistingBreakpoints, + "cap", maxWireBreakpoints, "session", c.Session) } return out, changed } diff --git a/apply/apply_test.go b/apply/apply_test.go index 1bed916..6c143a8 100644 --- a/apply/apply_test.go +++ b/apply/apply_test.go @@ -3,6 +3,7 @@ package apply_test import ( "context" "encoding/json" + "errors" "reflect" "strings" "testing" @@ -456,3 +457,104 @@ func TestDiscardedChangeIsCounted(t *testing.T) { t.Fatalf("Runs should stay 1, got %d", r) } } + +// reverter always errors, so the pipeline rolls its change back. A reverted component +// must NOT be charged a discard: its change never reached the writeback layer at all. +type reverter struct{} + +func (reverter) Name() string { return "testrevert" } +func (reverter) Enabled(*components.Ctx) bool { return true } +func (reverter) Reformat(req *bschemas.BifrostChatRequest, _ *components.Report, _ *components.Ctx) error { + for i := range req.Input { + if req.Input[i].Role == bschemas.ChatMessageRoleAssistant && req.Input[i].Content != nil { + for b := range req.Input[i].Content.ContentBlocks { + if req.Input[i].Content.ContentBlocks[b].Text != nil { + s := "mutated then reverted" + req.Input[i].Content.ContentBlocks[b].Text = &s + } + } + } + } + return errors.New("deliberate failure so the pipeline reverts") +} + +func init() { + components.Register("testrevert", func([]byte) (components.Component, error) { return reverter{}, nil }) +} + +// A rolled-back component must not appear as a discard. Otherwise the counter meant to +// catch #32-class bugs becomes a false-positive generator. +func TestRevertedComponentNotChargedDiscard(t *testing.T) { + // testrewrite runs AFTER and produces the discard, so the writeback layer really does + // throw a change away at that index. Pre-fix, testrevert's ChangedIdx was recorded + // before the rollback, so it got charged for a discard caused by the other component. + cfg := pipe(t, "pipeline: [testrewrite, testrevert]\n") + agg := metrics.NewAggregator() + p, _ := cfg.Build(agg) + + body := []byte(`{"model":"claude-x","messages":[ + {"role":"user","content":"go"}, + {"role":"assistant","content":[{"type":"text","text":"narration"},{"type":"tool_use","id":"toolu_r","name":"Bash","input":{}}]} + ]}`) + apply.Body(context.Background(), p, store.NewMemory(store.Options{}), bschemas.Anthropic, body, "", false) + + cs := agg.Snapshot().Components["testrevert"] + if cs.Reverted != 1 { + t.Fatalf("expected the component to be reverted, got %+v", cs) + } + if cs.Discarded != 0 { + t.Fatalf("reverted component charged %d discards — its change never reached writeback", cs.Discarded) + } +} + +// One discarded message is charged to exactly ONE component, not to every component +// that touched it. Two components rewrite the same unmodellable message; only the last +// one's change is what the writeback layer actually threw away. +func TestDiscardChargedOnceNotPerToucher(t *testing.T) { + cfg := pipe(t, "pipeline: [testrewrite, testrewrite2]\n") + agg := metrics.NewAggregator() + p, _ := cfg.Build(agg) + + body := []byte(`{"model":"claude-x","messages":[ + {"role":"user","content":"go"}, + {"role":"assistant","content":[{"type":"text","text":"a long narration both components rewrite"},{"type":"tool_use","id":"toolu_d","name":"Bash","input":{}}]} + ]}`) + apply.Body(context.Background(), p, store.NewMemory(store.Options{}), bschemas.Anthropic, body, "", false) + + snap := agg.Snapshot() + total := snap.Components["testrewrite"].Discarded + snap.Components["testrewrite2"].Discarded + if total != 1 { + t.Fatalf("one discarded message charged %d times (testrewrite=%d testrewrite2=%d)", + total, snap.Components["testrewrite"].Discarded, snap.Components["testrewrite2"].Discarded) + } + // It must land on the LAST toucher — its change is the discarded state. + if snap.Components["testrewrite2"].Discarded != 1 { + t.Fatalf("discard should be charged to the last component to change the message, got %v", snap.TopDiscarded) + } +} + +// contentRewriter2 is a second rewriter so two components touch one message. +type contentRewriter2 struct{} + +func (contentRewriter2) Name() string { return "testrewrite2" } +func (contentRewriter2) Enabled(*components.Ctx) bool { return true } +func (contentRewriter2) Reformat(req *bschemas.BifrostChatRequest, _ *components.Report, _ *components.Ctx) error { + for i := range req.Input { + if req.Input[i].Role != bschemas.ChatMessageRoleAssistant || req.Input[i].Content == nil { + continue + } + for b := range req.Input[i].Content.ContentBlocks { + if req.Input[i].Content.ContentBlocks[b].Text != nil { + s := "even shorter" + req.Input[i].Content.ContentBlocks[b].Text = &s + } + } + } + return nil +} + +func init() { + components.Register("testrewrite2", func([]byte) (components.Component, error) { + return contentRewriter2{}, nil + }) +} diff --git a/apply/metawrite.go b/apply/metawrite.go index cffdcce..37b5386 100644 --- a/apply/metawrite.go +++ b/apply/metawrite.go @@ -103,21 +103,29 @@ const maxWireBreakpoints = 4 // cap applies across all of them together. Structural (gjson path queries) for the // same reason hasCacheBreakpoint is: a tool output whose text merely contains the // string "cache_control" must not count. +// +// `cachePoint` is the Bedrock Converse spelling, and Bedrock places it as its OWN +// entry in the `system` and `tools` arrays — the two locations defect 2 is about. Those +// paths must be counted or the cap stays breachable on Bedrock exactly as it was on +// Anthropic (constructed: 6 on the wire, counter blind to 3 of them). var breakpointPaths = []string{ "system.#.cache_control", "tools.#.cache_control", "messages.#.cache_control", "messages.#.content.#.cache_control", + "system.#.cachePoint", + "tools.#.cachePoint", "messages.#.content.#.cachePoint", } // wireBreakpoints counts every breakpoint the provider will see in this request. // -// A component cannot count these for itself: `system` and `tools` never reach it, and -// bifrost drops cache_control on block types it does not model — on real Claude Code -// traffic that hides all three of the agent's own breakpoints (2 in `system`, 1 on a -// `tool_result` block). Counting only what the component saw yielded a budget of 3 -// free slots when 1 was free: 6 on the wire, and a 400 (issue #32). +// A component cannot count these for itself. `system` and `tools` never reach it at +// all, and the `tool_result` blocks this package normalizes into synthetic role=tool +// messages lose their mark on the way in (see toolMessage) — so on real Claude Code +// traffic all three of the agent's own breakpoints were invisible to it (2 in +// `system`, 1 on a `tool_result` block), and it computed 3 free slots when 1 was free: +// 6 on the wire, and a 400 (issue #32). func wireBreakpoints(body []byte) int { n := 0 for _, p := range breakpointPaths { diff --git a/apply/metawrite_test.go b/apply/metawrite_test.go index 9991093..67e8b6f 100644 --- a/apply/metawrite_test.go +++ b/apply/metawrite_test.go @@ -1,6 +1,10 @@ package apply -import "testing" +import ( + "testing" + + "github.com/tidwall/sjson" +) // The metadata-write exception must be exactly that: added cache_control keys and // nothing else. Anything wider would put us back to splicing a lossy re-marshal @@ -42,6 +46,21 @@ func TestApplyMetaWritesRefusesOnShapeMismatch(t *testing.T) { } } +// Bedrock spells the breakpoint `cachePoint` and puts it as its OWN entry in `system` +// and `tools` — the two locations defect 2 is about. Missing those paths left the cap +// breachable on Bedrock exactly as it was on Anthropic. +func TestBedrockCachePointCounted(t *testing.T) { + body := []byte(`{ + "system":[{"text":"a"},{"cachePoint":{"type":"default"}},{"text":"b"},{"cachePoint":{"type":"default"}}], + "tools":[{"toolSpec":{"name":"x"}},{"cachePoint":{"type":"default"}}], + "messages":[{"role":"user","content":[{"text":"hi"},{"cachePoint":{"type":"default"}}]}]}`) + // 2 in system + 1 in tools + 1 in message content = 4, all of them at the cap. The + // pre-fix counter saw only the content one, so it believed 3 slots were free. + if got := wireBreakpoints(body); got != 4 { + t.Fatalf("wireBreakpoints = %d, want 4 (2 system + 1 tool + 1 content); a Bedrock cap breach would go unseen", got) + } +} + func TestBreakpointCounting(t *testing.T) { body := []byte(`{ "system":[{"type":"text","cache_control":{"type":"ephemeral"}},{"type":"text","cache_control":{"type":"ephemeral"}},{"type":"text"}], @@ -55,3 +74,30 @@ func TestBreakpointCounting(t *testing.T) { t.Errorf("wireBreakpoints = %d, want 5", got) } } + +// A request that arrives already over the cap is the client's problem — we forward it +// untouched. Shouting ERROR about it would blame context-guru for a request it never +// changed, so the breach check compares against the inbound count. Asserted at the +// condition level: the log call itself is not observable from here. +func TestBreachIsOursOnly(t *testing.T) { + over := []byte(`{"system":[ + {"type":"text","cache_control":{"type":"ephemeral"}},{"type":"text","cache_control":{"type":"ephemeral"}}, + {"type":"text","cache_control":{"type":"ephemeral"}},{"type":"text","cache_control":{"type":"ephemeral"}}, + {"type":"text","cache_control":{"type":"ephemeral"}}],"messages":[{"role":"user","content":"hi"}]}`) + inbound := wireBreakpoints(over) + if inbound != 5 { + t.Fatalf("fixture should carry 5 inbound breakpoints, got %d", inbound) + } + // Unchanged body: over the cap, but out == inbound, so it is not ours to report. + if out := wireBreakpoints(over); out > maxWireBreakpoints && out > inbound { + t.Fatal("a client-caused breach we did not add to must not be reported as ours") + } + // If we DID add one, it must be reported. + worse, err := sjson.SetRawBytes(over, "messages.0.cache_control", []byte(`{"type":"ephemeral"}`)) + if err != nil { + t.Fatal(err) + } + if out := wireBreakpoints(worse); !(out > maxWireBreakpoints && out > inbound) { + t.Fatalf("we added a breakpoint over the cap (%d -> %d); that must be reported", inbound, out) + } +} diff --git a/apply/prefixsplit_test.go b/apply/prefixsplit_test.go index 1982a69..80072c6 100644 --- a/apply/prefixsplit_test.go +++ b/apply/prefixsplit_test.go @@ -196,13 +196,32 @@ func TestSplitAppliedThroughBodyFull(t *testing.T) { } } -// Gated on cacheinject: a pipeline without it must leave the body byte-identical. -func TestSplitGatedOnCacheinject(t *testing.T) { +// Gated: a pipeline with neither cachesplit nor cacheinject must leave the body +// byte-identical, so `off` stays a true passthrough control for A/B runs. +func TestSplitGatedOnConfig(t *testing.T) { full, _, _ := blockWithGitTail(6000) in := sysBody(textBlock(full, true)) got, changed := runBody(t, pipeWith(t, "pipeline: [format]\n"), in, false) if changed || string(got) != string(in) { - t.Fatal("split fired without cacheinject configured") + t.Fatal("split fired with neither cachesplit nor cacheinject configured") + } +} + +// `cachesplit` alone enables the split. This is what keeps #32's preset change (dropping +// the unmeasured cacheinject) from silently disabling the measured split along with it. +func TestSplitEnabledByCachesplitAlone(t *testing.T) { + full, _, _ := blockWithGitTail(6000) + in := sysBody(textBlock(full, true)) + got, changed := runBody(t, pipeWith(t, "pipeline: [cachesplit]\n"), in, false) + if !changed { + t.Fatal("cachesplit did not enable the volatile-tail split") + } + if n := gjson.GetBytes(got, "system.#").Int(); n != 2 { + t.Fatalf("expected the system block to split into 2, got %d", n) + } + // cachesplit must add NO breakpoint of its own — it is not a placement policy. + if before, after := wireBreakpoints(in), wireBreakpoints(got); after != before { + t.Fatalf("cachesplit changed the breakpoint count %d -> %d", before, after) } } diff --git a/components/component.go b/components/component.go index ffe09f0..64b356c 100644 --- a/components/component.go +++ b/components/component.go @@ -129,7 +129,7 @@ type Ctx struct { // budget against this number rather than what it can see in Input, for two reasons // measured on real Claude Code traffic: 2 of its 3 breakpoints live in the // top-level `system` array, which components never see at all, and the third sits - // on a `tool_result` block whose cache_control bifrost drops on unmarshal. Both + // on a `tool_result` block whose mark the host's own normalize step drops. Both // were invisible, so the budget came out as 3 free slots when only 1 was free — // enough to put 6 on the wire and take a 400 (issue #32). The host fills it from // the raw body; 0 means "unknown, fall back to what you can see". diff --git a/components/pipeline.go b/components/pipeline.go index f227982..33dad3b 100644 --- a/components/pipeline.go +++ b/components/pipeline.go @@ -1,9 +1,8 @@ package components import ( - "bytes" - "encoding/json" "fmt" + "reflect" "github.com/maximhq/bifrost/core/schemas" "github.com/rossoctl/context-guru/schema" @@ -95,7 +94,6 @@ func (p *Pipeline) runOne(comp Component, req *schemas.BifrostChatRequest, c *Ct return rep } - rep.ChangedIdx = changedIdx(before, req.Input) after := tokensOf(req.Input) switch { case err != nil: @@ -119,6 +117,10 @@ func (p *Pipeline) runOne(comp Component, req *schemas.BifrostChatRequest, c *Ct rep.TokensAfter = rep.TokensBefore default: rep.TokensAfter = after + // Only a change that SURVIVED can be discarded by the writeback layer. Recording + // it in the revert branches above would charge a rolled-back component for a + // discard it never caused. + rep.ChangedIdx = changedIdx(before, req.Input) } return rep } @@ -129,18 +131,21 @@ func tokensOf(msgs []schemas.ChatMessage) int { // changedIdx returns the indices at which a component's output differs from its // input, so the writeback layer can attribute a discarded change to the component -// that made it. Compared on the canonical marshal, the same form the writeback loop -// uses to decide "changed". Count changes (summarize) yield nil — those go down the -// rebuild path, which never discards per-message. +// that made it. Count changes (summarize) yield nil — those go down the rebuild path, +// which never discards per-message. +// +// reflect.DeepEqual, not a marshal-and-compare: this runs per component per request +// purely for a diagnostic, so it must stay cheap. Measured on a realistic 80-message +// request, marshalling both sides cost 20.52 ms/op vs 16.56 ms (−19.3%) and 3,206 extra +// allocs. Struct equality is the same decision here — the writeback loop's own marshal +// is what actually decides whether to splice. func changedIdx(before, after []schemas.ChatMessage) []int { if len(before) != len(after) { return nil } var out []int for i := range after { - a, err1 := json.Marshal(before[i]) - b, err2 := json.Marshal(after[i]) - if err1 != nil || err2 != nil || !bytes.Equal(a, b) { + if !reflect.DeepEqual(before[i], after[i]) { out = append(out, i) } } @@ -151,19 +156,32 @@ func changedIdx(before, after []schemas.ChatMessage) []int { // writeback layer threw away, so a silently-suppressed component is visible in // telemetry instead of looking like a working one. discarded maps req.Input index -> // number of discarded changes at that index; hosts call this after the splice. +// +// One discarded message is charged to exactly ONE component: the LAST one that +// changed that index. Several components can touch the same message, but the +// writeback layer discards the final cumulative state, so that component's change is +// the one actually thrown away — charging every earlier toucher too would make this +// counter a false-positive generator, and its whole point is to be trustworthy enough +// to catch a #32-class bug. func (p *Pipeline) RecordDiscards(rr *RunReport, discarded map[int]int) { if p == nil || rr == nil || len(discarded) == 0 { return } - for _, rep := range rr.Components { - n := 0 - for _, i := range rep.ChangedIdx { - n += discarded[i] + // owner[i] = index into rr.Components of the last component that changed message i. + owner := map[int]int{} + for c := range rr.Components { + for _, i := range rr.Components[c].ChangedIdx { + owner[i] = c } - if n == 0 { - continue + } + counts := map[int]int{} // component index -> discards charged + for i, n := range discarded { + if c, ok := owner[i]; ok { + counts[c] += n } - d := Report{Component: rep.Component, Kind: rep.Kind, Discarded: n} + } + for c, n := range counts { + d := Report{Component: rr.Components[c].Component, Kind: rr.Components[c].Kind, Discarded: n} safeEmit(func() { p.emitter.Component(d) }) } } diff --git a/components/reformat/cacheinject.go b/components/reformat/cacheinject.go index ca8d05a..27b9862 100644 --- a/components/reformat/cacheinject.go +++ b/components/reformat/cacheinject.go @@ -146,7 +146,7 @@ func (ci Cacheinject) Reformat(req *schemas.BifrostChatRequest, rep *components. // happen with them: the positions we can SEE are dropped from `want` (never mark // twice), and the BUDGET is computed from the host's raw-body count, which also // sees the ones we cannot — the `system` array components never receive, and - // `tool_result` blocks whose cache_control bifrost drops on unmarshal. On real + // `tool_result` blocks whose mark apply's own normalize drops. On real // Claude Code traffic that is all 3 of them, so counting only Input gave a budget // of 3 free slots when 1 was free: 6 on the wire, and a 400 (issue #32). visible := 0 @@ -292,3 +292,35 @@ func cacheAware(p schemas.ModelProvider) bool { return false } } + +// --------------------------------------------------------------------------- // + +func init() { components.Register("cachesplit", newCachesplit) } + +// Cachesplit is a marker component: it carries no logic of its own, and exists so a +// preset can enable the volatile-tail split (apply/prefixsplit.go) WITHOUT also +// enabling cacheinject's breakpoint placement. +// +// The two were one config entry until #32, which separated them because their evidence +// is not comparable. The split is measured: −34.1% cost and 0% → 96.7% cache hit in an +// isolated A/B, because it moves a churning env snapshot out of a hashed prefix. +// Placement has never been measured at all — until #32 its breakpoints never reached +// the provider. So the split ships on by default and placement does not. +// +// It is a Reformat that always skips: the actual rewrite is body-level (it edits the +// top-level `system` array, which components never see) and lives in `apply`, gated on +// this name being present. ponytail: a marker beats plumbing a new config flag through +// every host. +type Cachesplit struct{} + +func newCachesplit([]byte) (components.Component, error) { return Cachesplit{}, nil } + +func (Cachesplit) Name() string { return "cachesplit" } + +func (Cachesplit) Enabled(*components.Ctx) bool { return true } + +// Reformat is intentionally a no-op — see the type doc. The split happens in apply. +func (Cachesplit) Reformat(_ *schemas.BifrostChatRequest, rep *components.Report, _ *components.Ctx) error { + rep.Skipped = true + return nil +} diff --git a/components/reformat/cacheinject_test.go b/components/reformat/cacheinject_test.go index 26fe713..108d2bb 100644 --- a/components/reformat/cacheinject_test.go +++ b/components/reformat/cacheinject_test.go @@ -283,8 +283,8 @@ func TestTTLConfig(t *testing.T) { } // The provider's cap counts breakpoints this component cannot see: `system` and -// `tools` never reach it, and bifrost drops cache_control on block types it does not -// model. Real Claude Code traffic carries (system=2, tools=0, messages=1), so the +// `tools` never reach it, and the host's normalize step drops the mark on tool_result +// blocks it rewrites. Real Claude Code traffic carries (system=2, tools=0, messages=1), so the // true remaining budget is 1, not 3 (issue #32, defect 2). The synthetic 60-message // shape below is exactly the probe that produced 4 message marks — 6 on the wire. func TestBudgetCountsInvisibleBreakpoints(t *testing.T) { diff --git a/config/config.go b/config/config.go index 11334ae..adf6a78 100644 --- a/config/config.go +++ b/config/config.go @@ -100,31 +100,31 @@ func (c *Config) applyPreset() error { // Build time as a clear error. var presets = map[string][]string{ "off": {}, // passthrough: no components (baseline / A-B control) - "safe": {"format", "cacheinject"}, - "balanced": {"format", "dedup", "failed_run", "cmdfilter", "cacheinject"}, - "aggressive": {"format", "dedup", "failed_run", "cmdfilter", "smartcrush", "extract", "extract_llm", "cacheinject"}, - "coding": {"format", "skeleton", "cmdfilter", "cacheinject"}, - "mcp": {"format", "smartcrush", "cacheinject"}, + "safe": {"format", "cachesplit"}, + "balanced": {"format", "dedup", "failed_run", "cmdfilter", "cachesplit"}, + "aggressive": {"format", "dedup", "failed_run", "cmdfilter", "smartcrush", "extract", "extract_llm", "cachesplit"}, + "coding": {"format", "skeleton", "cmdfilter", "cachesplit"}, + "mcp": {"format", "smartcrush", "cachesplit"}, // agent: tuned for long agentic sessions (e.g. Claude Code on SWE-bench), // where the dominant cost is the transcript of tool outputs (file reads) // re-sent every turn. mask (drop old tool outputs) is the biggest lever // there — ~27% content-token savings with no task-reward loss in the // eval-containers SWE-bench sweep (see docs/RESULTS.md); extract + failed_run - // + dedup add relevance/supersession/dup wins; cacheinject keeps the prefix - // cacheable. Order: lossless first, then offload old-then-large, cache last. - "agent": {"format", "dedup", "failed_run", "mask", "extract", "extract_llm", "cacheinject"}, + // + dedup add relevance/supersession/dup wins; cachesplit keeps the shared system + // prefix cacheable. Order: lossless first, then offload old-then-large, cache last. + "agent": {"format", "dedup", "failed_run", "mask", "extract", "extract_llm", "cachesplit"}, // general: the recommended all-round pipeline, safe+effective for any agent/ // benchmark. Ordered by pipeline semantics: lossless repack first (format, toon) // so downstream token counts are honest; cheap structural offloaders next (dedup, // failed_run, cmdfilter); age-based mask; relevance-based extract; the blind // head/tail collapse as the last-resort catch-all for anything still oversized; - // cacheinject last so the cache breakpoint sits on the final bytes. Every offloader + // cachesplit last (it edits `system`, not `messages`). Every offloader // defaults to marker_mode:full (reversible via the injected expand tool) and skips // content already carrying a placeholder, so they never double-reduce. Combines the // levers that proved reward-neutral in the benchmark sweeps without stacking the // two overlapping old-context reducers (mask is the one kept; summarize // is its own preset — see docs/components.md redundancy notes). - "general": {"format", "toon", "dedup", "failed_run", "cmdfilter", "mask", "extract", "extract_llm", "collapse", "cacheinject"}, + "general": {"format", "toon", "dedup", "failed_run", "cmdfilter", "mask", "extract", "extract_llm", "collapse", "cachesplit"}, // summarize restructures the whole transcript (changes the message count) — run // it alone so no other component's in-place edits race apply's rebuild. "summarize": {"summarize"}, @@ -132,8 +132,8 @@ var presets = map[string][]string{ // recommended defaults (codesmart is the proxy default). Their tuned per-component // settings live in presetConfigs; the name-lists here keep PresetPipeline (used by // /compact?preset=) resolving them. - "codesmart": {"format", "dedup", "failed_run", "cmdfilter", "extract_llm", "extract", "cacheinject"}, - "codesafe": {"format", "dedup", "failed_run", "cmdfilter", "extract", "collapse", "cacheinject"}, + "codesmart": {"format", "dedup", "failed_run", "cmdfilter", "extract_llm", "extract", "cachesplit"}, + "codesafe": {"format", "dedup", "failed_run", "cmdfilter", "extract", "collapse", "cachesplit"}, } // presetConfigs carries FULL config docs for presets whose behavior depends on tuned @@ -143,13 +143,13 @@ var presets = map[string][]string{ // relevance-trimmer extract_llm routed to the CHEAP model (model.source: config, // nil-when-unset ⇒ it silently no-ops to deterministic — see docs), gated at 3000 // tok so most turns make no model call, ≤4 calls/req; the free deterministic extract -// catches smaller noise; cacheinject keeps the prefix warm. +// catches smaller noise; cachesplit keeps the shared system prefix warm. // - codesafe: the deterministic-only variant (NO LLM, by policy) — same structural // offloaders plus a blind collapse fallback, zero model calls. // // Component defaults are left untouched, so general/agent/aggressive are unaffected. var presetConfigs = map[string]string{ - "codesmart": `pipeline: [format, dedup, failed_run, cmdfilter, extract_llm, extract, cacheinject] + "codesmart": `pipeline: [format, dedup, failed_run, cmdfilter, extract_llm, extract, cachesplit] components: extract: min_tokens: 400 @@ -162,7 +162,7 @@ components: min_request_tokens: 3000 llm_every_n_requests: 1 llm_max_per_request: 4`, - "codesafe": `pipeline: [format, dedup, failed_run, cmdfilter, extract, collapse, cacheinject] + "codesafe": `pipeline: [format, dedup, failed_run, cmdfilter, extract, collapse, cachesplit] components: collapse: max_tokens: 3000`, diff --git a/config/config_test.go b/config/config_test.go index 4b5d127..db70da3 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -46,7 +46,7 @@ func TestRichPresetCarriesComponentConfig(t *testing.T) { if err != nil { t.Fatal(err) } - want := []string{"format", "dedup", "failed_run", "cmdfilter", "extract_llm", "extract", "cacheinject"} + want := []string{"format", "dedup", "failed_run", "cmdfilter", "extract_llm", "extract", "cachesplit"} if strings.Join(c.Pipeline, ",") != strings.Join(want, ",") { t.Fatalf("codesmart pipeline = %v, want %v", c.Pipeline, want) } @@ -108,3 +108,32 @@ func TestStoreOptionsParse(t *testing.T) { t.Fatalf("store options not parsed: %+v", c.Store) } } + +// cacheinject is deliberately absent from every default preset (#32). Its breakpoints +// only started reaching the provider with that fix, and placement has never been shown +// to help — so it must not be on by default. `cachesplit` carries the volatile-tail +// split, which IS measured, so disabling placement does not disable the split with it. +func TestNoPresetEnablesCacheinjectByDefault(t *testing.T) { + for name := range presets { + p, _ := PresetPipeline(name) + for _, c := range p { + if c == "cacheinject" { + t.Errorf("preset %q enables cacheinject; placement is unmeasured and must be opt-in", name) + } + } + } + // The split must still be on in the recommended presets, or dropping cacheinject + // would be a cost regression rather than a neutral default change. + for _, name := range []string{"general", "codesmart", "codesafe", "balanced"} { + p, _ := PresetPipeline(name) + found := false + for _, c := range p { + if c == "cachesplit" { + found = true + } + } + if !found { + t.Errorf("preset %q lost the volatile-tail split", name) + } + } +} diff --git a/docs/components.md b/docs/components.md index 7867f7d..4ad0372 100644 --- a/docs/components.md +++ b/docs/components.md @@ -11,7 +11,8 @@ messages (`role:"tool"`; for Anthropic, `tool_result` blocks normalized to that |---|---|---|---|---|---| | `format` | Reformat | nothing (compacts JSON) | n/a (lossless) | pretty-printed JSON tool output | `min_tokens` (50) | | `toon` | Reformat | nothing (re-encodes JSON arrays as TOON) | n/a (lossless) | uniform flat JSON object-arrays | `min_tokens` (50) | -| `cacheinject` | Reformat | nothing (adds `cache_control`) | n/a (lossless) | Anthropic-family requests | — | +| `cacheinject` | Reformat | nothing (adds `cache_control`) | n/a (lossless) | Anthropic-family requests; **opt-in, in no preset** — placement is unmeasured (#32) | `ttl` (5m) | +| `cachesplit` | Reformat | nothing (splits a `system` block) | n/a (lossless) | Anthropic-family requests; **in the default presets** — enables the measured volatile-tail split | — | | `skeleton` | Offload | function/method bodies | via expand | fenced ` ```lang ` code blocks | `min_tokens` (80) | | `dedup` | Offload | later byte-identical tool outputs | via expand | repeated identical outputs | `min_tokens` (100) | | `collapse` | Offload | middle of an oversized output | via expand | any large tool output (fallback) | `max_tokens` (2000), `head_lines` (20), `tail_lines` (20) | @@ -23,12 +24,12 @@ messages (`role:"tool"`; for Anthropic, `tool_result` blocks normalized to that | `mask` | Offload | older tool outputs (age-based) | via expand | more than `keep_recent` outputs | `keep_recent` (3), `min_tokens` (100), `keep_head_chars` (96) | | `summarize` | Offload (LLM) | the middle of the transcript → one summary | via expand | long trajectories | `summary_level` (regular), `keep_last` (3), `min_tokens` (500), `resummarize_tokens` (6000), `model.source`, `trigger` | -Presets (`config`): `off` `[]` · `safe` `[format, cacheinject]` · `balanced` -`[format, dedup, failed_run, cmdfilter, cacheinject]` · `aggressive` adds `smartcrush, extract` · -`coding` `[format, skeleton, cmdfilter, cacheinject]` · `mcp` `[format, smartcrush, cacheinject]` · -**`agent`** `[format, dedup, failed_run, mask, extract, cacheinject]` — for long agentic sessions; +Presets (`config`): `off` `[]` · `safe` `[format, cachesplit]` · `balanced` +`[format, dedup, failed_run, cmdfilter, cachesplit]` · `aggressive` adds `smartcrush, extract` · +`coding` `[format, skeleton, cmdfilter, cachesplit]` · `mcp` `[format, smartcrush, cachesplit]` · +**`agent`** `[format, dedup, failed_run, mask, extract, cachesplit]` — for long agentic sessions; `mask` is the biggest lever there (~27–30% content-token savings, no reward loss — see [RESULTS.md](RESULTS.md)) · -**`general`** `[format, toon, dedup, failed_run, cmdfilter, mask, extract, collapse, cacheinject]` — the +**`general`** `[format, toon, dedup, failed_run, cmdfilter, mask, extract, collapse, cachesplit]` — the recommended all-round pipeline: the reward-neutral levers of `agent` plus the situational shrinkers (`toon`/`cmdfilter`/`collapse`) that cost nothing when they don't fire. `balanced` is **not** recommended for agentic traffic — it omits `mask`, so it barely helps (6% vs 31% in the Terminal-Bench replay) · @@ -96,9 +97,14 @@ after: [2]{id,name}: output, or not smaller. ### `cacheinject` -Places an Anthropic `cache_control: {type: ephemeral}` breakpoint on the last content block of -the message just **before** the newest turn (a stable prefix boundary), so the provider KV cache -hits across turns. Adds a control directive, changes no model-visible content. +Places Anthropic `cache_control: {type: ephemeral}` breakpoints at the positions that minimise +billed input cost, so the provider KV cache is read rather than re-processed. Adds control +directives, changes no model-visible content. + +**In no preset — opt in explicitly.** Until [#32](https://github.com/rossoctl/context-guru/issues/32) +its breakpoints never reached the provider on Claude Code traffic, so the placement policy has +never been measured. The presets carry `cachesplit` instead, which enables the volatile-tail split +(measured) without the placement (not). - **Lossiness:** none. **Shines:** Anthropic/Bedrock/Vertex agents that don't self-cache (the savings lever is provider-side cache hits, invisible to `/stats` token counts). **Inert:** diff --git a/docs/components/cacheinject.md b/docs/components/cacheinject.md index f20085f..a0cc616 100644 --- a/docs/components/cacheinject.md +++ b/docs/components/cacheinject.md @@ -18,10 +18,13 @@ A second, independent defect compounded it: the 4-breakpoint budget counted only `messages`. Real traffic puts 2 of its 3 breakpoints in the top-level `system` array - and the third on a `tool_result` block whose `cache_control` bifrost drops — all - three invisible. The component computed 3 free slots when 1 was free. On a - 60-message request it emitted 4 message marks, **6 on the wire**, which the provider - rejects with a 400. That never fired in production *only* because the first defect + and the third on a `tool_result` block — and that third mark is lost by **this + repo's own** `normalize`, which rebuilds each `tool_result` into a synthetic + `role=tool` message from text + `tool_use_id` alone (`toolMessage`), dropping + `cache_control` along with everything else it does not copy. So all three were + invisible and the component computed 3 free slots when 1 was free. On a 60-message + request it emitted 4 message marks, **6 on the wire**, which the provider rejects + with a 400. That never fired in production *only* because the first defect suppressed the marks; fixing one without the other would have produced a live 400. Both are fixed (see [design.md](../design.md) — the metadata-write exception). @@ -130,7 +133,41 @@ configured but are a different mechanism. See below. ## What placement is actually worth -Placeholder — filled by the `cacheonly` vs `off` measurement in #32. +**Still unmeasured.** #32 made the policy reach the provider for the first time; it did +not establish what the policy is worth. One `cacheonly` vs `off` pair was attempted and +is reported here in full because it is all the evidence there is — not because it settles +anything. + +SWE-bench Verified, `aws/claude-sonnet-5`, n=1 per arm. `off`'s second trial died on a +Docker-compose error, so only `astropy-12907` completed in both arms: + +| metric | off | cacheonly | delta | +|---|--:|--:|--:| +| steps | 14 | 11 | −21.4% | +| billed cost | $0.17607 | $0.14931 | −15.2% | +| cache-read | 609,968 | 461,844 | −24.3% | +| cache-write | 8,695 | 11,062 | +27.2% | +| cache-hit | 98.59% | 96.66% | −1.93 pp | + +The −15.2% is **not a saving** — the agent took 3 fewer steps, and on this traffic cost +tracks steps at corr 0.95. Per step, cost is +7.9% and cache-write +61.9%. + +!!! warning "What this does NOT show" + **No mechanism is established for the cache-write difference, and the obvious one is + ruled out.** The tempting story — that the extra breakpoint lands *above* + claude-code's own and shortens the readable prefix — does not hold: across three + captures, **0 of 106** of our marks land above the agent's. Ours consistently sits one + message *below*, where the policy's own Rule 2 says an extra breakpoint costs exactly + zero. + + **`acted=0` does not isolate placement.** It only rules out content compaction. The + `cacheonly` arm still runs `splitVolatileTail`, which rewrites the `system` array, so + the arm is "placement + split", not "placement". A single trial per arm also cannot + separate either from the step-count nondeterminism that produced the 3-step gap. + + Treat the table as one task, once, on a contended box, with a degenerate control. The + honest summary is that placement's value **has still never been measured** — which is + why `cacheinject` is not in any default preset. ## How the breakpoints reach the wire @@ -154,20 +191,22 @@ that let this survive unnoticed. ## The 4-breakpoint budget is computed by the host, not the component The provider caps `cache_control` at **4 across `system` + `tools` + `messages` -together**, and a component sees none of the first two. Worse, bifrost drops -`cache_control` on block types it does not model, so even some *message* breakpoints are -invisible to it. +together**, and a component sees none of the first two. Worse, `apply`'s own `normalize` +rebuilds each `tool_result` block into a synthetic `role=tool` message from text + +`tool_use_id` alone (`toolMessage`), so a `cache_control` on such a block never reaches +the component either — even some *message* breakpoints are invisible to it. On real Claude Code traffic that hides **all three** of the agent's own breakpoints — measured, 1,771 of 1,794 requests carry exactly `(system=2, tools=0, messages=1)`, with the message one sitting on a `tool_result` block. Counting only what it could see, the -component computed `budget = 4 − 1 = 3` when 1 slot was free, and on a 60-message +component computed `budget = 4 − 0 = 4` when 1 slot was free, and on a 60-message request emitted 4 marks: **6 on the wire, which the provider rejects with a 400.** So `apply` counts them structurally from the raw body and passes the total as -`Ctx.ExistingBreakpoints`; the component budgets against that. `apply` also counts the -output and logs an error if it ever exceeds 4, so a breach shows up in telemetry rather -than as a provider 400. +`Ctx.ExistingBreakpoints`; the component budgets against that. The count covers the +Bedrock `cachePoint` spelling too, including its own entries in `system` and `tools`. +`apply` also counts the output and logs an error if **we** pushed it over 4 — a request +that arrived over the cap is forwarded as-is and is not reported as ours. ## Lossiness @@ -181,6 +220,19 @@ markable block below so the prefix is still written. ## Configuration +!!! warning "Not enabled by any preset — opt in explicitly" + Since #32, `cacheinject` is in **no** default preset. Its breakpoints only began + reaching the provider with that fix, and placement has never been shown to help, so + shipping it on by default would enable an unmeasured policy on every request. + + The presets carry **`cachesplit`** instead — a marker component that enables the + [volatile-tail split](#the-volatile-tail-split) (which *is* measured) without the + breakpoint placement. The two were one config entry until #32; separating them is + what keeps disabling placement from silently disabling the split too. + + Add `cacheinject` to a pipeline by hand to run the placement study, or when your agent + does not set its own `cache_control` (the case where the policy should help most). + ```yaml components: cacheinject: @@ -208,9 +260,14 @@ it believed it had three. ## The volatile-tail split -Enabling `cacheinject` also switches on a body-level repair in `apply/prefixsplit.go` -that no breakpoint placement can achieve, because a cache entry hashes **everything -before** its breakpoint and no position can exclude part of a single block. +Enabling **`cachesplit`** (or `cacheinject`) switches on a body-level repair in +`apply/prefixsplit.go` that no breakpoint placement can achieve, because a cache entry +hashes **everything before** its breakpoint and no position can exclude part of a single +block. + +This is the mechanism the default presets keep. It is a different lever from placement +and has its own, much stronger evidence — everything below was measured with placement +contributing `$0`, so none of it depends on the policy this page's other half describes. Claude Code appends a live environment snapshot to the **end** of its main system block: diff --git a/docs/design.md b/docs/design.md index 73c2e09..3eaaf67 100644 --- a/docs/design.md +++ b/docs/design.md @@ -144,14 +144,21 @@ breakpoint the caller set. the component that made it (via `Report.ChangedIdx`), surfacing as `discarded_changes` per component and `top_discarded` in `/stats`. Before this, a mutated-then-discarded component looked byte-identical to a working Reformat — which is how #32 survived two full benchmark studies. +Attribution is deliberately conservative, because a counter meant to catch that class of bug is +worthless if it cries wolf: `ChangedIdx` is recorded only on the surviving path (a reverted +component is never charged), and one discarded message is charged to exactly ONE component — the +last one to change it, whose state is what the writeback layer actually threw away. **Breakpoint budgeting is a host job.** The provider caps `cache_control` at 4 across `system` + -`tools` + `messages` together, and a component sees none of the first two — nor cache_control on -blocks bifrost drops. On real Claude Code traffic that hides all three of the agent's own -breakpoints, so a component counting only what it saw computed 3 free slots when 1 was free. -`apply` counts them from the raw body (`wireBreakpoints`) and passes the total as -`Ctx.ExistingBreakpoints`; exceeding the cap on output logs an error rather than waiting for the -provider's 400. +`tools` + `messages` together, and a component sees none of the first two. Nor does it see a +`cache_control` on a `tool_result` block: `normalize` rebuilds those into synthetic `role=tool` +messages from text + `tool_use_id` alone (`toolMessage`), dropping the mark. (bifrost is not the +culprit here — it round-trips `cache_control` on `tool_result` fine.) On real Claude Code traffic +that hides all three of the agent's own breakpoints, so a component counting only what it saw +computed 4 free slots when 1 was free. `apply` counts them from the raw body (`wireBreakpoints`, +covering the Bedrock `cachePoint` spelling and its own `system`/`tools` entries) and passes the +total as `Ctx.ExistingBreakpoints`. A breach is logged only when *we* pushed the total over the +cap — an already-over-cap request is forwarded untouched and is not ours to report. If a component changes the message *count* (none of the v1 set does), the slot map no longer aligns, so `apply` forwards the original untouched. diff --git a/docs/how-to/choose-a-preset.md b/docs/how-to/choose-a-preset.md index 3c03de0..937ccbb 100644 --- a/docs/how-to/choose-a-preset.md +++ b/docs/how-to/choose-a-preset.md @@ -32,16 +32,19 @@ per-component behavior is in [Components](../components.md). No components. Passthrough. Use it as the A/B control when you measure savings — the baseline in [Benchmarks](../RESULTS.md) is this preset. -### `safe` — `[format, cacheinject]` +### `safe` — `[format, cachesplit]` Two lossless [Reformat](../components.md#reformat-lossless) components only: compact JSON -(`format`) and an Anthropic cache breakpoint (`cacheinject`). Nothing is ever dropped, so there is -nothing to expand. +(`format`) and the Anthropic volatile-tail split (`cachesplit`). Nothing is ever dropped, so there +is nothing to expand. - **Fits:** any traffic where you want a zero-risk win and no reversibility surface. -- **Caveat:** `cacheinject`'s savings are provider-side cache hits, invisible to `/stats` token +- **Caveat:** `cachesplit`'s savings are provider-side cache hits, invisible to `/stats` token counts — it will show up under `top_passthrough`. That's expected, not dead weight. +- **Note:** breakpoint *placement* (`cacheinject`) is deliberately **not** here — it is unmeasured + and opt-in since [#32](https://github.com/rossoctl/context-guru/issues/32). `cachesplit` carries + the part with measured savings. -### `balanced` — `[format, dedup, failed_run, cmdfilter, cacheinject]` +### `balanced` — `[format, dedup, failed_run, cmdfilter, cachesplit]` The default. Adds three cheap, high-precision offloaders: exact-dup removal (`dedup`), superseded test/build runs (`failed_run`), and DSL command-log filtering (`cmdfilter`). @@ -50,7 +53,7 @@ test/build runs (`failed_run`), and DSL command-log filtering (`cmdfilter`). one. Its builtins cover pytest / npm-install / make; author more with a [custom DSL filter](custom-dsl-filter.md). -### `aggressive` — `[format, dedup, failed_run, cmdfilter, smartcrush, extract, cacheinject]` +### `aggressive` — `[format, dedup, failed_run, cmdfilter, smartcrush, extract, cachesplit]` `balanced` plus JSON-array crushing (`smartcrush`) and query-relevance projection (`extract`). - **Fits:** you want more savings and accept structural/LLM offload with expand recovery. @@ -58,7 +61,7 @@ test/build runs (`failed_run`), and DSL command-log filtering (`cmdfilter`). the default `deterministic` strategy is free. Keep the [store](recover-context.md) on so the extra offloads stay recoverable. -### `coding` — `[format, skeleton, cmdfilter, cacheinject]` +### `coding` — `[format, skeleton, cmdfilter, cachesplit]` Swaps in `skeleton`, which tree-sitter-parses fenced code blocks and replaces function bodies with `{ … }`, keeping signatures/imports/types. @@ -66,14 +69,14 @@ Swaps in `skeleton`, which tree-sitter-parses fenced code blocks and replaces fu - **Caveat:** `skeleton` is inert on unfenced file reads, unknown languages, or when the skeleton isn't smaller than the body. -### `mcp` — `[format, smartcrush, cacheinject]` +### `mcp` — `[format, smartcrush, cachesplit]` Targets homogeneous JSON arrays (list endpoints, search hits): keep `keep_first` + `keep_last` items plus any item carrying an error signal, drop the middle. - **Fits:** MCP tools and REST list endpoints returning long uniform arrays. - **Caveat:** inert on non-array output or arrays below `min_items`. -### `agent` — `[format, dedup, failed_run, mask, extract, cacheinject]` +### `agent` — `[format, dedup, failed_run, mask, extract, cachesplit]` Tuned for long agentic sessions (e.g. Claude Code on SWE-bench) where the dominant cost is the transcript of old tool outputs re-sent every turn.