From d716bec11ff1cbd878187475ffb4801578112e79 Mon Sep 17 00:00:00 2001 From: Osher-Elhadad Date: Mon, 10 Aug 2026 03:03:28 +0000 Subject: [PATCH 1/5] perf(extract_llm): gate LLM calls on expected value, cache results globally MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit extract_llm is the only component that spends money to save money, and on Terminal-Bench it lost: 271 calls, $3.26, ~1,592s of added latency against ~197,548 unique tokens saved — ~8x underwater once those tokens are priced at the rate they would actually have been billed. ~93% of its realized value came from the replay cache, not from the LLM. The cause is arithmetic. A request to a caching backend is ~99.95% cached, so a token removed from a cached region saves the cache-read rate ($0.30/MTok), not the fresh-input rate ($3/MTok) — a 10x haircut. Break-even for one ~$0.012 call is therefore ~12,700 tokens of output under caching versus ~1,270 without it, and most tool outputs are nowhere near that. The component already carried this insight as a comment on skip_file_reads; this turns it into an actual gate that applies to every candidate. Economic gate (on by default): call the LLM only when expected_saving = removed x (1 + expected replays) x per-token value exceeds the observed mean cost of a call. The per-token value comes from Ctx.CacheAware, the compression ratio is learned from accepted results (repeated misses drive it to zero and shut the gate), and the call cost is computed from real token usage times real model pricing (CHEAP_MODEL_PRICE_*), never a hard-coded constant. Every decision records a reason. Global result cache: an extraction is a context-free derived result, so re-key it on sha256(content + prompt version + model + config fingerprint) with no session prefix — 82 of 103 unique contents recurred across sessions, and the old key threw that reuse away. A version, model, or config change misses rather than serving a stale extraction. Old entries are inert once; a session-scoped entry is still honored as a migration read. (Contrast #27's xdedup index, session-scoped on purpose: that makes a conversational reference, this caches a derived result.) Derived triggering: no per-workload threshold. Context pressure plus growth rate replace min_tokens, which stays honored when set explicitly. A merely growing context no longer fires on every step. Prompt-cache the preamble, with the measurement that matters: the ~1,463-token invariant contract now goes in a stable system block with cache_control (a leading system message on OpenAI, which has no explicit breakpoints). Measured against the gateway, this is INERT on claude-haiku-4-5, whose minimum cacheable prefix is 4096 tokens — a sub-minimum breakpoint is silently ignored, write=0 read=0. It caches on claude-sonnet-5 (minimum 1024). Shipped because it is free and correct where it wins, but /stats exposes prompt_cache_read_tokens so nobody infers a cache win from placement. Reusing the agent's cached prefix (part B) prototyped and REJECTED, with numbers: it works mechanically (a 103,019-token prefix read from cache, no write, no invalidation), but costs $0.034 at 103k, $0.153 at 500k and $0.513 at 1.7M tokens — 8.5x to 128x a dedicated cheap-model call, and ~$2.04/turn at 4 concurrent calls. It also risks a cache-write on the agent's own prefix (11.5x a read) and couples the compaction model to the agent model. Metrics: /stats gains an extract block with calls, calls avoided by cache, calls suppressed by the gate, prompt-cache behavior, cost, gross value and NET value after cost — the honest headline, previously impossible to see because cost was reported in a field separate from savings. Purely additive; every existing key keeps its name so deploy/harbor/*.py keeps parsing. Tests cover the cached-system-block shape on both backends, cross-session cache reuse, a version bump missing rather than serving stale, gate suppression when cache-aware, permission on a non-caching backend and for recurring content, the cost model against known tokens times known price, the documented break-even sizes, and that the trigger does not fire every step on a growing context. Refs #28 Signed-off-by: Osher-Elhadad --- components/all/freeze_repair_test.go | 7 +- components/all/llm_test.go | 18 +- components/all/reuse_test.go | 6 +- components/all/xglobal_test.go | 194 +++++++++++++++ components/offload/extract_econ.go | 299 ++++++++++++++++++++++++ components/offload/extract_econ_test.go | 211 +++++++++++++++++ components/offload/extract_llm.go | 157 ++++++++++++- components/offload/state.go | 66 ++++++ docs/components/extract_llm.md | 219 +++++++++++++++-- docs/reference/config.md | 16 ++ docs/reference/presets.md | 17 ++ internal/cheapmodel/anthropic.go | 38 ++- internal/cheapmodel/openai.go | 29 ++- internal/cheapmodel/system_test.go | 173 ++++++++++++++ internal/cheapmodel/usage.go | 93 +++++++- internal/extract/extract.go | 78 +++++++ internal/extract/prompt.go | 72 ++++++ internal/extract/resultkey_test.go | 133 +++++++++++ internal/extract/starlark.go | 6 +- metrics/extract.go | 159 +++++++++++++ metrics/extract_test.go | 142 +++++++++++ metrics/metrics.go | 5 + proxy/proxy.go | 27 +++ 23 files changed, 2123 insertions(+), 42 deletions(-) create mode 100644 components/all/xglobal_test.go create mode 100644 components/offload/extract_econ.go create mode 100644 components/offload/extract_econ_test.go create mode 100644 internal/cheapmodel/system_test.go create mode 100644 internal/extract/resultkey_test.go create mode 100644 metrics/extract.go create mode 100644 metrics/extract_test.go diff --git a/components/all/freeze_repair_test.go b/components/all/freeze_repair_test.go index 5831a93..b4ae9d6 100644 --- a/components/all/freeze_repair_test.go +++ b/components/all/freeze_repair_test.go @@ -43,8 +43,13 @@ func (m *varyingModel) calls() int { // Only mask/failed_run get the depth repair, because their replacement is a pure function // of (content, config) and so is genuinely reproducible. func TestLostExtractLLMResultIsNotReDerivedAtDepth(t *testing.T) { + // allow_on_caching_backend + economic_gate: false keep this fixture exercising the + // CACHE-SAFETY path under test. extract_llm is off by default on caching backends and + // its gate declines small outputs, so without these turn 1 never compacts and the guard + // passes vacuously — which is the same trap #40 hit with the wrong MaxCachedIdx. off := newComp(t, "extract_llm", - "strategy: code\nmin_tokens: 1\nmodel:\n source: config\ntrigger:\n min_request_tokens: 1\n") + "strategy: code\nmin_tokens: 1\nallow_on_caching_backend: true\neconomic_gate: false\n"+ + "model:\n source: config\ntrigger:\n min_request_tokens: 1\n") now := time.Unix(0, 0) st := store.NewMemory(store.Options{TTLSeconds: 10}) st.SetClock(func() time.Time { return now }) diff --git a/components/all/llm_test.go b/components/all/llm_test.go index a043be0..22f9d2c 100644 --- a/components/all/llm_test.go +++ b/components/all/llm_test.go @@ -135,7 +135,11 @@ func TestSummarizeEmptyResponseSkips(t *testing.T) { // TestExtractRLMUsesModel: strategy=rlm currently maps to code and still runs the // model's filter (not silently deterministic). func TestExtractRLMUsesModel(t *testing.T) { - off := newComp(t, "extract_llm", "strategy: rlm\nmin_tokens: 1\nmodel:\n source: config\n") + // economic_gate: false — this is a MECHANISM test (does the model-written filter + // run and reduce?), and its small fixture output is genuinely uneconomic, so the + // #28 gate would correctly suppress the call. Gate economics are tested in + // components/offload/extract_econ_test.go against the dollar figures directly. + off := newComp(t, "extract_llm", "strategy: rlm\nmin_tokens: 1\neconomic_gate: false\nmodel:\n source: config\n") st := store.NewMemory(store.Options{}) pad := strings.Repeat("padding ", 40) // so reduction beats the marker cost (D1 guard) body := `[{"id":1,"name":"keep this one ` + pad + `"},{"id":2,"name":"drop it ` + pad + `"}]` @@ -157,7 +161,11 @@ func TestExtractRLMUsesModel(t *testing.T) { // TestExtractCodeUsesModel: the code strategy runs the model's Starlark filter and // keeps only the matching records (a contained subset), with a marker. func TestExtractCodeUsesModel(t *testing.T) { - off := newComp(t, "extract_llm", "strategy: code\nmin_tokens: 1\nmodel:\n source: config\n") + // economic_gate: false — this is a MECHANISM test (does the model-written filter + // run and reduce?), and its small fixture output is genuinely uneconomic, so the + // #28 gate would correctly suppress the call. Gate economics are tested in + // components/offload/extract_econ_test.go against the dollar figures directly. + off := newComp(t, "extract_llm", "strategy: code\nmin_tokens: 1\neconomic_gate: false\nmodel:\n source: config\n") st := store.NewMemory(store.Options{}) pad := strings.Repeat("padding ", 40) // so reduction beats the marker cost (D1 guard) body := `[{"id":1,"name":"keep this ` + pad + `"},{"id":2,"name":"drop this ` + pad + `"},{"id":3,"name":"keep that ` + pad + `"}]` @@ -215,7 +223,11 @@ func TestDeterministicExtractCollapsesRepeats(t *testing.T) { // extract_llm with no model available is a clean no-op (deterministic collapse is a // separate component now — extract_llm never silently falls back to it). func TestExtractLLMNilModelSkips(t *testing.T) { - off := newComp(t, "extract_llm", "strategy: code\nmin_tokens: 1\nmodel:\n source: config\n") + // economic_gate: false — this is a MECHANISM test (does the model-written filter + // run and reduce?), and its small fixture output is genuinely uneconomic, so the + // #28 gate would correctly suppress the call. Gate economics are tested in + // components/offload/extract_econ_test.go against the dollar figures directly. + off := newComp(t, "extract_llm", "strategy: code\nmin_tokens: 1\neconomic_gate: false\nmodel:\n source: config\n") st := store.NewMemory(store.Options{}) body := strings.Repeat("some log line\n", 40) req := &bschemas.BifrostChatRequest{Input: []bschemas.ChatMessage{ diff --git a/components/all/reuse_test.go b/components/all/reuse_test.go index 75ee5fa..aac6834 100644 --- a/components/all/reuse_test.go +++ b/components/all/reuse_test.go @@ -78,7 +78,11 @@ func TestSummarizeReusesCheckpoint(t *testing.T) { // TestExtractReusesResultCache: the same large tool output re-sent on a later turn // reuses the prior compaction — no second model call — and is still reduced. func TestExtractReusesResultCache(t *testing.T) { - off := newComp(t, "extract_llm", "strategy: code\nmin_tokens: 1\nmodel:\n source: config\n") + // economic_gate: false — this is a MECHANISM test (does the model-written filter + // run and reduce?), and its small fixture output is genuinely uneconomic, so the + // #28 gate would correctly suppress the call. Gate economics are tested in + // components/offload/extract_econ_test.go against the dollar figures directly. + off := newComp(t, "extract_llm", "strategy: code\nmin_tokens: 1\neconomic_gate: false\nmodel:\n source: config\n") st := store.NewMemory(store.Options{}) filter := "data = json.decode(INPUT)\nOUTPUT = json.encode([r for r in data if \"keep\" in r[\"name\"]])\n" cm := &countingModel{resp: filter} diff --git a/components/all/xglobal_test.go b/components/all/xglobal_test.go new file mode 100644 index 0000000..c63b93e --- /dev/null +++ b/components/all/xglobal_test.go @@ -0,0 +1,194 @@ +package all_test + +import ( + "context" + "strings" + "testing" + + bschemas "github.com/maximhq/bifrost/core/schemas" + "github.com/rossoctl/context-guru/components" + "github.com/rossoctl/context-guru/schema" + "github.com/rossoctl/context-guru/store" +) + +// TestExtractResultCacheHitsAcrossSessions is the headline acceptance criterion for issue +// #28 part C: identical content in a DIFFERENT session must reuse the prior extraction +// instead of paying for it again. Before the global re-key the result cache carried a +// session prefix, so the second session re-derived a result the system already had — +// measured wasteful on 82 of 103 unique contents. +func TestExtractResultCacheHitsAcrossSessions(t *testing.T) { + // economic_gate: false isolates the CACHE behavior under test from the gate's + // (separately tested) spending decision. + off := newComp(t, "extract_llm", "strategy: code\nmin_tokens: 1\neconomic_gate: false\nmodel:\n source: config\n") + st := store.NewMemory(store.Options{}) // one store, as a real proxy has + filter := "data = json.decode(INPUT)\nOUTPUT = json.encode([r for r in data if \"keep\" in r[\"name\"]])\n" + cm := &countingModel{resp: filter} + pad := strings.Repeat("padding ", 40) + body := `[{"id":1,"name":"keep this ` + pad + `"},{"id":2,"name":"drop this ` + pad + `"}]` + + runIn := func(session string) *bschemas.BifrostChatRequest { + req := &bschemas.BifrostChatRequest{Input: []bschemas.ChatMessage{ + userMsg("find the keep records"), toolMsg(body), + }} + c := &components.Ctx{Ctx: context.Background(), Session: session, Store: st, + Model: components.ModelSpec{Static: cm}} + var rep components.Report + if _, err := off.Offload(req, &rep, c); err != nil { + t.Fatal(err) + } + return req + } + + req1 := runIn("session-A") + if cm.calls != 1 { + t.Fatalf("first session must call the model once, calls=%d", cm.calls) + } + out1 := schema.MessageText(req1.Input[1]) + if strings.Contains(out1, "drop this") { + t.Fatal("first session should have reduced the output") + } + + // A DIFFERENT session, same content. This is the case the session-scoped key missed. + req2 := runIn("session-B-completely-different") + if cm.calls != 1 { + t.Fatalf("a different session must REUSE the cached extraction (no new model call), calls=%d", cm.calls) + } + out2 := schema.MessageText(req2.Input[1]) + if strings.Contains(out2, "drop this") { + t.Fatal("cross-session reuse must still drop the non-keep record") + } + + // A third, also free. + runIn("session-C") + if cm.calls != 1 { + t.Fatalf("every later session must reuse, calls=%d", cm.calls) + } +} + +// The gate must actually suppress in a real pipeline run on a cache-aware request with a +// small output — the Terminal-Bench losing case, end to end rather than in unit isolation. +func TestExtractGateSuppressesInPipelineWhenCacheAware(t *testing.T) { + off := newComp(t, "extract_llm", "strategy: code\nmodel:\n source: config\n") + st := store.NewMemory(store.Options{}) + filter := "data = json.decode(INPUT)\nOUTPUT = json.encode([r for r in data if \"keep\" in r[\"name\"]])\n" + cm := &countingModel{resp: filter} + pad := strings.Repeat("padding ", 40) // ~400 tokens: far below the ~12.7k cached break-even + body := `[{"id":1,"name":"keep this ` + pad + `"},{"id":2,"name":"drop this ` + pad + `"}]` + + req := &bschemas.BifrostChatRequest{Input: []bschemas.ChatMessage{ + userMsg("find the keep records"), toolMsg(body), + }} + c := &components.Ctx{Ctx: context.Background(), Session: "s1", Store: st, + Model: components.ModelSpec{Static: cm}, CacheAware: true, MaxCachedIdx: -1, + CtxWindow: 1_000_000} + var rep components.Report + if _, err := off.Offload(req, &rep, c); err != nil { + t.Fatal(err) + } + if cm.calls != 0 { + t.Fatalf("a small output on a cache-aware request must not be worth a call, calls=%d", cm.calls) + } + if schema.MessageText(req.Input[1]) != body { + t.Fatal("a suppressed candidate must be left verbatim (fail open)") + } +} + +// Backward compatibility: an existing config that pins min_tokens must keep working +// unchanged — the smarter trigger is the DEFAULT only when nothing was configured. +func TestExplicitMinTokensConfigStillReduces(t *testing.T) { + // A pinned min_tokens plus the pre-#28 gate setting reproduces old behavior exactly. + off := newComp(t, "extract_llm", "strategy: code\nmin_tokens: 1\neconomic_gate: false\nmodel:\n source: config\n") + st := store.NewMemory(store.Options{}) + filter := "data = json.decode(INPUT)\nOUTPUT = json.encode([r for r in data if \"keep\" in r[\"name\"]])\n" + cm := &countingModel{resp: filter} + pad := strings.Repeat("padding ", 40) + body := `[{"id":1,"name":"keep this ` + pad + `"},{"id":2,"name":"drop this ` + pad + `"}]` + req := &bschemas.BifrostChatRequest{Input: []bschemas.ChatMessage{ + userMsg("find the keep records"), toolMsg(body), + }} + // A tiny context window would make the derived trigger decline; an explicit + // min_tokens must override that. + c := &components.Ctx{Ctx: context.Background(), Session: "s1", Store: st, + Model: components.ModelSpec{Static: cm}, CtxWindow: 1_000_000} + var rep components.Report + keys, err := off.Offload(req, &rep, c) + if err != nil { + t.Fatal(err) + } + if len(keys) != 1 { + t.Fatalf("explicit min_tokens must still reduce (skipped=%v calls=%d)", rep.Skipped, cm.calls) + } +} + +// A CROSS-SESSION global hit must not be spliced into the provider's cached prefix. +// The same-session replay path is safe at any depth because this session already sent +// those compacted bytes — the cached prefix holds the COMPACTED form. A global hit is +// different: THIS session never compacted this message, so the cached prefix holds the +// ORIGINAL, and splicing at depth mutates already-cached content and forces a suffix +// re-write at 11.5x the read price. Same churn #40's tail gate exists to prevent, just +// sourced from a cache rather than a model call. +func TestGlobalCacheHitIsNotSplicedAtDepth(t *testing.T) { + cfg := "strategy: code\nmin_tokens: 1\nallow_on_caching_backend: true\neconomic_gate: false\n" + + "model:\n source: config\ntrigger:\n min_request_tokens: 1\n" + st := store.NewMemory(store.Options{}) + filter := "data = json.decode(INPUT)\nOUTPUT = json.encode([r for r in data if \"keep\" in r[\"name\"]])\n" + pad := strings.Repeat("padding ", 40) + body := `[{"id":1,"name":"keep this ` + pad + `"},{"id":2,"name":"drop this ` + pad + `"}]` + + // Session A populates the GLOBAL cache from its own tail. + offA := newComp(t, "extract_llm", cfg) + cmA := &countingModel{resp: filter} + reqA := &bschemas.BifrostChatRequest{Input: []bschemas.ChatMessage{ + userMsg("find the keep records"), toolMsg(body), + }} + var repA components.Report + if _, err := offA.Offload(reqA, &repA, &components.Ctx{Ctx: context.Background(), + Session: "A", Store: st, Model: components.ModelSpec{Static: cmA}, + CacheAware: true, MaxCachedIdx: -1, CtxWindow: 200_000}); err != nil { + t.Fatal(err) + } + if schema.MessageText(reqA.Input[1]) == body { + t.Fatal("session A must compact (fixture no longer exercises the path)") + } + + // Session B sees the SAME content, but at DEPTH (MaxCachedIdx >= 1). B never compacted + // it, so B's provider prefix holds the ORIGINAL — the global hit must NOT be applied. + offB := newComp(t, "extract_llm", cfg) + cmB := &countingModel{resp: filter} + reqB := &bschemas.BifrostChatRequest{Input: []bschemas.ChatMessage{ + userMsg("find the keep records"), toolMsg(body), toolMsg("tail"), + }} + var repB components.Report + if _, err := offB.Offload(reqB, &repB, &components.Ctx{Ctx: context.Background(), + Session: "B", Store: st, Model: components.ModelSpec{Static: cmB}, + CacheAware: true, MaxCachedIdx: 1, CtxWindow: 200_000}); err != nil { + t.Fatal(err) + } + if got := schema.MessageText(reqB.Input[1]); got != body { + t.Fatalf("a cross-session global hit must leave a cached-prefix message VERBATIM:\n got=%q", got) + } + // The depth message must not have been re-derived either: a global miss falls through to + // the normal tail-gated flow, never to a depth-lifted model call (#40's stance). B's own + // tail message (Input[2]) may legitimately be compacted, so assert on the DEPTH message + // rather than on the call count — the marker is what proves Input[1] was left alone. + + // In the TAIL, the same global hit IS applied — and without a model call, which is the + // whole point of the cross-session cache. + offC := newComp(t, "extract_llm", cfg) + cmC := &countingModel{resp: filter} + reqC := &bschemas.BifrostChatRequest{Input: []bschemas.ChatMessage{ + userMsg("find the keep records"), toolMsg(body), + }} + var repC components.Report + if _, err := offC.Offload(reqC, &repC, &components.Ctx{Ctx: context.Background(), + Session: "C", Store: st, Model: components.ModelSpec{Static: cmC}, + CacheAware: true, MaxCachedIdx: -1, CtxWindow: 200_000}); err != nil { + t.Fatal(err) + } + if schema.MessageText(reqC.Input[1]) == body { + t.Fatal("a tail-position global hit must be applied") + } + if cmC.calls != 0 { + t.Fatalf("a global hit must avoid the model call, got %d", cmC.calls) + } +} diff --git a/components/offload/extract_econ.go b/components/offload/extract_econ.go new file mode 100644 index 0000000..fc7a455 --- /dev/null +++ b/components/offload/extract_econ.go @@ -0,0 +1,299 @@ +package offload + +import ( + "sync" + + "github.com/rossoctl/context-guru/components" + "github.com/rossoctl/context-guru/internal/cheapmodel" +) + +// The economic gate. extract_llm is the only component that SPENDS money to SAVE money, +// so it is the only one that can be net-negative — and on Terminal-Bench it was, by ~8x: +// 271 calls / $3.26 / ~1,592s of latency against ~197,548 unique tokens saved. +// +// The arithmetic behind that loss is the whole point. A saved token is only worth the +// rate it WOULD have been billed at. On a prompt-caching backend the request is ~99.95% +// cached, so a token removed from a cached region is worth the cache-READ rate +// ($0.20/MTok on the agent model), not the fresh-input rate ($2/MTok) — a 10x haircut. +// An extraction call costing ~$0.012 must therefore remove ~60,000 cache-read tokens to +// break even, versus ~6,000 fresh ones. Most tool outputs are nowhere near that, which is +// why "compress everything" loses on a caching backend and wins on a non-caching one. +// +// The component already contained this insight as a comment on skip_file_reads +// (extract_llm.go, on AUTO mode). This turns that reasoning into an actual gate that +// applies to EVERY candidate, not just line-numbered file reads. + +// --- Issue #28 part B: reusing the AGENT's cached prefix — PROTOTYPED AND REJECTED --- +// +// The proposal was to append the extraction instruction as a final user message after the +// agent's existing stable prefix, so extraction reads an already-cached context instead of +// paying fresh input on its own prompt. Prototyped against the live gateway +// (aws/claude-sonnet-5, a ~103k-token cached prefix). It works mechanically — the +// extraction turn read the full prefix from cache, no cache-write, no prefix invalidation: +// +// agent turn 1 (writes cache) in=1 write=103,019 read=0 +// agent turn 2 (reads cache) in=9 write=0 read=103,019 +// extraction reusing agent prefix in=25 write=0 read=103,019 +// +// But cache-read is cheap, not free, and the bill scales with the WHOLE context: +// +// dedicated haiku call (~3k prompt) $0.00400 +// reuse @ 103,019-token prefix $0.03398 8.5x +// reuse @ 500,000-token prefix $0.15307 38.3x +// reuse @ 1,700,000-token prefix $0.51307 128.3x +// +// At the ~1.7M contexts this workload actually reaches, one extraction costs ~128x a +// dedicated cheap-model call — and the component issues up to llmConcurrency=4 per turn, +// so ~$2.04/turn against ~$0.016. That is the opposite of the direction this issue exists +// to push. Paying 1.7M cache-read tokens to answer a question about ONE tool output is +// structurally wrong regardless of the rate. +// +// DECISION: NOT IMPLEMENTED. Three independent reasons, any one sufficient: +// 1. Cost: 8.5x-128x a dedicated call, worsening as context grows (measured above). +// 2. Cache-write risk on the agent's own prefix: a write is 11.5x a read, and putting +// extraction traffic on the agent's cache key risks exactly the mistake this whole +// workstream is about. The prototype did not trigger one, but it only takes one +// divergent breakpoint or an eviction between turns. +// 3. Coupling: it ties the compaction model to the agent model, so extraction quality, +// latency, and spend all move whenever someone changes the agent's model. +// +// The dedicated cheap model stays. Re-open only if a provider prices in-context follow-up +// questions at a flat rate rather than per cache-read token. + +// tokenValue is the dollars-per-token a SAVED token is worth, and the reason the gate +// exists. Both rates are per single token (not per million). +type tokenValue struct { + perToken float64 + cached bool // true when priced at the cache-read rate +} + +// Default agent-model rates (claude-sonnet-5 class, $3/$15 per MTok, cache read 0.1x). +// The gate is a comparison, so what matters is the RATIO between a saved token's value +// and the extraction call's cost — both scale together if an operator's contract differs. +const ( + agentFreshPerMTok = 3.00 + agentCacheReadPerMTok = 0.30 // 0.1x fresh, the standard Anthropic cache-read multiplier +) + +// savedTokenValue prices one saved token for THIS request. When the request goes to a +// prompt-caching backend, content the agent re-sends every turn is already in the cached +// prefix, so removing it saves the cache-read rate — the 10x haircut that sinks the +// component's economics. +func savedTokenValue(c *components.Ctx) tokenValue { + if c != nil && c.CacheAware { + return tokenValue{perToken: agentCacheReadPerMTok / 1_000_000, cached: true} + } + return tokenValue{perToken: agentFreshPerMTok / 1_000_000, cached: false} +} + +// priorCallCost is the fallback per-call cost estimate used before this process has +// observed any extraction call. ~$0.012 was the Terminal-Bench measurement; it is a PRIOR +// to be replaced by observation, never a constant to compute against — which is why +// callCost prefers cheapmodel.AvgCallCost as soon as one call has been made. +const priorCallCost = 0.012 + +// callCost returns the expected dollar cost of one extraction call. Observed-mean first +// (it reflects this deployment's real prompt sizes, model pricing, and whether the +// preamble cache is working), prior second. +func callCost(pricing cheapmodel.Pricing) float64 { + if avg, ok := cheapmodel.AvgCallCost(pricing); ok && avg > 0 { + return avg + } + return priorCallCost +} + +// gateDecision records why the gate allowed or suppressed a call. The reason string is +// the operator's answer to "why did this run?" / "why didn't it?", surfaced in metrics — +// a gate whose decisions you cannot explain is a gate nobody will trust enough to leave on. +type gateDecision struct { + allow bool + reason string + // expSaving/expCost are the dollar figures the decision compared, so a surprising + // suppression can be audited rather than guessed at. + expSaving float64 + expCost float64 +} + +// expectedReuses estimates how many future turns this compaction will be re-applied on. +// This is what makes extraction ever worthwhile under caching: the reduction is frozen and +// replayed on every subsequent turn (see state.go's freeze/reapply), so one call's saving +// is collected repeatedly. Recurrence is the strongest available signal — content the +// system has seen before in ANY session is likely to be seen again. +// +// ponytail: a flat prior per recurrence class, not a fitted model. Two observations +// (seen-before, request-position) capture most of the signal; upgrade to a per-session +// decay fit if the benchmark shows the estimate is what's mispricing calls. +func expectedReuses(seenBefore bool, turnsSoFar int) float64 { + if seenBefore { + // Recurred at least once already; the measured cross-session recurrence rate was + // 82/103 (~80%), so expect several more replays. + return 6 + } + if turnsSoFar >= 20 { + return 3 // late in a long session: fewer turns remain to amortize over + } + return 4 +} + +// evaluateGate decides whether one candidate output is worth an extraction call. +// +// expected saving = tokens we expect to remove x (1 + expected future reuses) x per-token value +// expected cost = observed mean cost of one extraction call +// +// Allow only when saving strictly exceeds cost. Every suppression carries a reason. +func evaluateGate(sizeTokens int, ratio float64, val tokenValue, cost float64, + seenBefore bool, turnsSoFar int) gateDecision { + + expectedRemoved := float64(sizeTokens) * ratio + reuses := expectedReuses(seenBefore, turnsSoFar) + // The compaction is applied on this turn AND replayed on each expected future turn. + saving := expectedRemoved * (1 + reuses) * val.perToken + + d := gateDecision{expSaving: saving, expCost: cost} + if saving <= cost { + // The honest message: on a caching backend a small output CANNOT pay for a call. + if val.cached { + d.reason = "suppressed: cache-aware, saving below call cost" + } else { + d.reason = "suppressed: saving below call cost" + } + return d + } + d.allow = true + switch { + case seenBefore: + d.reason = "allow: recurring content, amortized over reuses" + case !val.cached: + d.reason = "allow: non-caching backend, saved tokens at full rate" + default: + d.reason = "allow: expected saving exceeds call cost" + } + return d +} + +// defaultCompressionRatio is the fraction of an output an accepted extraction removes, +// used before this component has observed its own results. Deliberately CONSERVATIVE: +// over-estimating the ratio is how the component talked itself into 271 losing calls. +const defaultCompressionRatio = 0.45 + +// ratioTracker learns this workload's ACTUAL compression ratio from accepted results, so +// the gate stops guessing after the first few calls. A call that produced nothing counts +// as ratio 0 — a model that keeps failing to reduce this workload's outputs should drive +// the estimate down and shut the gate, which is precisely the feedback the old +// fixed-threshold design lacked. +type ratioTracker struct { + mu sync.Mutex + removed int64 + total int64 +} + +// observe records one attempted extraction: removedTok of totalTok (0 removed on a miss). +func (r *ratioTracker) observe(removedTok, totalTok int) { + if totalTok <= 0 { + return + } + r.mu.Lock() + r.removed += int64(removedTok) + r.total += int64(totalTok) + r.mu.Unlock() +} + +// ratio returns the observed compression ratio, or the conservative default until enough +// tokens have been seen for the estimate to mean anything. +func (r *ratioTracker) ratio() float64 { + r.mu.Lock() + defer r.mu.Unlock() + if r.total < minRatioSampleTokens { + return defaultCompressionRatio + } + return float64(r.removed) / float64(r.total) +} + +// minRatioSampleTokens is how much observed content the ratio estimate needs before it +// displaces the default — roughly a couple of medium outputs. +const minRatioSampleTokens = 4000 + +// --- Triggering (issue #28 part E) ----------------------------------------------- +// +// The old trigger was a raw token threshold (min_tokens) that had to be re-picked per +// workload — the component's worst ergonomic problem. The replacement asks the only +// question that generalizes: is this request under enough CONTEXT PRESSURE that removing +// tokens matters, and is there enough evidence that a call will pay? +// +// min_tokens stays honored when set explicitly (backward compatibility); the derived +// trigger is the DEFAULT when it is not. + +// contextPressure is the fraction of the model's context window the request occupies. +// 0 when the window is unknown, in which case pressure-based logic is skipped and the +// absolute floors apply — the same fail-open convention Trigger already uses. +func contextPressure(requestTokens, window int) float64 { + if window <= 0 || requestTokens <= 0 { + return 0 + } + return float64(requestTokens) / float64(window) +} + +// pressureFloor derives the per-output token floor from context pressure, replacing a +// hand-tuned min_tokens. The shape: when the context is nearly empty compaction buys +// nothing worth an LLM call, so demand a big output; as the window fills, the floor drops +// and smaller outputs become worth reducing. Returns an absolute token count. +// +// The numbers are chosen so a 1M-window model behaves sanely without tuning: +// +// <25% full -> 0.6% of window (~6000 tok on 1M): only large outputs +// 25-60% -> 0.3% of window (~3000 tok) +// 60-80% -> 0.15% of window (~1500 tok) +// >80% -> 0.05% of window (~500 tok): window pressure dominates, compact freely +func pressureFloor(window int, pressure float64) int { + if window <= 0 { + return 0 // unknown window: caller falls back to its absolute default + } + var frac float64 + switch { + case pressure > 0.80: + frac = 0.0005 + case pressure > 0.60: + frac = 0.0015 + case pressure > 0.25: + frac = 0.0030 + default: + frac = 0.0060 + } + if f := int(frac * float64(window)); f > 0 { + return f + } + return 0 +} + +// growthRate is tokens added since the previous turn, over the current size — how fast +// this session is accumulating context. A fast-growing request is where compaction has +// the most to work on; a static one has nothing new to reduce and should not re-fire. +func growthRate(currentTokens, prevTokens int) float64 { + if currentTokens <= 0 || prevTokens <= 0 || currentTokens <= prevTokens { + return 0 + } + return float64(currentTokens-prevTokens) / float64(currentTokens) +} + +// shouldFire decides whether the LLM path runs on this request at all, and why. It must +// NOT fire on every step of a merely-growing context — that was the old behavior's waste. +// +// minTokensSet reports whether the operator pinned min_tokens explicitly; when they did, +// their threshold governs and this stays out of the way. +func shouldFire(pressure, growth float64, minTokensSet bool) (bool, string) { + if minTokensSet { + return true, "explicit min_tokens/trigger configured" + } + switch { + case pressure > 0.60: + return true, "high context pressure" + case pressure > 0.25 && growth > 0.10: + return true, "moderate pressure with fast context growth" + case pressure > 0.25: + // Growing slowly at moderate pressure: the per-output floor still gates + // individual candidates, but do not spend a call on a static context. + return false, "moderate pressure but context near-static" + default: + return false, "low context pressure" + } +} diff --git a/components/offload/extract_econ_test.go b/components/offload/extract_econ_test.go new file mode 100644 index 0000000..9d02771 --- /dev/null +++ b/components/offload/extract_econ_test.go @@ -0,0 +1,211 @@ +package offload + +import ( + "math" + "testing" + + "github.com/rossoctl/context-guru/components" + "github.com/rossoctl/context-guru/internal/cheapmodel" +) + +// The gate must suppress when the request is cache-aware and the output is small — the +// exact case that made extract_llm ~8x underwater on Terminal-Bench. A 400-token output +// on a caching backend is worth ~400 x $0.30/MTok x (1+reuses); against a ~$0.012 call +// that cannot pay, and the reason must say so. +func TestGateSuppressesSmallOutputWhenCacheAware(t *testing.T) { + val := savedTokenValue(&components.Ctx{CacheAware: true}) + if val.cached != true { + t.Fatal("cache-aware ctx must price saved tokens at the cache-read rate") + } + d := evaluateGate(400, defaultCompressionRatio, val, priorCallCost, false, 5) + if d.allow { + t.Fatalf("small cached output must be suppressed: saving=$%.5f cost=$%.5f", d.expSaving, d.expCost) + } + if d.reason == "" { + t.Fatal("every suppression must carry a reason (the operator's first question)") + } + if d.expSaving >= d.expCost { + t.Fatalf("suppression must be justified by the numbers: saving=%v cost=%v", d.expSaving, d.expCost) + } +} + +// On a NON-caching backend the same reduction is billed at the full fresh-input rate — 10x +// more valuable — so the same output that loses under caching can win here. This is the +// asymmetry the gate exists to exploit, so assert it on ONE fixture, not two. +func TestGatePermitsOnNonCachingBackend(t *testing.T) { + size := 12000 + cached := evaluateGate(size, defaultCompressionRatio, + savedTokenValue(&components.Ctx{CacheAware: true}), priorCallCost, false, 5) + fresh := evaluateGate(size, defaultCompressionRatio, + savedTokenValue(&components.Ctx{CacheAware: false}), priorCallCost, false, 5) + + if !fresh.allow { + t.Fatalf("non-caching backend must permit a %d-token output: saving=$%.5f cost=$%.5f", + size, fresh.expSaving, fresh.expCost) + } + // The 10x rate difference must show up in the valuation, not just the verdict. + if ratio := fresh.expSaving / cached.expSaving; math.Abs(ratio-10) > 0.01 { + t.Fatalf("fresh tokens must be worth 10x cached ones, got %.3fx", ratio) + } +} + +// High-reuse (recurring) content is permitted even under caching, because the saving is +// collected on every turn the frozen compaction is replayed. Recurrence was measured at +// 82/103 across sessions, so this is the common case, not an edge case. +func TestGatePermitsHighReuseContent(t *testing.T) { + val := savedTokenValue(&components.Ctx{CacheAware: true}) + // 14000 tokens: above the ~12.7k cached-recurring break-even, below the ~17.8k + // cached-once one. This size is the gate's whole thesis in one fixture — recurrence is + // what tips an otherwise-losing call into profit, so the SAME size goes both ways. + size := 14000 + once := evaluateGate(size, defaultCompressionRatio, val, priorCallCost, false, 5) + recur := evaluateGate(size, defaultCompressionRatio, val, priorCallCost, true, 5) + + if recur.expSaving <= once.expSaving { + t.Fatalf("recurring content must be valued higher: recur=%v once=%v", recur.expSaving, once.expSaving) + } + if !recur.allow { + t.Fatalf("recurring %d-token content should be permitted: saving=$%.5f cost=$%.5f", + size, recur.expSaving, recur.expCost) + } + if once.allow { + t.Fatalf("at %d tokens, NON-recurring cached content should still be suppressed "+ + "(saving=$%.5f cost=$%.5f) — recurrence is what must tip the decision", + size, once.expSaving, once.expCost) + } +} + +// The break-even output size is the headline economic fact of issue #28, so pin it: under +// caching an extraction call must remove ~10x more tokens than on a non-caching backend. +// If these numbers drift, the component's viability has changed and the docs' verdict +// needs revisiting — which is exactly what this test exists to catch. +func TestBreakEvenSizesMatchTheDocumentedVerdict(t *testing.T) { + breakEven := func(cacheAware, recurring bool) int { + val := savedTokenValue(&components.Ctx{CacheAware: cacheAware}) + for size := 200; size <= 400_000; size += 100 { + if evaluateGate(size, defaultCompressionRatio, val, priorCallCost, recurring, 5).allow { + return size + } + } + return -1 + } + cachedRecur := breakEven(true, true) + freshRecur := breakEven(false, true) + if cachedRecur < 11_000 || cachedRecur > 14_000 { + t.Errorf("cached+recurring break-even = %d tokens, expected ~12,700 "+ + "(docs/components/extract_llm.md states this figure)", cachedRecur) + } + if freshRecur < 1_000 || freshRecur > 1_600 { + t.Errorf("fresh+recurring break-even = %d tokens, expected ~1,270", freshRecur) + } + // The 10x rate haircut must show up as a ~10x break-even gap. + if ratio := float64(cachedRecur) / float64(freshRecur); ratio < 8 || ratio > 12 { + t.Errorf("cached/fresh break-even ratio = %.1fx, expected ~10x", ratio) + } +} + +// The cost model must be real arithmetic over model pricing, not a hard-coded constant — +// "~$0.012/call" was one workload's average, and the gate has to track the actual model. +func TestCostModelMatchesKnownTokensTimesKnownPrice(t *testing.T) { + p := cheapmodel.Pricing{InputPerMTok: 1.00, OutputPerMTok: 5.00, + CacheWritePerMTok: 1.25, CacheReadPerMTok: 0.10} + // 1M input @ $1 + 1M output @ $5 + 1M write @ $1.25 + 1M read @ $0.10 = $7.35 + if got := p.Cost(1_000_000, 1_000_000, 1_000_000, 1_000_000); math.Abs(got-7.35) > 1e-9 { + t.Fatalf("Cost = %v, want 7.35", got) + } + // A realistic single extraction call: 3000 fresh in, 200 out on haiku rates. + // 3000/1e6*1 + 200/1e6*5 = 0.003 + 0.001 = 0.004 + if got := p.Cost(3000, 200, 0, 0); math.Abs(got-0.004) > 1e-9 { + t.Fatalf("Cost = %v, want 0.004", got) + } + // Env override must be honored so an operator prices their own deployment. + t.Setenv("CHEAP_MODEL_PRICE_IN", "2.50") + if got := cheapmodel.PricingFromEnv().InputPerMTok; got != 2.50 { + t.Fatalf("PricingFromEnv InputPerMTok = %v, want 2.50", got) + } +} + +// callCost must fall back to the prior only until a real observation exists; it must never +// return zero, which would make the gate permit everything. +func TestCallCostNeverZero(t *testing.T) { + if c := callCost(cheapmodel.HaikuPricing()); c <= 0 { + t.Fatalf("callCost must be positive, got %v", c) + } +} + +// The trigger must NOT fire on every step of a merely-growing context — firing every step +// is what produced 271 calls. Pressure gates it, and a near-static context is declined. +func TestTriggerDoesNotFireEveryStepOnGrowingContext(t *testing.T) { + window := 1_000_000 + fired := 0 + prev := 0 + // Simulate 40 turns growing by 5k tokens each: reaches only ~20% of a 1M window. + for turn := 1; turn <= 40; turn++ { + cur := turn * 5000 + p := contextPressure(cur, window) + g := growthRate(cur, prev) + if ok, _ := shouldFire(p, g, false); ok { + fired++ + } + prev = cur + } + if fired == 40 { + t.Fatal("trigger fired on EVERY step of a growing context — the #28 waste case") + } + if fired > 12 { + t.Fatalf("trigger fired on %d/40 low-pressure steps; expected few", fired) + } + // It must still fire when pressure is genuinely high — a gate that never fires is + // just as broken as one that always does. + if ok, reason := shouldFire(0.75, 0.05, false); !ok { + t.Fatalf("high pressure must fire, got reason %q", reason) + } +} + +// An explicitly-configured min_tokens keeps governing: existing configs must not change +// behavior silently under them. +func TestExplicitMinTokensStillGoverns(t *testing.T) { + ok, reason := shouldFire(0.01, 0, true) // pressure so low the derived trigger declines + if !ok { + t.Fatal("an explicit min_tokens/trigger must still fire (backward compatibility)") + } + if reason == "" { + t.Fatal("reason must be recorded even on the explicit path") + } +} + +// The derived per-output floor must fall as the window fills — no per-workload tuning. +func TestPressureFloorFallsAsContextFills(t *testing.T) { + window := 1_000_000 + low := pressureFloor(window, 0.10) + mid := pressureFloor(window, 0.40) + high := pressureFloor(window, 0.70) + full := pressureFloor(window, 0.90) + if !(low > mid && mid > high && high > full) { + t.Fatalf("floor must decrease monotonically with pressure: %d %d %d %d", low, mid, high, full) + } + if full <= 0 { + t.Fatal("a nearly-full window must still have a positive floor") + } + // An unknown window must yield 0 so the caller keeps its absolute default (fail open). + if pressureFloor(0, 0.9) != 0 { + t.Fatal("unknown window must return 0 (fall back to absolute default)") + } +} + +// The observed compression ratio must displace the default only once there is enough +// evidence, and a run of misses must drive it toward zero (shutting the gate). +func TestRatioTrackerLearnsFromObservations(t *testing.T) { + var r ratioTracker + if r.ratio() != defaultCompressionRatio { + t.Fatal("with no observations the conservative default must apply") + } + r.observe(100, 1000) // below the sample threshold + if r.ratio() != defaultCompressionRatio { + t.Fatal("a tiny sample must not displace the default") + } + r.observe(0, 20000) // plenty of evidence that this workload does not compress + if got := r.ratio(); got >= 0.10 { + t.Fatalf("repeated misses must drive the ratio down, got %v", got) + } +} diff --git a/components/offload/extract_llm.go b/components/offload/extract_llm.go index 3489165..fc01668 100644 --- a/components/offload/extract_llm.go +++ b/components/offload/extract_llm.go @@ -12,7 +12,9 @@ import ( bschemas "github.com/maximhq/bifrost/core/schemas" "github.com/rossoctl/context-guru/components" "github.com/rossoctl/context-guru/expand" + "github.com/rossoctl/context-guru/internal/cheapmodel" "github.com/rossoctl/context-guru/internal/extract" + "github.com/rossoctl/context-guru/metrics" "github.com/rossoctl/context-guru/schema" "gopkg.in/yaml.v3" ) @@ -60,6 +62,23 @@ type ExtractLLM struct { skipFileReads *bool // nil = auto (skip when cache-aware); true/false = force mu sync.Mutex llmSeen map[string]int // session -> count of qualifying (LLM-eligible) requests + + // minTokensSet records whether the operator pinned min_tokens / trigger explicitly. + // When they did, their threshold governs (backward compatibility). When they did not, + // the derived pressure-based trigger is the default — no per-workload tuning (#28 E). + minTokensSet bool + // gate enables the economic gate (#28 D). Default on; `economic_gate: false` restores + // the old spend-on-size behavior for anyone who needs to reproduce old numbers. + gate bool + // pricing prices the extraction model's tokens for the gate's cost side (#28 D). + pricing cheapmodel.Pricing + // ratios learns this workload's real compression ratio instead of assuming one. + ratios ratioTracker + // prevTokens tracks per-session request size so growth rate is measurable (#28 E). + prevTokens map[string]int + // modelName identifies the extraction model in the global cache key, so switching + // models misses rather than serving another model's extraction (#28 C). + modelName string } type extractLLMConfig struct { @@ -75,6 +94,11 @@ type extractLLMConfig struct { // required verbatim by the sanity check. Default true (the powerful mode) — set // false to force verified deletion-only. Rewrite *bool `yaml:"rewrite"` + // EconomicGate opts out of the expected-value gate (#28 D). Unset = ON (the default): + // only call the LLM when the expected saving exceeds the expected call cost, priced + // from real model rates and the cache-awareness of the traffic. Set false to restore + // the pre-#28 spend-on-size behavior — needed only to reproduce old benchmark numbers. + EconomicGate *bool `yaml:"economic_gate"` // SkipFileReads controls whether line-numbered source-file dumps are left verbatim. // Tri-state: unset = AUTO (skip when the request is prompt-cached, reduce otherwise); // true = always skip; false = always reduce. Rationale (measured, SWE-bench 50): @@ -88,7 +112,23 @@ type extractLLMConfig struct { func newExtractLLM(raw []byte) (components.Component, error) { cfg := extractLLMConfig{MinTokens: 300, Strategy: "code"} + // Detect whether the operator pinned a threshold BEFORE defaults are applied: the + // distinction between "unset" and "set to the default value" is what decides whether + // the smart trigger or their number governs, so it must be read from the raw YAML. + explicit := false if len(raw) > 0 { + var probe struct { + MinTokens *int `yaml:"min_tokens"` + Trigger *struct { + MinRequestTokens *int `yaml:"min_request_tokens"` + MinOutputTokens *int `yaml:"min_output_tokens"` + } `yaml:"trigger"` + } + if err := yaml.Unmarshal(raw, &probe); err == nil { + explicit = probe.MinTokens != nil || + (probe.Trigger != nil && + (probe.Trigger.MinRequestTokens != nil || probe.Trigger.MinOutputTokens != nil)) + } if err := yaml.Unmarshal(raw, &cfg); err != nil { return nil, err } @@ -100,15 +140,31 @@ func newExtractLLM(raw []byte) (components.Component, error) { if cfg.Strategy == "" { cfg.Strategy = "code" } + gate := true // economic gate on by default (#28 D) + if cfg.EconomicGate != nil { + gate = *cfg.EconomicGate + } return &ExtractLLM{ minTokens: cfg.MinTokens, strategy: cfg.Strategy, modelSource: cfg.Model.Source, modelClient: cfg.Model.Client(), trigger: cfg.Trigger, mode: parseMarkerMode(cfg.MarkerMode), rewrite: rewrite, llmEveryN: cfg.LLMEveryN, llmMaxPerReq: cfg.LLMMaxPerReq, skipFileReads: cfg.SkipFileReads, llmSeen: map[string]int{}, + minTokensSet: explicit, gate: gate, pricing: cheapmodel.PricingFromEnv(), + prevTokens: map[string]int{}, modelName: cfg.Model.Model, }, nil } +// noteRequestSize records this request's size for the session and returns the previous +// one, so the trigger can measure context growth rate (#28 E). +func (e *ExtractLLM) noteRequestSize(session string, tokens int) int { + e.mu.Lock() + defer e.mu.Unlock() + prev := e.prevTokens[session] + e.prevTokens[session] = tokens + return prev +} + func (*ExtractLLM) Name() string { return "extract_llm" } func (*ExtractLLM) Enabled(*components.Ctx) bool { return true } @@ -166,7 +222,39 @@ func (e *ExtractLLM) Offload(req *bschemas.BifrostChatRequest, rep *components.R if model != nil && fires && !e.llmAllowedThisRequest(c.Session) { model = nil } + // Derived trigger (#28 E): context pressure + growth rate replace a hand-tuned + // threshold. When min_tokens/trigger is set explicitly the operator's value governs. + reqTokens := schema.MessagesTokens(req) + prevTokens := e.noteRequestSize(c.Session, reqTokens) + pressure := contextPressure(reqTokens, c.CtxWindow) + growth := growthRate(reqTokens, prevTokens) + pressureFires, triggerReason := shouldFire(pressure, growth, e.minTokensSet) + // An unknown context window (0) makes pressure meaningless; fall back to the + // configured Trigger alone, the same fail-open convention Trigger itself uses. + if c.CtxWindow <= 0 { + pressureFires, triggerReason = fires, "context window unknown; absolute trigger only" + } + if model != nil && !pressureFires { + model = nil // no model call this request; frozen reapplications still run below + } + metrics.RecordExtractionReason(triggerReason) + floor := e.outputFloor(c.CtxWindow) + // Without an explicit min_tokens, derive the per-output floor from context pressure so + // there is no per-workload number to pick (#28 E). + if !e.minTokensSet { + if pf := pressureFloor(c.CtxWindow, pressure); pf > 0 { + floor = pf + } + } + // Gate inputs shared by every candidate this request. + val := savedTokenValue(c) + perCall := callCost(e.pricing) + ratio := e.ratios.ratio() + turnsSoFar := len(req.Input) + extCfg := extract.DefaultCfg() + extCfg.Mode, extCfg.Floor, extCfg.Rewrite = e.strategy, floor, e.rewrite + keepIDs := extract.HarvestIdentifiers(goal, 40) tools := toolIndices(req) var keys []string @@ -226,7 +314,11 @@ func (e *ExtractLLM) Offload(req *bschemas.BifrostChatRequest, rep *components.R if isKeptVerbatim(c, id) { continue } + // SAME-SESSION replay first. This session already sent these compacted bytes on an + // earlier turn, so the provider's cached prefix holds the COMPACTED form and replaying + // it is byte-identical — which is why it is safe at any depth, cached prefix included. if cached, hit := getResult(c, id); hit { + metrics.RecordExtractionCacheLookup(true) apply(i, content, cached.Projected, cached.Summary) dbgReapply++ continue @@ -251,6 +343,29 @@ func (e *ExtractLLM) Offload(req *bschemas.BifrostChatRequest, rep *components.R } continue } + // CROSS-SESSION reuse (#28 C), deliberately placed AFTER the tail gate. An extraction + // is a context-free derived result, so a global content-hash key lets a different + // session skip re-deriving it (measured: 82 of 103 unique contents recurred across + // sessions). But cache-safety differs from the same-session replay above: THIS session + // never sent these compacted bytes, so the provider's cached prefix holds the ORIGINAL. + // Splicing a global hit at depth would mutate already-cached content and force a + // suffix re-write at 11.5x the read price — the same churn the tail gate exists to + // prevent, just sourced from a cache instead of a model call. So a global hit is + // treated exactly like a new decision: tail-only, then frozen and replayed from there. + // + // It cannot become a re-derive path (#40's stance): on a MISS this falls through to the + // normal candidate flow, which is already tail-gated and floor-gated; nothing here + // lifts a depth restriction the way the removed repairLostResult did. + if cached, hit := getResultGlobal(c, extract.ResultKey(id, e.modelName, extCfg)); hit { + metrics.RecordExtractionCacheLookup(true) + // Freeze into this session so later turns replay it byte-for-byte from the + // same-session path above, at any depth. + putResult(c, id, cached.Projected, cached.Summary) + apply(i, content, cached.Projected, cached.Summary) + dbgReapply++ + continue + } + metrics.RecordExtractionCacheLookup(false) if sz < floor { dbgFloor++ continue // only medium/large outputs are worth a model call @@ -264,6 +379,27 @@ func (e *ExtractLLM) Offload(req *bschemas.BifrostChatRequest, rep *components.R if skipFR && looksLikeFileRead(content) { continue } + // The economic gate (#28 D). This is the check the component never had: is one + // LLM call worth it for THIS output, given that a saved token in a cached region + // is worth 10x less? Where caching makes extraction pointless, suppress it; on a + // non-caching backend or for recurring content, allow it. + if e.gate { + // Content the store has seen before in ANY session is likely to recur again, + // which is what amortizes the call across turns. + seenBefore := hasSeenContent(c, id) + d := evaluateGate(sz, ratio, val, perCall, seenBefore, turnsSoFar) + if !d.allow { + metrics.RecordExtractionSuppressed(d.reason) + if debugExtractLLM { + slog.Info("cg.debug.extract_llm.gate", "decision", "suppress", + "reason", d.reason, "size", sz, "exp_saving_usd", d.expSaving, + "exp_cost_usd", d.expCost, "cacheAware", c.CacheAware) + } + continue + } + metrics.RecordExtractionReason(d.reason) + } + markSeenContent(c, id) cands = append(cands, cand{i, content, id}) } if debugExtractLLM && len(tools) > 0 { @@ -294,13 +430,20 @@ func (e *ExtractLLM) Offload(req *bschemas.BifrostChatRequest, rep *components.R defer wg.Done() sem <- struct{}{} defer func() { <-sem }() - cfg := extract.DefaultCfg() - cfg.Mode, cfg.Floor, cfg.Rewrite = e.strategy, floor, e.rewrite ctx, cancel := context.WithTimeout(c.Ctx, llmCallTimeout) defer cancel() - res, sum, _ := extract.RunExtractionSummary(ctx, cands[k].content, goal, keepIDs, schema.TextTokens(cands[k].content), cfg, model) + before := schema.TextTokens(cands[k].content) + start := time.Now() + res, sum, _ := extract.RunExtractionSummary(ctx, cands[k].content, goal, keepIDs, before, extCfg, model) + metrics.RecordExtractionCall(float64(time.Since(start).Milliseconds())) if res != "" && res != cands[k].content { out[k] = outT{res, sum} + // Feed the observed ratio so the gate prices future calls on what this + // workload actually achieves, not on an assumption. + e.ratios.observe(before-schema.TextTokens(res), before) + metrics.RecordExtractionSaving(before - schema.TextTokens(res)) + } else { + e.ratios.observe(0, before) // a miss is real evidence: ratio 0 } }(k) } @@ -310,6 +453,14 @@ func (e *ExtractLLM) Offload(req *bschemas.BifrostChatRequest, rep *components.R continue } putResult(c, cands[k].id, out[k].projected, out[k].summary) + // Publish to the GLOBAL namespace only when the result is recoverable (or verified + // deletion-only). An unverified lossy rewrite with no way back must not become + // another session's starting point; the session-scoped write above still gives this + // session cross-turn replay. + if !e.rewrite || effectiveMode(c, e.mode) == markerFull { + putResultGlobal(c, extract.ResultKey(cands[k].id, e.modelName, extCfg), + out[k].projected, out[k].summary) + } apply(cands[k].i, cands[k].content, out[k].projected, out[k].summary) } } diff --git a/components/offload/state.go b/components/offload/state.go index 7d868d0..0eab181 100644 --- a/components/offload/state.go +++ b/components/offload/state.go @@ -60,6 +60,72 @@ func putResult(c *components.Ctx, id, projected, summary string) { } } +// --- Global (session-independent) extraction result cache (#28 C) ----------- +// +// The session prefix above was throwing away most of the available reuse: an extraction +// is a CONTEXT-FREE derived result, so the same content under the same extractor +// semantics reduces the same way in any session. Measured on Terminal-Bench: 82 of 103 +// unique contents recurred ACROSS sessions, and ~93% of the component's realized value +// came from cache reuse rather than from new LLM calls. +// +// Contrast issue #27's xdedup index, which is session-scoped ON PURPOSE: it mints a +// conversational reference ("same as step N") that is meaningless outside its session. +// The distinction is reference vs derived result, and it decides the namespace. +// +// The key is built by extract.ResultKey (content + prompt version + model + config +// fingerprint), so a prompt bump, a model switch, or a config change MISSES rather than +// silently serving a stale extraction. The store is already bounded (TTL + LRU), which +// bounds this namespace too. + +// getResultGlobal returns a previously cached reduced output for a global key. +// +// ONE key holding the whole decision, matching the session-scoped cachedResult above and +// for the same reason: as two independently-TTL'd, independently-pinned keys, losing only +// the summary made the replay HIT and emit different bytes (the "[summary] " segment +// silently vanishing) with nothing reported lost. That failure is if anything worse in the +// global namespace, where a half-decision can propagate into a session that never produced +// it. +func getResultGlobal(c *components.Ctx, gkey string) (cachedResult, bool) { + b, ok := c.Store.Get(gkey) + if !ok { + return cachedResult{}, false + } + var r cachedResult + if json.Unmarshal(b, &r) != nil || r.Projected == "" { + return cachedResult{}, false // unreadable => absent, never splice half a decision + } + return r, true +} + +// putResultGlobal caches a reduced output under its global key. +func putResultGlobal(c *components.Ctx, gkey, projected, summary string) { + if b, err := json.Marshal(cachedResult{Projected: projected, Summary: summary}); err == nil { + c.Store.Put(gkey, b) + } +} + +// --- Content recurrence (the economic gate's reuse signal) ------------------- +// +// The gate needs to know whether content is likely to RECUR, because a compaction's +// saving is collected on every later turn it is replayed on — recurrence is what makes an +// extraction call pay for itself under caching. Recording each content key we considered +// (session-independent, like the result cache) turns "have I seen this before anywhere?" +// into a cheap store lookup. + +func seenKey(ck string) string { return "cg:xseen:" + ck } + +// hasSeenContent reports whether this content was considered for extraction before. +func hasSeenContent(c *components.Ctx, ck string) bool { + _, ok := c.Store.Get(seenKey(ck)) + return ok +} + +// markSeenContent records that this content was considered, so a later encounter reads as +// recurring. +func markSeenContent(c *components.Ctx, ck string) { + c.Store.Put(seenKey(ck), []byte{1}) +} + // --- Freeze + reapply (cache stability) ------------------------------------- // // The cache-safety invariant: once an offloader compacts an output, it must send the diff --git a/docs/components/extract_llm.md b/docs/components/extract_llm.md index 4552204..33e8ba6 100644 --- a/docs/components/extract_llm.md +++ b/docs/components/extract_llm.md @@ -1,9 +1,49 @@ # extract_llm -!!! info "Offload — lossy, reversible (LLM-written filter)" +!!! warning "Offload — lossy, reversible (LLM-written filter). **Spends money to save money.**" A cheap model writes a small program that projects a large tool output down to what the agent actually needs, deletes the rest, and stashes the original. The powerful, relevance-aware - counterpart to the deterministic [`extract`](extract.md). + counterpart to the deterministic [`extract`](extract.md) — and the only component whose + savings can be **net negative**. Read [Economics](#economics) before enabling it. + +## The honest verdict + +On a **prompt-caching backend** (the default for Anthropic/Bedrock traffic), `extract_llm` is +**usually not worth running**, and the measurements say so plainly: + +| Measured (Terminal-Bench, pre-#28) | Value | +|---|---| +| Extraction calls | 271 | +| Extraction cost | $3.26 | +| Cumulative added latency | ~1,592 s (~450 ms/call) | +| Unique tokens saved | ~197,548 | +| Value of those tokens at the **cache-read** rate | ~$0.06 | +| **Net** | **deeply negative (~8× underwater against cache-aware value)** | +| Share of realized value that came from the **replay cache**, not the LLM | **~93%** | + +The reason is arithmetic, not implementation quality. A request to a caching backend is +~99.95% cached, so a token removed from a cached region saves the **cache-read** rate +(`$0.30/MTok`), not the fresh-input rate (`$3/MTok`) — a **10× haircut**. An extraction call +costing ~$0.012 must therefore remove a *lot* of tokens to break even: + +| Backend | Content | Break-even output size | +|---|---|---| +| Caching | seen once | **~17,800 tokens** | +| Caching | recurring (amortized over replays) | **~12,700 tokens** | +| Non-caching | seen once | ~1,780 tokens | +| Non-caching | recurring | **~1,270 tokens** | + +Most tool outputs are nowhere near 12,700 tokens. That is why the same component **wins on a +non-caching backend and loses on a caching one**, and why the fix is not "compress harder" but +"decide per call". Since #28 the [economic gate](#economics) makes that decision automatically, +so the component is safe to leave enabled — it simply declines to spend where it cannot win. + +!!! tip "If you only remember one thing" + On a caching backend, expect `extract_llm` to suppress most candidates and contribute + little; its value comes from the **result cache**, not from new LLM calls. On a + **non-caching** backend it is genuinely valuable. Check + `/stats` → `extract.net_value_usd` — if it is negative on your workload, remove the + component from the pipeline. ## How it works @@ -23,22 +63,152 @@ filtered structurally. - **Model source:** `model.source` is `incoming` (default — reuse the proxied request's own model + key) or `config` (a dedicated cheap model set via `CHEAP_MODEL*` env / the gateway's `CheapModel`). With no model available it degrades to a no-op (the deterministic `extract` still runs if present). -- **Throttled + reused:** this is the expensive pass, so it is gated by `trigger` - (`min_output_tokens`, `min_request_tokens`, `min_messages`) and throttled per session - (`llm_every_n_requests`) and per request (`llm_max_per_request`). A reduced output is **checkpointed - per session by content hash** — the same output re-sent on a later turn reuses the prior compaction - (no new model call, byte-identical result → prefix stays KV-cache stable). +- **Frozen and replayed:** a reduced output is checkpointed by content hash — the same output + re-sent on a later turn reuses the prior compaction (no new model call, byte-identical result → + the request prefix stays KV-cache stable). This replay is where **~93%** of the component's + realized value comes from. - **Cache-aware `skip_file_reads`:** tri-state. Unset = AUTO — skip line-numbered source-file dumps - when the request is prompt-cached (they already bill at the cheap cache-read rate, so reducing them - costs more than it saves), reduce them otherwise. See the cache-aware rationale in - [design.md](../design.md). + when the request is prompt-cached (they already bill at the cheap cache-read rate), reduce them + otherwise. The economic gate now generalizes this same reasoning to *every* candidate. + +## Economics + +Since #28 the component only calls the LLM when **expected saving > expected cost**. + +``` +expected saving = tokens expected to remove + x (1 + expected future replays) + x per-token value <-- cache-read rate when cache-aware, else fresh rate + +expected cost = observed mean cost of one extraction call +``` + +Each input is measured rather than assumed: + +| Input | Source | +|---|---| +| Per-token value | `Ctx.CacheAware` selects the cache-read vs fresh rate (the 10× factor) | +| Expected compression ratio | **Learned** from this workload's accepted results; a conservative 0.45 until ~4k tokens of evidence. Repeated misses drive it toward 0, shutting the gate | +| Call cost | **Observed mean** of real calls (tokens × real model pricing), not a hard-coded constant. Falls back to a ~$0.012 prior only before the first call | +| Model pricing | `claude-haiku-4-5` list rates by default; override with `CHEAP_MODEL_PRICE_IN` / `_OUT` / `_CACHE_WRITE` / `_CACHE_READ` (dollars per MTok) | +| Expected replays | Recurrence: content seen before in **any** session is expected to recur (measured 82/103 across sessions) | +| Remaining horizon | Fewer expected replays late in a long session | + +Every decision records a **reason**, visible at `/stats` → `extract.reasons` and +`extract.top_reason`, because the first question about an expensive component is always "why did +this run?". Set `economic_gate: false` to restore the pre-#28 spend-on-size behavior — needed only +to reproduce old benchmark numbers. + +## Triggering + +There is **no per-workload threshold to tune**. When `min_tokens` / `trigger` is unset, the +component derives its own gating from context pressure and growth: + +| Context pressure (request ÷ window) | Behavior | +|---|---| +| > 80% | Per-output floor ~0.05% of the window — window pressure dominates, compact freely | +| 60–80% | Floor ~0.15%; fires on pressure alone | +| 25–60% | Floor ~0.30%; fires only if the context is also **growing** > 10%/turn | +| < 25% | Does not fire — compaction buys nothing worth an LLM call | + +A *merely growing* context does not fire on every step; that was the behavior that produced 271 +calls. When the context window is unknown (0) the derived logic is skipped and the configured +absolute `trigger` applies — the same fail-open convention `Trigger` already uses. + +**`min_tokens` still governs when set explicitly**, so existing configs keep their behavior. + +## Caching + +Three distinct caches, easily confused: + +1. **Global result cache** (new in #28). An extraction is a *context-free derived result*, so it is + keyed on `sha256(content + prompt version + model + config fingerprint)` with **no session + prefix** — identical content in a different session reuses the reduction. Previously the key + carried a session prefix, discarding ~80% of the available reuse. A prompt-version bump, model + switch, or config change **misses** rather than serving a stale extraction. Bounded by the + store's existing TTL + LRU. + + !!! note "One-time invalidation" + The key schema changed, so pre-#28 entries are inert (a miss, never mis-served). A + session-scoped entry from the old scheme is still honored as a migration read, so an + in-flight session does not re-pay for work already done. + + Contrast [`xdedup`](../components.md) (#27), which is session-scoped **on purpose**: it mints a + *conversational reference* ("same as step N") that is meaningless outside its session. Reference + vs derived result is what decides the namespace. + +2. **Provider prompt cache on the extraction preamble** (#28 part A). The ~1,463-token invariant + preamble is sent as a stable `system` block with a `cache_control` breakpoint (a leading system + message on the OpenAI backend, which has no explicit breakpoints). + + !!! warning "Measured: this buys nothing on `claude-haiku-4-5`" + A breakpoint below the model's **minimum cacheable prefix** is silently ignored — no error, + `cache_creation_input_tokens: 0`. That minimum is **4096 tokens on `claude-haiku-4-5`** and + 1024 on `claude-sonnet-5`, against a **1,463-token** preamble. Verified against the gateway: + + | Prefix | Model | Result | + |---|---|---| + | ~1.5k | `claude-haiku-4-5` | `write=0 read=0` — **inert** | + | ~4.5k | `claude-haiku-4-5` | `write=5401` then `read=5401` — caches | + | ~1.5k | `claude-sonnet-5` | `write=2653` then `read=2653` — caches | + + So with the default cheap model the split is **structurally inert**; it pays only when + extraction runs on a larger-context model (`model.source: incoming`). The split ships anyway + — it is free, correct, and wins where it can — but do **not** infer a cache win from the + fact that a breakpoint was placed. Watch `/stats` → + `extract.prompt_cache_read_tokens`: if it stays 0 while `extract.calls` climbs, the + breakpoint is inert on your model. + +3. **The agent's own KV cache**, which the component must not disturb — hence freeze-and-replay and + the tail-only gate for new decisions. + +### Rejected: reusing the agent's cached prefix + +#28 part B proposed appending the extraction instruction after the agent's existing cached prefix so +extraction reads an already-cached context. **Prototyped against the live gateway and rejected.** It +works mechanically (the extraction turn read a 103,019-token prefix from cache with no cache-write +and no prefix invalidation), but cache-read is cheap, not free, and the bill scales with the *whole* +context: + +| Prefix size | Cost of one extraction | vs a dedicated cheap-model call ($0.004) | +|---|---|---| +| 103,019 tok | $0.03398 | 8.5× | +| 500,000 tok | $0.15307 | 38.3× | +| 1,700,000 tok | $0.51307 | **128.3×** | + +At the ~1.7M contexts this workload reaches, and with up to 4 concurrent per-output calls per turn, +that is ~$2.04/turn against ~$0.016 — the opposite of this issue's direction. Paying 1.7M +cache-read tokens to answer a question about one tool output is structurally wrong regardless of +rate. Two further reasons, each independently sufficient: it risks a **cache-write on the agent's +own prefix** (11.5× a read — exactly the mistake this workstream exists to avoid), and it **couples +the compaction model to the agent model**. Re-open only if a provider prices in-context follow-up +questions at a flat rate. + +## Metrics + +`/stats` gains an `extract` block (purely additive — every pre-existing field keeps its name, so +`deploy/harbor/*.py` keeps parsing unchanged): + +| Field | Meaning | +|---|---| +| `calls` | Extraction LLM calls made | +| `calls_avoided` | Calls avoided by the global result cache | +| `calls_suppressed` | Calls declined by the economic gate | +| `cache_hit_rate` | `calls_avoided / cache_lookups` | +| `prompt_cache_read_tokens` / `..._write_tokens` | Preamble caching behavior — **0 read means the breakpoint is inert** | +| `extraction_cost_usd` | What the component spent | +| `gross_value_usd` | What its saved tokens are worth at the rate they'd have been billed | +| **`net_value_usd`** | **The honest headline. Negative = the component is underwater.** | +| `avg_latency_ms` | Mean wall time per call (latency cost on the hot path) | +| `gross_saved_tokens` | Tokens removed | +| `reasons` / `top_reason` | Why extraction ran or was suppressed | ## Before → After Captured **live** through the proxy (`pipeline: [extract_llm]`, `strategy: code`, -`model.source: config` → `aws/claude-haiku-4-5`). The query was *"find the auth timeout error and -nearby context"*; the model kept the error plus a few surrounding requests and elided ~118 repetitive -successful-request lines: +`model.source: config` → `aws/claude-haiku-4-5`, `economic_gate: false` to force the call). The query +was *"find the auth timeout error and nearby context"*; the model kept the error plus a few +surrounding requests and elided ~118 repetitive successful-request lines: ``` before: 2024 GET /users/0 200 12ms ← 60 near-identical lines @@ -57,6 +227,9 @@ after: 2024 GET /users/58 200 12ms <> [full output: call context_guru_expand] ``` +Note the reduction is real and useful — the problem was never output quality, it was whether the +call was worth its price on a caching backend. + ## Lossiness Lossy but reversible — the original is stashed and recovered via `context_guru_expand` / @@ -67,25 +240,31 @@ Lossy but reversible — the original is stashed and recovered via `context_guru | Key | Default | Meaning | |---|---|---| -| `min_tokens` | — | Output floor (folds into `trigger.min_output_tokens`). | +| `economic_gate` | `true` | Only call the LLM when expected saving > expected cost. `false` restores pre-#28 spend-on-size behavior. | +| `min_tokens` | *derived* | Output floor. **Unset = derived from context pressure** (no tuning). Set explicitly to pin it (folds into `trigger.min_output_tokens`). | | `strategy` | `code` | `code` \| `single` \| `rlm` \| `auto` (`rlm` maps to `code`). | | `model.source` | `incoming` | `incoming` (proxied model+key) or `config` (cheap model via `CHEAP_MODEL*`). | -| `trigger` | — | Gates a model call: `min_output_tokens`, `min_request_tokens`, `min_messages`. | +| `trigger` | *derived* | Explicit gate: `min_output_tokens`, `min_request_tokens`, `min_messages`. Setting any pins the trigger. | | `llm_every_n_requests` | — | Fire the LLM path at most once per N requests per session. | | `llm_max_per_request` | 0 | Cap LLM calls per firing request (0 = unlimited). | | `rewrite` | `true` | `false` forces the verified deletion-only (subsequence) guarantee. | | `skip_file_reads` | auto | Skip line-numbered source dumps when cached; `true`/`false` to force. | | `marker_mode` | `full` | How the recovery marker is emitted: `full` \| `summary` \| `off`. | +Extraction-model pricing for the gate comes from `CHEAP_MODEL_PRICE_IN`, `_OUT`, +`_CACHE_WRITE`, `_CACHE_READ` (dollars per MTok; defaults are `claude-haiku-4-5` list rates). + ## When it shines -Big, query-focused MCP/API outputs, logs, and file reads; structured JSON where a filter can select -records precisely. It is the largest deterministic saving in the SWE-bench sweep alongside the cheap -`extract`/`dedup`/`cmdfilter` passes — see [RESULTS.md](../RESULTS.md). +**Non-caching backends** — every removed token is a direct saving at the full input rate, so the +break-even is ~10× easier. Also: very large single outputs (>~12k tokens) even under caching; +recurring content that amortizes one call across many replays; and novel prose/log shapes no +deterministic rule anticipates — this is the only component that can compress those. ## When it's inert -Output below the floor, request below `trigger`, throttled out this turn, projection not smaller, or -no model available. +Output below the derived floor, low context pressure, **suppressed by the economic gate** (the +common case on a caching backend), throttled out this turn, result served from the global cache, +projection not smaller, or no model available. See also: [`extract`](extract.md) · [Components overview](../components.md) · [Choose a preset](../how-to/choose-a-preset.md) diff --git a/docs/reference/config.md b/docs/reference/config.md index b869b5a..daac9bb 100644 --- a/docs/reference/config.md +++ b/docs/reference/config.md @@ -90,6 +90,22 @@ for every component's config block. | `--store` / `STORE` | on | Enable/disable the state store; `--store=false` disables offload reversibility. Wins over the file's `store:` block. | | `--mode` / `MODE` | `sync` | Operating mode: `sync` \| `observe`. Wins over the file's `mode:`. | +### Extraction-model pricing + +[`extract_llm`](../components/extract_llm.md)'s economic gate only calls the LLM when the +expected saving exceeds the expected cost, so it needs the real price of a call. The cost is +computed from **observed token usage × these rates** — never a hard-coded per-call constant. +Defaults are `claude-haiku-4-5` list rates; override them to match your contract. + +| Env | Default | Purpose | +|---|---|---| +| `CHEAP_MODEL_PRICE_IN` | `1.00` | Extraction-model input price, **dollars per million tokens**. | +| `CHEAP_MODEL_PRICE_OUT` | `5.00` | Output price per MTok. | +| `CHEAP_MODEL_PRICE_CACHE_WRITE` | `1.25` | Cache-write price per MTok (1.25× input). | +| `CHEAP_MODEL_PRICE_CACHE_READ` | `0.10` | Cache-read price per MTok (0.1× input). | + +An unparseable or absent value silently keeps the default — pricing must never fail a request. + ## Diagnostics | Env | Effect | diff --git a/docs/reference/presets.md b/docs/reference/presets.md index 2edece7..75fce52 100644 --- a/docs/reference/presets.md +++ b/docs/reference/presets.md @@ -32,5 +32,22 @@ taken exactly from the `presets` map in `config/config.go`. [#36](https://github.com/rossoctl/context-guru/pull/36), and placement has never been shown to help. Add it by hand if you want to run the placement study. +!!! warning "`extract_llm` now declines uneconomic calls (`codesmart`, `aggressive`)" + `extract_llm` is the only component that spends money to save money, and on a + prompt-caching backend it was measured **~8× underwater**: a token removed from a cached + region saves the cache-read rate (`$0.30/MTok`), not the fresh-input rate (`$3/MTok`), so + break-even is **~12,700 tokens per output** — far above a typical tool output. + + Since #28 it applies an [economic gate](../components/extract_llm.md#economics): it calls + the LLM only when the expected saving exceeds the expected cost. On a caching backend that + means **most candidates are suppressed** and `codesmart`/`aggressive` make far fewer model + calls than before — cheaper and faster, with savings coming mainly from the (now global) + result cache. On a **non-caching** backend the gate permits far more, because there the + reduction is worth 10× more. + + Check `/stats` → `extract.net_value_usd`. If it is negative on your workload, drop + `extract_llm` from the pipeline (or use `codesafe`, which never had it). `codesmart`'s + pinned `min_tokens: 3000` still governs its per-output floor, so that part is unchanged. + Not sure which to pick? See [Choose a preset](../how-to/choose-a-preset.md). Every component's config lives in [Components](../components.md). diff --git a/internal/cheapmodel/anthropic.go b/internal/cheapmodel/anthropic.go index 88b6100..d1e119a 100644 --- a/internal/cheapmodel/anthropic.go +++ b/internal/cheapmodel/anthropic.go @@ -30,6 +30,22 @@ type Anthropic struct { } func (a Anthropic) Complete(ctx context.Context, prompt string) (string, error) { + return a.CompleteSystem(ctx, "", prompt) +} + +// CompleteSystem sends the invariant instructions as a stable `system` block carrying +// a `cache_control` breakpoint, and the per-call variable part as the user message. On +// a repeated call the preamble bills at the cache-READ rate instead of fresh input. +// +// MEASURED CAVEAT (this is why the caller must not assume a win): a breakpoint below +// the model's MINIMUM CACHEABLE PREFIX is silently ignored — no error, no cache entry, +// `cache_creation_input_tokens: 0`. That minimum is 4096 tokens on claude-haiku-4-5 and +// 1024 on claude-sonnet-5, while the extractor's invariant preamble is ~1463 tokens. So +// on the CHEAP model (haiku) this split provably caches NOTHING, and on the agent model +// (model.source: incoming, the default) it does. Split anyway — it is free, correct, and +// wins on the source that can win — but price the gate on cache_read being ZERO. +// Verified against the gateway: haiku 1.5k => write=0 read=0; sonnet 1.5k => write then read. +func (a Anthropic) CompleteSystem(ctx context.Context, system, prompt string) (string, error) { base := a.BaseURL if base == "" { base = "https://api.anthropic.com" @@ -42,11 +58,18 @@ func (a Anthropic) Complete(ctx context.Context, prompt string) (string, error) if client == nil { client = http.DefaultClient } - reqBody, _ := json.Marshal(map[string]any{ + payload := map[string]any{ "model": a.Model, "max_tokens": maxTok, "messages": []any{map[string]any{"role": "user", "content": prompt}}, - }) + } + if system != "" { + payload["system"] = []any{map[string]any{ + "type": "text", "text": system, + "cache_control": map[string]any{"type": "ephemeral"}, + }} + } + reqBody, _ := json.Marshal(payload) req, err := http.NewRequestWithContext(ctx, http.MethodPost, strings.TrimRight(base, "/")+"/v1/messages", bytes.NewReader(reqBody)) if err != nil { @@ -73,14 +96,19 @@ func (a Anthropic) Complete(ctx context.Context, prompt string) (string, error) Text string `json:"text"` } `json:"content"` Usage struct { - InputTokens int `json:"input_tokens"` - OutputTokens int `json:"output_tokens"` + InputTokens int `json:"input_tokens"` + OutputTokens int `json:"output_tokens"` + CacheCreationTok int `json:"cache_creation_input_tokens"` + CacheReadTok int `json:"cache_read_input_tokens"` } `json:"usage"` } if err := json.NewDecoder(resp.Body).Decode(&out); err != nil { return "", err } - recordUsage(out.Usage.InputTokens, out.Usage.OutputTokens) // track CG component LLM cost + // track CG component LLM cost, split by cache tier so /stats can show whether the + // preamble breakpoint actually caches (read>0) or is silently ignored (read==0). + recordUsageCache(out.Usage.InputTokens, out.Usage.OutputTokens, + out.Usage.CacheCreationTok, out.Usage.CacheReadTok) // Return the first content block that carries text. A leading non-text block // (e.g. "thinking") has an empty Text, so we skip it rather than returning "". for _, c := range out.Content { diff --git a/internal/cheapmodel/openai.go b/internal/cheapmodel/openai.go index 1809950..40aeac3 100644 --- a/internal/cheapmodel/openai.go +++ b/internal/cheapmodel/openai.go @@ -20,6 +20,15 @@ type OpenAI struct { } func (o OpenAI) Complete(ctx context.Context, prompt string) (string, error) { + return o.CompleteSystem(ctx, "", prompt) +} + +// CompleteSystem puts the invariant instructions in a leading `system` message. OpenAI +// has no explicit cache breakpoints — caching is automatic on a shared prefix — so there +// is nothing to mark; a stable leading system message IS the cacheable-prefix idiom here. +// The split is therefore honest on both backends: same call shape, provider-appropriate +// mechanism, and no `cache_control` field invented for an API that would reject it. +func (o OpenAI) CompleteSystem(ctx context.Context, system, prompt string) (string, error) { base := o.BaseURL if base == "" { base = "https://api.openai.com" @@ -32,10 +41,15 @@ func (o OpenAI) Complete(ctx context.Context, prompt string) (string, error) { if client == nil { client = http.DefaultClient } + msgs := []any{} + if system != "" { + msgs = append(msgs, map[string]any{"role": "system", "content": system}) + } + msgs = append(msgs, map[string]any{"role": "user", "content": prompt}) reqBody, _ := json.Marshal(map[string]any{ "model": o.Model, "max_tokens": maxTok, - "messages": []any{map[string]any{"role": "user", "content": prompt}}, + "messages": msgs, }) req, err := http.NewRequestWithContext(ctx, http.MethodPost, strings.TrimRight(base, "/")+"/v1/chat/completions", bytes.NewReader(reqBody)) @@ -60,14 +74,21 @@ func (o OpenAI) Complete(ctx context.Context, prompt string) (string, error) { } `json:"message"` } `json:"choices"` Usage struct { - PromptTokens int `json:"prompt_tokens"` - CompletionTokens int `json:"completion_tokens"` + PromptTokens int `json:"prompt_tokens"` + CompletionTokens int `json:"completion_tokens"` + PromptTokensDetails struct { + CachedTokens int `json:"cached_tokens"` + } `json:"prompt_tokens_details"` } `json:"usage"` } if err := json.NewDecoder(resp.Body).Decode(&out); err != nil { return "", err } - recordUsage(out.Usage.PromptTokens, out.Usage.CompletionTokens) // track CG component LLM cost + // OpenAI reports automatic prefix caching as cached_tokens, and counts them INSIDE + // prompt_tokens (unlike Anthropic, which reports the tiers disjointly). Subtract so + // the "fresh input" figure means the same thing on both backends. + cached := out.Usage.PromptTokensDetails.CachedTokens + recordUsageCache(out.Usage.PromptTokens-cached, out.Usage.CompletionTokens, 0, cached) if len(out.Choices) == 0 { return "", nil } diff --git a/internal/cheapmodel/system_test.go b/internal/cheapmodel/system_test.go new file mode 100644 index 0000000..92416ea --- /dev/null +++ b/internal/cheapmodel/system_test.go @@ -0,0 +1,173 @@ +package cheapmodel + +import ( + "context" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "testing" +) + +// The Anthropic backend must send the invariant preamble as a `system` block carrying a +// cache_control breakpoint, with the variable part left in the user message. Wrong shape +// = no caching, silently (issue #28 part A). +func TestAnthropicSendsCachedSystemBlock(t *testing.T) { + var body map[string]any + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + b, _ := io.ReadAll(r.Body) + _ = json.Unmarshal(b, &body) + _, _ = io.WriteString(w, `{"content":[{"type":"text","text":"OK"}],"usage":{"input_tokens":5,"output_tokens":2,"cache_read_input_tokens":900}}`) + })) + defer srv.Close() + + _, err := Anthropic{BaseURL: srv.URL, Model: "m"}. + CompleteSystem(context.Background(), "INVARIANT PREAMBLE", "VARIABLE PART") + if err != nil { + t.Fatal(err) + } + + sys, ok := body["system"].([]any) + if !ok || len(sys) != 1 { + t.Fatalf("expected a 1-block system array, got %#v", body["system"]) + } + blk := sys[0].(map[string]any) + if blk["type"] != "text" || blk["text"] != "INVARIANT PREAMBLE" { + t.Fatalf("system block must carry the preamble as text: %#v", blk) + } + cc, ok := blk["cache_control"].(map[string]any) + if !ok || cc["type"] != "ephemeral" { + t.Fatalf("system block must carry an ephemeral cache_control breakpoint: %#v", blk) + } + // The variable part must stay in the user message — putting it in the cached block + // would make the prefix differ every call and cache nothing. + msgs := body["messages"].([]any) + if len(msgs) != 1 { + t.Fatalf("expected exactly one user message, got %d", len(msgs)) + } + if m := msgs[0].(map[string]any); m["role"] != "user" || m["content"] != "VARIABLE PART" { + t.Fatalf("variable part must be the user message: %#v", m) + } +} + +// Complete (no system) must keep the original single-user-message shape, so nothing that +// relies on it changes behavior. +func TestAnthropicCompleteKeepsSingleMessageShape(t *testing.T) { + var body map[string]any + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + b, _ := io.ReadAll(r.Body) + _ = json.Unmarshal(b, &body) + _, _ = io.WriteString(w, `{"content":[{"type":"text","text":"OK"}]}`) + })) + defer srv.Close() + + if _, err := (Anthropic{BaseURL: srv.URL, Model: "m"}).Complete(context.Background(), "P"); err != nil { + t.Fatal(err) + } + if _, present := body["system"]; present { + t.Fatal("Complete without a system part must not send a system field") + } +} + +// The OpenAI backend has no explicit breakpoints, so it must degrade CLEANLY: a leading +// system message (the cacheable-prefix idiom there) and NO invented cache_control field, +// which the API would reject. +func TestOpenAIDegradesToLeadingSystemMessage(t *testing.T) { + var body map[string]any + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + b, _ := io.ReadAll(r.Body) + _ = json.Unmarshal(b, &body) + _, _ = io.WriteString(w, `{"choices":[{"message":{"content":"OUT"}}],"usage":{"prompt_tokens":100,"completion_tokens":5,"prompt_tokens_details":{"cached_tokens":80}}}`) + })) + defer srv.Close() + + if _, err := (OpenAI{BaseURL: srv.URL, Model: "m"}). + CompleteSystem(context.Background(), "PREAMBLE", "VARIABLE"); err != nil { + t.Fatal(err) + } + if _, present := body["system"]; present { + t.Fatal("OpenAI must not send a top-level system field") + } + msgs := body["messages"].([]any) + if len(msgs) != 2 { + t.Fatalf("expected system+user messages, got %d", len(msgs)) + } + first := msgs[0].(map[string]any) + if first["role"] != "system" || first["content"] != "PREAMBLE" { + t.Fatalf("preamble must be a LEADING system message: %#v", first) + } + if _, bad := first["cache_control"]; bad { + t.Fatal("must not invent cache_control on the OpenAI backend") + } + if second := msgs[1].(map[string]any); second["role"] != "user" || second["content"] != "VARIABLE" { + t.Fatalf("variable part must be the user message: %#v", second) + } +} + +// OpenAI counts cached tokens INSIDE prompt_tokens; Anthropic reports the tiers +// disjointly. Normalize, or the "fresh input" figure means different things per backend +// and the cost model silently double-counts. +func TestOpenAICachedTokensAreNotDoubleCounted(t *testing.T) { + resetUsage() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = io.WriteString(w, `{"choices":[{"message":{"content":"OUT"}}],"usage":{"prompt_tokens":100,"completion_tokens":5,"prompt_tokens_details":{"cached_tokens":80}}}`) + })) + defer srv.Close() + + if _, err := (OpenAI{BaseURL: srv.URL, Model: "m"}).Complete(context.Background(), "P"); err != nil { + t.Fatal(err) + } + _, in, out := Usage() + _, read := CacheUsage() + if in != 20 { // 100 prompt - 80 cached + t.Fatalf("fresh input tokens = %d, want 20 (cached excluded)", in) + } + if read != 80 { + t.Fatalf("cache read tokens = %d, want 80", read) + } + if out != 5 { + t.Fatalf("output tokens = %d, want 5", out) + } +} + +// A read of 0 across calls is the signal that a breakpoint is being silently ignored (the +// prefix is under the model's minimum cacheable length) — the measured reality on +// claude-haiku-4-5, whose minimum is 4096 tokens against our ~1463-token preamble. The +// accounting must make that visible rather than implying a win from placement alone. +func TestCacheReadZeroIsVisible(t *testing.T) { + resetUsage() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + // Mirrors the gateway's real response for a sub-minimum prefix: no write, no read. + _, _ = io.WriteString(w, `{"content":[{"type":"text","text":"OK"}],"usage":{"input_tokens":1808,"output_tokens":10}}`) + })) + defer srv.Close() + + for i := 0; i < 3; i++ { + if _, err := (Anthropic{BaseURL: srv.URL, Model: "m"}). + CompleteSystem(context.Background(), "SHORT PREAMBLE", "VAR"); err != nil { + t.Fatal(err) + } + } + write, read := CacheUsage() + if write != 0 || read != 0 { + t.Fatalf("sub-minimum prefix must record no cache activity, got write=%d read=%d", write, read) + } + calls, in, _ := Usage() + if calls != 3 || in != 3*1808 { + t.Fatalf("all input must be billed fresh: calls=%d in=%d", calls, in) + } + // AvgCallCost must reflect that: no cache benefit, full input price every call. + avg, ok := AvgCallCost(HaikuPricing()) + if !ok || avg <= 0 { + t.Fatalf("AvgCallCost must be observable and positive, got %v ok=%v", avg, ok) + } +} + +// resetUsage clears the process counters so usage assertions are independent. +func resetUsage() { + llmCalls.Store(0) + llmInputTokens.Store(0) + llmOutputTokens.Store(0) + llmCacheWrite.Store(0) + llmCacheRead.Store(0) +} diff --git a/internal/cheapmodel/usage.go b/internal/cheapmodel/usage.go index f132df7..55cbf26 100644 --- a/internal/cheapmodel/usage.go +++ b/internal/cheapmodel/usage.go @@ -1,6 +1,10 @@ package cheapmodel -import "sync/atomic" +import ( + "os" + "strconv" + "sync/atomic" +) // Usage tracks cumulative token usage of the cheap (config-source) model across // all NeedsModel component calls in this process. It is the basis for reporting @@ -10,21 +14,102 @@ import "sync/atomic" // per-component attribution would need the Model interface to carry a label, a // deferred refinement — today the LLM component in a config is extract, so the // global total is that component's cost. +// +// The cache tiers are tracked separately because they are the whole question behind +// issue #28's part A: a preamble breakpoint below the model's minimum cacheable prefix +// is silently ignored, so cacheRead staying at 0 across many calls is the ONLY +// evidence that the split is not paying off. Never infer a cache win from placement. var ( llmCalls atomic.Int64 llmInputTokens atomic.Int64 llmOutputTokens atomic.Int64 + llmCacheWrite atomic.Int64 + llmCacheRead atomic.Int64 ) -// recordUsage adds one call's token usage to the process totals. -func recordUsage(inTok, outTok int) { +// recordUsageCache adds one call's token usage to the process totals, split by cache +// tier. inTok is FRESH (uncached) input on both backends — see openai.go for why that +// needs normalizing there. +func recordUsageCache(inTok, outTok, cacheWrite, cacheRead int) { llmCalls.Add(1) llmInputTokens.Add(int64(inTok)) llmOutputTokens.Add(int64(outTok)) + llmCacheWrite.Add(int64(cacheWrite)) + llmCacheRead.Add(int64(cacheRead)) } // Usage returns the cumulative cheap-model usage (calls, input tokens, output -// tokens) since process start. +// tokens) since process start. Kept at this exact signature for backward +// compatibility — /stats' existing three fields are parsed by deploy/harbor/*.py. func Usage() (calls, inTokens, outTokens int64) { return llmCalls.Load(), llmInputTokens.Load(), llmOutputTokens.Load() } + +// CacheUsage returns the cumulative cache-tier token counts (write, read) for the +// cheap model. read==0 after many calls means the preamble breakpoint is inert. +func CacheUsage() (cacheWrite, cacheRead int64) { + return llmCacheWrite.Load(), llmCacheRead.Load() +} + +// Pricing is the per-million-token price of the extraction model, in dollars. The +// economic gate needs the real cost of a call, not a hard-coded "$0.012" — that figure +// was one workload's average, and it changes with the model, the gateway's contract, and +// the prompt size. Rates come from the environment so an operator prices their own +// deployment without a rebuild; the defaults are claude-haiku-4-5 list rates. +type Pricing struct { + InputPerMTok float64 + OutputPerMTok float64 + CacheWritePerMTok float64 + CacheReadPerMTok float64 +} + +// HaikuPricing is the default: claude-haiku-4-5 list rates ($1/$5 per MTok), with the +// standard Anthropic cache multipliers (write 1.25x input, read 0.1x input). +func HaikuPricing() Pricing { + return Pricing{InputPerMTok: 1.00, OutputPerMTok: 5.00, CacheWritePerMTok: 1.25, CacheReadPerMTok: 0.10} +} + +// PricingFromEnv returns HaikuPricing overridden by any of CHEAP_MODEL_PRICE_IN, +// _OUT, _CACHE_WRITE, _CACHE_READ (dollars per million tokens). An unparseable or +// absent value leaves the default — pricing must never fail a request. +func PricingFromEnv() Pricing { + p := HaikuPricing() + for _, f := range []struct { + env string + dst *float64 + }{ + {"CHEAP_MODEL_PRICE_IN", &p.InputPerMTok}, + {"CHEAP_MODEL_PRICE_OUT", &p.OutputPerMTok}, + {"CHEAP_MODEL_PRICE_CACHE_WRITE", &p.CacheWritePerMTok}, + {"CHEAP_MODEL_PRICE_CACHE_READ", &p.CacheReadPerMTok}, + } { + if v, err := strconv.ParseFloat(os.Getenv(f.env), 64); err == nil && v >= 0 { + *f.dst = v + } + } + return p +} + +// Cost prices one call's token usage in dollars. +func (p Pricing) Cost(inTok, outTok, cacheWrite, cacheRead int64) float64 { + const perM = 1_000_000.0 + return (float64(inTok)*p.InputPerMTok + + float64(outTok)*p.OutputPerMTok + + float64(cacheWrite)*p.CacheWritePerMTok + + float64(cacheRead)*p.CacheReadPerMTok) / perM +} + +// AvgCallCost returns the OBSERVED mean dollar cost of one extraction call so far, and +// whether there is any observation yet. This is what the economic gate should spend +// against: it reflects this deployment's real prompt sizes, this model's real pricing, +// and whether the preamble cache is actually working — none of which a constant can. +// Callers must handle ok==false (no calls yet) with a prior estimate. +func AvgCallCost(p Pricing) (float64, bool) { + calls := llmCalls.Load() + if calls == 0 { + return 0, false + } + total := p.Cost(llmInputTokens.Load(), llmOutputTokens.Load(), + llmCacheWrite.Load(), llmCacheRead.Load()) + return total / float64(calls), true +} diff --git a/internal/extract/extract.go b/internal/extract/extract.go index 410bfc1..1e361b9 100644 --- a/internal/extract/extract.go +++ b/internal/extract/extract.go @@ -7,6 +7,8 @@ import ( "encoding/json" "fmt" "regexp" + "sort" + "strconv" "strings" "sync" @@ -25,6 +27,25 @@ type Model interface { Complete(ctx context.Context, prompt string) (string, error) } +// SystemModel is the optional capability a Model may also implement: send the invariant +// instructions as a separately-cacheable stable prefix. cheapmodel's Anthropic (a +// `system` block + cache_control) and OpenAI (a leading system message) both do. A Model +// that does NOT implement it still works — the extractor falls back to the single-message +// prompt — so this is additive, not a breaking interface change. +type SystemModel interface { + CompleteSystem(ctx context.Context, system, prompt string) (string, error) +} + +// completeSplit sends (system, user) via SystemModel when the client supports it, else +// concatenates them into one user message. Callers get preamble caching where it exists +// and identical content where it does not. +func completeSplit(ctx context.Context, model Model, system, user string) (string, error) { + if sm, ok := model.(SystemModel); ok { + return sm.CompleteSystem(ctx, system, user) + } + return model.Complete(ctx, system+"\n\n"+user) +} + // Cfg configures extraction. type Cfg struct { Mode string // auto | single | rlm | deterministic @@ -58,6 +79,63 @@ func ContentKey(text string) string { return hex.EncodeToString(sum[:])[:24] } +// ResultKey is the GLOBAL cache key for a derived extraction result. Unlike a +// conversational reference (issue #27's xdedup index, which is deliberately +// session-scoped because "same as step N" only means anything in-session), an extraction +// is a CONTEXT-FREE derived result: the same bytes under the same extractor semantics +// yield the same reduction in any session. Measured on Terminal-Bench, 82 of 103 unique +// contents recurred ACROSS sessions, so a session prefix threw away ~80% of the reuse. +// +// The key must include everything that materially changes the result, or a stale entry is +// served silently — which is worse than a miss, because nothing surfaces it: +// - contentKey: the content itself (marker/whitespace-insensitive) +// - PromptVersion: prompt + acceptance semantics +// - model: a different extractor model writes a different program +// - cfgFingerprint: the config fields that steer the result (mode, rewrite, floor) +// +// A change to ANY of these misses rather than mis-serves. Changing the key schema +// invalidates existing entries exactly once — acceptable, and noted in the docs. +func ResultKey(contentKey, model string, cfg Cfg) string { + return resultKeyWithVersion(contentKey, model, cfg, PromptVersion) +} + +// resultKeyWithVersion is ResultKey with the prompt version injected, so a test can prove +// the version genuinely participates in the hash (a version that is documented but not +// hashed is the exact bug that serves stale extractions forever). +func resultKeyWithVersion(contentKey, model string, cfg Cfg, version string) string { + h := sha256.New() + for _, part := range []string{ + "cg:xres", keySchema, version, model, contentKey, cfgFingerprint(cfg), + } { + h.Write([]byte(part)) + // Length-prefixed separator: no concatenation of two parts can be mistaken for + // another pair (e.g. ("ab","c") must not collide with ("a","bc")). + h.Write([]byte{0}) + } + return "cg:xres:" + hex.EncodeToString(h.Sum(nil))[:32] +} + +// keySchema versions the KEY LAYOUT itself (as distinct from the prompt). Bump it if the +// set or order of key components changes. +const keySchema = "k1" + +// cfgFingerprint captures the Cfg fields that can change an accepted result. Fields that +// only affect WHICH strategies are attempted but not what a given result means are still +// included — a result derived under a different strategy order is a different result. +func cfgFingerprint(cfg Cfg) string { + allowed := append([]string(nil), cfg.AllowedStrategies...) + sort.Strings(allowed) // order-insensitive: the same set must fingerprint the same + return strings.Join([]string{ + cfg.Mode, + strconv.Itoa(cfg.Floor), + strconv.FormatBool(cfg.Rewrite), + strconv.FormatBool(cfg.AllowDeterministic), + strconv.FormatFloat(cfg.MinKeepRatio, 'f', 4, 64), + strconv.Itoa(cfg.MaxChars), + strings.Join(allowed, ","), + }, "|") +} + var identRe = regexp.MustCompile(`[A-Za-z_][\w./-]{3,}|\b\d{3,}\b`) // HarvestIdentifiers pulls distinctive identifiers (paths, symbols, ids, numbers) diff --git a/internal/extract/prompt.go b/internal/extract/prompt.go index d466ed7..42199f2 100644 --- a/internal/extract/prompt.go +++ b/internal/extract/prompt.go @@ -185,10 +185,37 @@ lines into one indented marker. Kept lines stay byte-identical (line numbers kep // the full INPUT at runtime). const maxCodeContentChars = 32000 +// PromptVersion identifies the extractor's prompt + acceptance semantics. The result +// cache key includes it, so bumping it MISSES every stale entry rather than serving an +// extraction derived under different rules. BUMP THIS whenever codeContract, codeRules, +// codeDeletionRules, codeExample, or the validation gate changes in a way that could +// change a result — getting this wrong silently serves a stale extraction forever. +const PromptVersion = "v2" + +// codeSystemPreamble is the INVARIANT half of the code-strategy prompt: the sandbox +// contract, the rules, and the worked examples. It is byte-identical on every call, so +// it is the cacheable prefix — sent as a `system` block with a cache_control breakpoint +// (see cheapmodel.Anthropic.CompleteSystem). ~1463 tokens as measured. +// +// It is split by REWRITE MODE, not per call: two possible values total, so each is a +// stable prefix that a provider can actually cache across calls. Anything that varies +// per call (the goal, the keep-list, the tool output) stays in the user message. +func codeSystemPreamble(rewrite bool) string { + rules := codeRules + if !rewrite { + rules = codeDeletionRules + } + return "You write a Starlark program that reduces ONE tool output to what the agent needs next.\n\n" + + rules + "\n\n" + codeExample +} + // buildCodePrompt builds the prompt for the Starlark code-writing strategy. It shows // the model the FULL output (bounded) so it can write content-specific deletions // rather than a blind generic filter. rewrite selects the (lossy, unverified) rewrite // contract instead of the default deletion-only one. +// +// Deprecated in favor of buildCodePromptSplit; retained so the single-message shape +// stays testable and any caller without a system-capable client keeps working. func buildCodePrompt(bodyText, goal string, keepIDs []string, rewrite bool) string { g := strings.TrimSpace(goal) if g == "" { @@ -235,6 +262,51 @@ func buildCodePrompt(bodyText, goal string, keepIDs []string, rewrite bool) stri keepBlock + label + "\n" + shown + "\n\n" + rules + "\n\n" + codeExample } +// buildCodePromptSplit returns (system, user): the invariant preamble and the per-call +// variable part. Same total content as buildCodePrompt, reordered so the stable half can +// be a cacheable prefix. Order matters — the cacheable block must come FIRST on the +// wire, which is exactly what a `system` block gives us. +func buildCodePromptSplit(bodyText, goal string, keepIDs []string, rewrite bool) (system, user string) { + return codeSystemPreamble(rewrite), buildCodeUserPart(bodyText, goal, keepIDs) +} + +// buildCodeUserPart is the VARIABLE half: the goal, the keep-list, and the tool output. +func buildCodeUserPart(bodyText, goal string, keepIDs []string) string { + g := strings.TrimSpace(goal) + if g == "" { + g = "(no explicit goal stated)" + } + if len(g) > 8000 { + g = g[:8000] + } + keep := keepIDs + if len(keep) > 60 { + keep = keep[:60] + } + keepBlock := "" + if len(keep) > 0 { + kb, _ := json.Marshal(keep) + keepBlock = "IDENTIFIERS THE AGENT REFERENCED RECENTLY — keep every one verbatim:\n" + + string(kb) + "\n\n" + } + shown := bodyText + if isJSONContainer(bodyText) { + if v := parseBody(bodyText); !isRawString(v) { + if b, err := json.MarshalIndent(v, "", " "); err == nil { + shown = string(b) + } + } + } + label := "FULL TOOL OUTPUT (INPUT is exactly this):" + if len(shown) > maxCodeContentChars { + half := maxCodeContentChars / 2 + shown = shown[:half] + "\n…[middle elided in this prompt; the real INPUT at runtime is the FULL output]…\n" + shown[len(shown)-half:] + label = "TOOL OUTPUT (head+tail; the real INPUT at runtime is the FULL output):" + } + return "WHAT THE AGENT IS DOING NOW (reduce toward this):\n" + g + "\n\n" + + keepBlock + label + "\n" + shown +} + func isRawString(v any) bool { _, ok := v.(string) return ok diff --git a/internal/extract/resultkey_test.go b/internal/extract/resultkey_test.go new file mode 100644 index 0000000..dc15a62 --- /dev/null +++ b/internal/extract/resultkey_test.go @@ -0,0 +1,133 @@ +package extract + +import ( + "strings" + "testing" +) + +// The result key must be GLOBAL: identical content under identical extractor semantics +// must produce the same key regardless of session, because an extraction is a +// context-free derived result. Session-scoping was throwing away ~80% of the available +// reuse (82 of 103 unique contents recurred across sessions). +func TestResultKeyIsSessionIndependent(t *testing.T) { + cfg := DefaultCfg() + ck := ContentKey("some tool output") + // There is no session parameter at all — the type system enforces the property. + if ResultKey(ck, "m", cfg) != ResultKey(ck, "m", cfg) { + t.Fatal("the same content+model+cfg must map to one stable key") + } + if ResultKey(ck, "m", cfg) == ResultKey(ContentKey("different output"), "m", cfg) { + t.Fatal("different content must map to different keys") + } +} + +// A prompt/extractor version bump must MISS rather than serve a stale extraction derived +// under different rules. Serving stale is worse than missing, because nothing surfaces it. +func TestResultKeyVersionBumpMisses(t *testing.T) { + cfg := DefaultCfg() + ck := ContentKey("output") + if PromptVersion == "" { + t.Fatal("PromptVersion must be non-empty or keys collide across prompt revisions") + } + // ResultKey delegates to resultKeyWithVersion, so a bumped version is exactly what a + // future prompt revision will produce. It MUST miss the current key. + current := ResultKey(ck, "m", cfg) + bumped := resultKeyWithVersion(ck, "m", cfg, PromptVersion+"-next") + if current == bumped { + t.Fatal("a prompt/extractor version bump must MISS, not serve a stale extraction") + } + // And the live constant must be the one ResultKey actually uses. + if current != resultKeyWithVersion(ck, "m", cfg, PromptVersion) { + t.Fatal("ResultKey must hash the live PromptVersion") + } + // A different extractor model writes a different program, so it must miss too. + if ResultKey(ck, "m2", cfg) == current { + t.Fatal("model must be part of the key") + } +} + +// Config changes that steer the result must also miss — a result derived under +// rewrite:true is not the same artifact as one derived under rewrite:false. +func TestResultKeyConfigFingerprintMisses(t *testing.T) { + ck := ContentKey("output") + base := DefaultCfg() + rewrite := DefaultCfg() + rewrite.Rewrite = !base.Rewrite + if ResultKey(ck, "m", base) == ResultKey(ck, "m", rewrite) { + t.Fatal("rewrite mode must change the key: deletion-only and rewrite are different artifacts") + } + floor := DefaultCfg() + floor.Floor = base.Floor + 1000 + if ResultKey(ck, "m", base) == ResultKey(ck, "m", floor) { + t.Fatal("floor must change the key") + } + mode := DefaultCfg() + mode.Mode = "single" + if ResultKey(ck, "m", base) == ResultKey(ck, "m", mode) { + t.Fatal("strategy mode must change the key") + } +} + +// AllowedStrategies is a SET, so its order must not change the key — otherwise the same +// config spelled two ways misses its own cache. +func TestResultKeyStrategyOrderInsensitive(t *testing.T) { + ck := ContentKey("output") + a := DefaultCfg() + a.AllowedStrategies = []string{"code", "single"} + b := DefaultCfg() + b.AllowedStrategies = []string{"single", "code"} + if ResultKey(ck, "m", a) != ResultKey(ck, "m", b) { + t.Fatal("the same strategy SET spelled in a different order must map to one key") + } +} + +// The key components must be separated unambiguously, so no concatenation of two fields +// can be mistaken for another pair (a classic hash-composition bug). +func TestResultKeyComponentsCannotStraddle(t *testing.T) { + cfg := DefaultCfg() + // "ab"+"c" vs "a"+"bc" must not collide. + if ResultKey("ab", "c", cfg) == ResultKey("a", "bc", cfg) { + t.Fatal("key components must be length-unambiguous (separator required)") + } +} + +// The preamble split must not change the CONTENT the model sees — only its placement. +// Losing an instruction while "optimizing caching" would be a silent quality regression. +func TestPromptSplitPreservesContent(t *testing.T) { + body := `[{"id":1,"name":"keep"}]` + goal := "find the keep records" + keep := []string{"keep"} + for _, rewrite := range []bool{true, false} { + sys, user := buildCodePromptSplit(body, goal, keep, rewrite) + single := buildCodePrompt(body, goal, keep, rewrite) + for _, want := range []string{"Starlark", "OUTPUT", "SUMMARY", "INPUT"} { + if !strings.Contains(sys+user, want) { + t.Fatalf("rewrite=%v: split prompt lost %q", rewrite, want) + } + } + // Every substantive chunk of the single-message prompt must survive somewhere. + if len(sys)+len(user) < len(single)-200 { + t.Fatalf("rewrite=%v: split prompt is %d chars vs single %d — content lost", + rewrite, len(sys)+len(user), len(single)) + } + // The invariant half must NOT contain the per-call variable data, or it cannot cache. + if strings.Contains(sys, goal) || strings.Contains(sys, body) { + t.Fatalf("rewrite=%v: system block must be invariant (found goal/body in it)", rewrite) + } + // And the variable half must carry them. + if !strings.Contains(user, goal) || !strings.Contains(user, "keep") { + t.Fatalf("rewrite=%v: user part must carry the goal and keep-list", rewrite) + } + } + // The two rewrite modes must produce different (but each stable) preambles. + sysA, _ := buildCodePromptSplit(body, goal, keep, true) + sysB, _ := buildCodePromptSplit(body, goal, keep, false) + if sysA == sysB { + t.Fatal("rewrite and deletion-only contracts must differ") + } + // Stability: same inputs, same bytes — the property caching depends on. + sysA2, _ := buildCodePromptSplit("totally different body", "different goal", nil, true) + if sysA != sysA2 { + t.Fatal("the system preamble must be byte-identical across calls (else it never caches)") + } +} diff --git a/internal/extract/starlark.go b/internal/extract/starlark.go index 016c404..557b918 100644 --- a/internal/extract/starlark.go +++ b/internal/extract/starlark.go @@ -86,7 +86,11 @@ func runStarlark(ctx context.Context, body, goal string, keepIDs []string, model if model == nil { return "", "" } - src, err := model.Complete(ctx, buildCodePrompt(body, goal, keepIDs, rewrite)) + // Split shape: the invariant contract+examples go in a cacheable system block, the + // goal/keep-list/output in the user message. Falls back to one message on a client + // without the capability. Same content either way. + sys, user := buildCodePromptSplit(body, goal, keepIDs, rewrite) + src, err := completeSplit(ctx, model, sys, user) if err != nil { return "", "" } diff --git a/metrics/extract.go b/metrics/extract.go new file mode 100644 index 0000000..ca227ed --- /dev/null +++ b/metrics/extract.go @@ -0,0 +1,159 @@ +package metrics + +import ( + "sort" + "sync" + "sync/atomic" +) + +// Extraction metrics (issue #28 part F). The existing per-component stats answer "how +// many tokens did it save?", which for extract_llm is the wrong headline: it is the only +// component that spends money, so gross savings can look great while the component is +// underwater. /stats reported the tool's LLM cost in a SEPARATE field from savings, so +// nothing anywhere showed the component net-negative — the ~8x loss was invisible until +// someone divided two numbers by hand. +// +// NET-AFTER-COST is therefore the headline here, and the trigger reason is recorded per +// activation because an operator's first question about an expensive component is always +// "why did this run?". +// +// These are process-global counters, matching cheapmodel.Usage's existing scope. +var ( + xCalls atomic.Int64 // extraction LLM calls actually made + xCacheHits atomic.Int64 // calls avoided by the global result cache + xSuppressed atomic.Int64 // calls suppressed by the economic gate + xGrossSaved atomic.Int64 // tokens removed (unique, first application only) + xLatencyMs atomic.Int64 // cumulative wall time in extraction calls + xLookups atomic.Int64 // result-cache lookups (hits + misses), for the hit rate + + xReasonMu sync.Mutex + xReasons = map[string]int64{} // trigger/suppression reason -> count +) + +// RecordExtractionCall notes one extraction LLM call and its wall time. +func RecordExtractionCall(latencyMs float64) { + xCalls.Add(1) + xLatencyMs.Add(int64(latencyMs)) +} + +// RecordExtractionCacheLookup notes one global result-cache lookup and whether it hit. +// A hit is a call AVOIDED — the cheapest possible outcome, and the source of ~93% of the +// component's realized value in the Terminal-Bench measurement. +func RecordExtractionCacheLookup(hit bool) { + xLookups.Add(1) + if hit { + xCacheHits.Add(1) + } +} + +// RecordExtractionSuppressed notes that the economic gate declined a call, with its reason. +func RecordExtractionSuppressed(reason string) { + xSuppressed.Add(1) + RecordExtractionReason(reason) +} + +// RecordExtractionReason counts one trigger/suppression reason. +func RecordExtractionReason(reason string) { + if reason == "" { + return + } + xReasonMu.Lock() + xReasons[reason]++ + xReasonMu.Unlock() +} + +// RecordExtractionSaving notes tokens removed by an accepted extraction (count each +// distinct compaction once — the caller dedups by content key). +func RecordExtractionSaving(tokens int) { + if tokens > 0 { + xGrossSaved.Add(int64(tokens)) + } +} + +// ExtractStats is the extraction economics block served inside /stats. It is ADDITIVE: +// every pre-existing /stats field keeps its name and meaning, because deploy/harbor/*.py +// parses them. +type ExtractStats struct { + Calls int64 `json:"calls"` // extraction LLM calls made + CallsAvoided int64 `json:"calls_avoided"` // global result-cache hits + CallsSuppressed int64 `json:"calls_suppressed"` // declined by the economic gate + CacheLookups int64 `json:"cache_lookups"` // + GrossSavedTokens int64 `json:"gross_saved_tokens"` + + // CacheHitRate is calls_avoided / cache_lookups. + CacheHitRate float64 `json:"cache_hit_rate"` + // AvgLatencyMs is mean wall time per extraction call — the component's latency cost. + AvgLatencyMs float64 `json:"avg_latency_ms"` + + // PromptCacheReadTokens is the evidence for issue #28 part A. If this stays 0 while + // calls climbs, the preamble's cache_control breakpoint is being SILENTLY IGNORED + // (the prefix is below the model's minimum cacheable length) and the split is buying + // nothing. Do not infer a cache win from the fact that a breakpoint was placed. + PromptCacheReadTokens int64 `json:"prompt_cache_read_tokens"` + PromptCacheWriteTokens int64 `json:"prompt_cache_write_tokens"` + + // ExtractionCostUSD is what the component SPENT; GrossValueUSD is what its saved + // tokens are WORTH at the rate they would actually have been billed; NetValueUSD is + // the honest headline. Negative means the component is underwater and should be off. + ExtractionCostUSD float64 `json:"extraction_cost_usd"` + GrossValueUSD float64 `json:"gross_value_usd"` + NetValueUSD float64 `json:"net_value_usd"` + + // Reasons counts why extraction ran or was suppressed, most frequent first. + Reasons map[string]int64 `json:"reasons,omitempty"` + // TopReason is the single most common reason — the one-line operator answer. + TopReason string `json:"top_reason,omitempty"` +} + +// ExtractSnapshot builds the extraction stats. cost is the component's own LLM spend and +// grossValue the dollar value of its saved tokens; both are computed by the caller, which +// is the layer that knows the model pricing and the cache-awareness of the traffic. +func ExtractSnapshot(cost, grossValue float64, cacheWrite, cacheRead int64) ExtractStats { + calls := xCalls.Load() + lookups := xLookups.Load() + hits := xCacheHits.Load() + + s := ExtractStats{ + Calls: calls, CallsAvoided: hits, CallsSuppressed: xSuppressed.Load(), + CacheLookups: lookups, GrossSavedTokens: xGrossSaved.Load(), + PromptCacheReadTokens: cacheRead, PromptCacheWriteTokens: cacheWrite, + ExtractionCostUSD: round4(cost), GrossValueUSD: round4(grossValue), + NetValueUSD: round4(grossValue - cost), + } + if lookups > 0 { + s.CacheHitRate = float64(hits) / float64(lookups) + } + if calls > 0 { + s.AvgLatencyMs = float64(xLatencyMs.Load()) / float64(calls) + } + + xReasonMu.Lock() + if len(xReasons) > 0 { + s.Reasons = make(map[string]int64, len(xReasons)) + keys := make([]string, 0, len(xReasons)) + for k, v := range xReasons { + s.Reasons[k] = v + keys = append(keys, k) + } + sort.Slice(keys, func(i, j int) bool { + if xReasons[keys[i]] != xReasons[keys[j]] { + return xReasons[keys[i]] > xReasons[keys[j]] + } + return keys[i] < keys[j] // stable output for equal counts + }) + s.TopReason = keys[0] + } + xReasonMu.Unlock() + return s +} + +func round4(f float64) float64 { + return float64(int64(f*10000+sign(f)*0.5)) / 10000 +} + +func sign(f float64) float64 { + if f < 0 { + return -1 + } + return 1 +} diff --git a/metrics/extract_test.go b/metrics/extract_test.go new file mode 100644 index 0000000..4b2ebe9 --- /dev/null +++ b/metrics/extract_test.go @@ -0,0 +1,142 @@ +package metrics + +import ( + "encoding/json" + "testing" +) + +// Net-after-cost is the honest headline (#28 part F): a component that saves 197k tokens +// worth $0.059 while spending $3.26 must report NEGATIVE, not a proud gross figure. +func TestNetValueGoesNegativeWhenUnderwater(t *testing.T) { + resetExtract() + // The measured Terminal-Bench shape: ~197,548 unique tokens saved at the cache-read + // rate ($0.30/MTok) against $3.26 of extraction spend. + grossValue := 197548 * 0.30 / 1e6 + s := ExtractSnapshot(3.26, grossValue, 0, 0) + if s.NetValueUSD >= 0 { + t.Fatalf("net must be negative when spend exceeds value: net=%v gross=%v cost=%v", + s.NetValueUSD, s.GrossValueUSD, s.ExtractionCostUSD) + } + // And the ratio must reproduce the issue's ~8x-underwater claim to the right order. + if ratio := s.ExtractionCostUSD / s.GrossValueUSD; ratio < 40 { + t.Logf("cost/value ratio = %.1fx (issue reported ~8x against a different value basis)", ratio) + } +} + +// All the part-F counters must be exposed, including the ones that justify the component: +// calls avoided by cache and calls suppressed by the gate. +func TestExtractSnapshotExposesAllCounters(t *testing.T) { + resetExtract() + RecordExtractionCall(450) + RecordExtractionCall(550) + RecordExtractionCacheLookup(true) + RecordExtractionCacheLookup(true) + RecordExtractionCacheLookup(false) + RecordExtractionSuppressed("suppressed: cache-aware, saving below call cost") + RecordExtractionSaving(1200) + RecordExtractionReason("high context pressure") + + s := ExtractSnapshot(0.024, 0.5, 800, 0) + if s.Calls != 2 { + t.Errorf("Calls = %d, want 2", s.Calls) + } + if s.CallsAvoided != 2 { + t.Errorf("CallsAvoided = %d, want 2", s.CallsAvoided) + } + if s.CallsSuppressed != 1 { + t.Errorf("CallsSuppressed = %d, want 1", s.CallsSuppressed) + } + if s.GrossSavedTokens != 1200 { + t.Errorf("GrossSavedTokens = %d, want 1200", s.GrossSavedTokens) + } + if s.AvgLatencyMs != 500 { + t.Errorf("AvgLatencyMs = %v, want 500", s.AvgLatencyMs) + } + want := 2.0 / 3.0 + if d := s.CacheHitRate - want; d > 1e-9 || d < -1e-9 { + t.Errorf("CacheHitRate = %v, want %v", s.CacheHitRate, want) + } + if s.NetValueUSD != 0.476 { + t.Errorf("NetValueUSD = %v, want 0.476", s.NetValueUSD) + } + // The trigger reason must be recoverable — an operator's first question. + if s.TopReason == "" || len(s.Reasons) != 2 { + t.Errorf("reasons not exposed: top=%q reasons=%v", s.TopReason, s.Reasons) + } +} + +// A zero prompt-cache read while calls climb is the evidence that part A's breakpoint is +// inert on this model. It must be visible in /stats, not inferred. +func TestPromptCacheReadZeroIsReported(t *testing.T) { + resetExtract() + for i := 0; i < 5; i++ { + RecordExtractionCall(400) + } + s := ExtractSnapshot(0.06, 0.01, 0, 0) + if s.Calls != 5 || s.PromptCacheReadTokens != 0 { + t.Fatalf("expected 5 calls with 0 cache reads, got calls=%d read=%d", + s.Calls, s.PromptCacheReadTokens) + } +} + +// /stats must stay backward compatible: deploy/harbor/*.py parses these exact keys, so a +// rename or removal breaks the benchmark harness. Fields are ADDED, never changed. +func TestSnapshotJSONKeysAreBackwardCompatible(t *testing.T) { + a := NewAggregator() + b, err := json.Marshal(a.Snapshot()) + if err != nil { + t.Fatal(err) + } + var m map[string]any + if err := json.Unmarshal(b, &m); err != nil { + t.Fatal(err) + } + // Every key the harness reads today. + for _, k := range []string{ + "requests", "tokens_before", "tokens_after", "saved_tokens", "savings_pct", + "wasted_tokens", "bounces", "adjusted_saved", "components", "top_passthrough", + "llm_calls", "llm_input_tokens", "llm_output_tokens", + "cg_added_ms_avg", "upstream_ms_avg", "upstream_ms_avg_bypassed", + } { + if _, ok := m[k]; !ok { + t.Errorf("/stats lost backward-compatible key %q", k) + } + } + // The new block must be omitted when absent rather than serialized as null, so a + // parser that does not know about it sees no change at all. + if _, present := m["extract"]; present { + t.Error(`"extract" must be omitted when unset (omitempty)`) + } + + // And when present, it must carry the net figure. + snap := a.Snapshot() + xs := ExtractSnapshot(1.0, 0.1, 0, 0) + snap.Extract = &xs + b2, _ := json.Marshal(snap) + var m2 map[string]any + _ = json.Unmarshal(b2, &m2) + ext, ok := m2["extract"].(map[string]any) + if !ok { + t.Fatal(`"extract" block missing when set`) + } + for _, k := range []string{"calls", "calls_avoided", "calls_suppressed", + "extraction_cost_usd", "gross_value_usd", "net_value_usd", + "prompt_cache_read_tokens", "avg_latency_ms", "cache_hit_rate"} { + if _, ok := ext[k]; !ok { + t.Errorf("extract block missing %q", k) + } + } +} + +// resetExtract clears the process counters so assertions are independent. +func resetExtract() { + xCalls.Store(0) + xCacheHits.Store(0) + xSuppressed.Store(0) + xGrossSaved.Store(0) + xLatencyMs.Store(0) + xLookups.Store(0) + xReasonMu.Lock() + xReasons = map[string]int64{} + xReasonMu.Unlock() +} diff --git a/metrics/metrics.go b/metrics/metrics.go index 634e02e..bfdd62c 100644 --- a/metrics/metrics.go +++ b/metrics/metrics.go @@ -416,6 +416,11 @@ type Snapshot struct { LLMCalls int64 `json:"llm_calls"` LLMInputTokens int64 `json:"llm_input_tokens"` LLMOutputTokens int64 `json:"llm_output_tokens"` + // Extract is extract_llm's own economics (#28 part F), including NET savings after + // its LLM cost — the honest headline for the one component that spends to save. + // Purely ADDITIVE: no field above was renamed or removed, so deploy/harbor/*.py + // keeps parsing /stats unchanged. + Extract *ExtractStats `json:"extract,omitempty"` // End-to-end latency (W7): mean ms context-guru added per request, and mean // provider round-trip on the active vs bypassed (baseline) path — a with/without // context-guru session-latency comparison. diff --git a/proxy/proxy.go b/proxy/proxy.go index 41ca54d..3c90408 100644 --- a/proxy/proxy.go +++ b/proxy/proxy.go @@ -665,9 +665,36 @@ func (h *Handler) stats(w http.ResponseWriter, _ *http.Request) { Processed: q.Processed, Dropped: q.Dropped, Errors: q.Errors, } } + // extract_llm economics (#28 part F). Net-after-cost is the honest headline: the three + // LLM* fields above report what the component SPENT, and until now nothing anywhere + // compared that against what its savings were WORTH — which is how an 82x loss stayed + // invisible. Computed here, the layer that knows the model pricing and the cache mode. + // Purely additive: every pre-existing field keeps its name for deploy/harbor/*.py. + cacheWrite, cacheRead := cheapmodel.CacheUsage() + pricing := cheapmodel.PricingFromEnv() + cost := pricing.Cost(snap.LLMInputTokens, snap.LLMOutputTokens, cacheWrite, cacheRead) + // Value the saved tokens at the rate they would actually have been billed. On a + // caching backend a removed token saves the cache-READ rate (~10x cheaper), which is + // exactly why the component can be underwater while its token count looks impressive. + perSavedTok := agentCacheReadPerMTok / 1e6 + if h.opts.CacheMode == "off" { + perSavedTok = agentFreshPerMTok / 1e6 + } + xs := metrics.ExtractSnapshot(cost, float64(snap.SavedTokens)*perSavedTok, cacheWrite, cacheRead) + snap.Extract = &xs json.NewEncoder(w).Encode(snap) } +// Agent-model token rates used to VALUE saved tokens at /stats (claude-sonnet-5 class, +// $3/MTok fresh, 0.1x cache read). Mirrors components/offload/extract_econ.go, which +// applies the same rates inside the gate; kept as local constants rather than a shared +// export because the two layers may legitimately be priced differently (the gate prices +// the traffic it sees; /stats prices the aggregate). +const ( + agentFreshPerMTok = 3.00 + agentCacheReadPerMTok = 0.30 +) + // expand resolves a stashed original by id — the HTTP side of reversibility (the // model-callable tool loop is a separate concern, added with response handling). func (h *Handler) expand(w http.ResponseWriter, r *http.Request) { From 0d8ba0301aebce2596f10a80ae600f0df33ba18d Mon Sep 17 00:00:00 2001 From: Osher-Elhadad Date: Mon, 10 Aug 2026 04:28:00 +0000 Subject: [PATCH 2/5] fix(extract_llm): price calls by size and allow bounded exploration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Benchmarking the gate on real captures found two deadlocks of the same shape: a pessimistic estimate suppressing every call, so nothing was ever observed, so the estimate could never correct itself. A gate that cannot revise its own prior is an off switch, not a gate. 1. Flat per-call cost. The gate priced every call at the ~$0.012 Terminal-Bench average — roughly 5x the true cost on a workload with small outputs — and suppressed everything. Observed cost was $0.0024/call, and calls that were ~2.2x profitable were declined. Cost is now analytic and size-aware (preamble 1,463 tok + shown content + overhead, at real rates), reconciled with the observed mean once real calls exist, so it is right on the first call with no observations at all. 2. Pessimistic compression ratio. The default was 0.45; measured on real captures an accepted extraction removes only 31-254 tokens per call on 400-2,000-token outputs — a real ratio near 0.12. Corrected, with the direction of conservatism made explicit: for a SPENDING gate, conservative means UNDER-estimating the saving. But a pessimistic prior on a workload below break-even then suppressed everything and forwent a genuine +$0.0094 net, so the tracker now gets a bounded budget of 3 exploratory calls to learn whether this workload actually compresses. Also fixes /compact hard-coding the context window as unknown, which silently disabled every fraction-based trigger AND the new pressure-based triggering on that endpoint — so offline replay/eval measured a different component than ships. Measured after these fixes (replay of real captures, aws/claude-haiku-4-5; forced = pre-#28 behavior, gated = new default): Terminal-Bench, non-caching: net +$0.0287 vs +$0.0091 (3.2x), 17,286 vs 6,664 tokens saved, 4,654 vs 10,287 ms avg latency SWE-bench, non-caching: 13 calls -> 2, net -$0.0571 -> -$0.0016 (-97% waste) Both, caching backend: loss reduced and latency ~30% lower, but still NEGATIVE — the gate cannot make the component pay when saved tokens bill at the cache-read rate The honest verdict, now documented: on a non-caching backend extract_llm earns its place; on a caching backend it does not, even after #28. Docs recommend codesafe (no LLM pass) or dropping extract_llm from codesmart for caching traffic. Break-even sizes are pinned by test so a drift in these figures fails CI rather than silently invalidating the documented verdict. Refs #28 Signed-off-by: Osher-Elhadad --- components/offload/extract_econ.go | 136 ++++++++++++++++++---- components/offload/extract_econ_test.go | 144 +++++++++++++++++++++--- components/offload/extract_llm.go | 11 +- docs/components/extract_llm.md | 80 ++++++++++--- docs/reference/presets.md | 14 ++- proxy/proxy.go | 13 ++- 6 files changed, 340 insertions(+), 58 deletions(-) diff --git a/components/offload/extract_econ.go b/components/offload/extract_econ.go index fc7a455..bc4fe0f 100644 --- a/components/offload/extract_econ.go +++ b/components/offload/extract_econ.go @@ -86,20 +86,71 @@ func savedTokenValue(c *components.Ctx) tokenValue { return tokenValue{perToken: agentFreshPerMTok / 1_000_000, cached: false} } -// priorCallCost is the fallback per-call cost estimate used before this process has -// observed any extraction call. ~$0.012 was the Terminal-Bench measurement; it is a PRIOR -// to be replaced by observation, never a constant to compute against — which is why -// callCost prefers cheapmodel.AvgCallCost as soon as one call has been made. +// priorCallCost is a last-resort per-call cost estimate (~the Terminal-Bench average). +// It is only used when neither an observation nor a size is available; see callCost for +// why a flat prior must never be the primary estimate. const priorCallCost = 0.012 -// callCost returns the expected dollar cost of one extraction call. Observed-mean first -// (it reflects this deployment's real prompt sizes, model pricing, and whether the -// preamble cache is working), prior second. -func callCost(pricing cheapmodel.Pricing) float64 { +// Prompt-size constants for the analytic cost estimate, in tokens. +const ( + // preambleTokens is the invariant contract + examples sent on every call (measured + // 1463 for the code strategy). It is billed as fresh input whenever the provider's + // minimum cacheable prefix is above it — which is the case on claude-haiku-4-5. + preambleTokens = 1463 + // promptOverheadTokens covers the goal + keep-list + labels in the variable part. + promptOverheadTokens = 200 + // expectedOutputTokens is a Starlark filter program's typical length (observed ~77 + // on Terminal-Bench captures; kept a little higher so cost is not under-estimated). + expectedOutputTokens = 200 + // maxShownTokens bounds the content shown to the model (maxCodeContentChars ≈ 32k + // chars ≈ 5k tokens), so a huge output does not inflate the estimated prompt without + // limit — the real prompt is truncated to head+tail. + maxShownTokens = 5000 +) + +// callCost returns the expected dollar cost of ONE extraction call for a candidate of +// sizeTokens. +// +// A flat per-call constant is the wrong model and caused a real cold-start deadlock in +// development: the gate priced every call at the ~$0.012 Terminal-Bench average, which is +// ~5x the true cost on a workload with small outputs, so it suppressed everything — and +// because it suppressed everything, no call was ever observed and the estimate never +// corrected itself. Measured on a real capture: observed cost was $0.0024/call against +// the $0.012 prior, and the gate wrongly declined calls that were in fact ~2.2x profitable. +// +// So the estimate is analytic and size-aware first (prompt = preamble + shown content + +// overhead, priced at real rates), blended with the observed mean once real calls exist. +// The observed mean alone is not enough either: it is an average over past candidate +// sizes, and cost genuinely scales with THIS candidate's size. +func callCost(pricing cheapmodel.Pricing, sizeTokens int) float64 { + shown := sizeTokens + if shown > maxShownTokens { + shown = maxShownTokens + } + inTok := int64(preambleTokens + shown + promptOverheadTokens) + analytic := pricing.Cost(inTok, expectedOutputTokens, 0, 0) + + // Reconcile with reality: if observed calls came in cheaper or dearer than the + // analytic model predicts (a working preamble cache, a different tokenizer, a + // gateway contract), scale by that ratio rather than discarding size-sensitivity. if avg, ok := cheapmodel.AvgCallCost(pricing); ok && avg > 0 { - return avg + if base := analyticBaseline(pricing); base > 0 { + if ratio := avg / base; ratio > 0.1 && ratio < 10 { + return analytic * ratio + } + } + return (analytic + avg) / 2 // ratio implausible: hedge between the two + } + if analytic <= 0 { + return priorCallCost } - return priorCallCost + return analytic +} + +// analyticBaseline is the analytic cost of a mid-sized candidate, used as the denominator +// when scaling the analytic estimate to observed reality. +func analyticBaseline(pricing cheapmodel.Pricing) float64 { + return pricing.Cost(preambleTokens+2000+promptOverheadTokens, expectedOutputTokens, 0, 0) } // gateDecision records why the gate allowed or suppressed a call. The reason string is @@ -142,7 +193,7 @@ func expectedReuses(seenBefore bool, turnsSoFar int) float64 { // // Allow only when saving strictly exceeds cost. Every suppression carries a reason. func evaluateGate(sizeTokens int, ratio float64, val tokenValue, cost float64, - seenBefore bool, turnsSoFar int) gateDecision { + seenBefore bool, turnsSoFar int, explore bool) gateDecision { expectedRemoved := float64(sizeTokens) * ratio reuses := expectedReuses(seenBefore, turnsSoFar) @@ -150,6 +201,13 @@ func evaluateGate(sizeTokens int, ratio float64, val tokenValue, cost float64, saving := expectedRemoved * (1 + reuses) * val.perToken d := gateDecision{expSaving: saving, expCost: cost} + if saving <= cost && explore { + // No trustworthy ratio yet — spend a bounded call to find out rather than + // letting a pessimistic prior justify itself forever. + d.allow = true + d.reason = "allow: exploring (learning this workload's compression ratio)" + return d + } if saving <= cost { // The honest message: on a caching backend a small output CANNOT pay for a call. if val.cached { @@ -172,9 +230,18 @@ func evaluateGate(sizeTokens int, ratio float64, val tokenValue, cost float64, } // defaultCompressionRatio is the fraction of an output an accepted extraction removes, -// used before this component has observed its own results. Deliberately CONSERVATIVE: -// over-estimating the ratio is how the component talked itself into 271 losing calls. -const defaultCompressionRatio = 0.45 +// used before this component has observed its own results. +// +// MEASURED, and much lower than intuition suggests. On real captures an accepted +// extraction removed only 31-254 tokens per call on outputs of 400-2000 tokens — an actual +// ratio around 0.10, not the 0.45 originally assumed here. The model mostly declines to cut +// aggressively (correctly: its contract is recall-first), so most of a "reduction" is small. +// +// Note the DIRECTION of conservatism: for a spending gate, conservative means +// UNDER-estimating the saving, i.e. a LOW ratio. An optimistic ratio is precisely how the +// component talked itself into 271 losing calls, so this errs low and lets the observed +// tracker raise it if a workload really does compress well. +const defaultCompressionRatio = 0.12 // ratioTracker learns this workload's ACTUAL compression ratio from accepted results, so // the gate stops guessing after the first few calls. A call that produced nothing counts @@ -182,9 +249,10 @@ const defaultCompressionRatio = 0.45 // the estimate down and shut the gate, which is precisely the feedback the old // fixed-threshold design lacked. type ratioTracker struct { - mu sync.Mutex - removed int64 - total int64 + mu sync.Mutex + removed int64 + total int64 + explored int // calls spent learning the ratio (bounded by maxExploreCalls) } // observe records one attempted extraction: removedTok of totalTok (0 removed on a miss). @@ -209,9 +277,39 @@ func (r *ratioTracker) ratio() float64 { return float64(r.removed) / float64(r.total) } +// exploring reports whether the tracker still lacks the evidence to estimate a ratio, and +// consumes one exploration slot if so. +// +// This closes the SECOND deadlock of the same shape as the flat-cost one (see callCost). +// The ratio starts at a deliberately pessimistic 0.12; on a workload whose outputs sit +// below the resulting break-even, the gate suppresses every call — so the tracker never +// observes anything and the pessimistic default becomes permanent and self-justifying. +// Measured on a real capture: the gate forwent a genuine +$0.0094 net because of exactly +// this. A gate that can never revise its own prior is not a gate, it is an off switch. +// +// So allow a small, BOUNDED number of calls through before the estimate is trustworthy. +// The exploration budget is tiny relative to a session (each call is ~$0.003-0.008), and it +// buys the one thing the gate cannot compute from first principles: whether THIS workload's +// outputs actually compress. +func (r *ratioTracker) exploring() bool { + r.mu.Lock() + defer r.mu.Unlock() + if r.total >= minRatioSampleTokens || r.explored >= maxExploreCalls { + return false + } + r.explored++ + return true +} + +// maxExploreCalls bounds the exploration budget. Small on purpose: enough to learn the +// ratio (a handful of outputs clears minRatioSampleTokens), far too few to reproduce the +// 271-call loss even if every one of them is wasted. +const maxExploreCalls = 3 + // minRatioSampleTokens is how much observed content the ratio estimate needs before it -// displaces the default — roughly a couple of medium outputs. -const minRatioSampleTokens = 4000 +// displaces the default. Kept small so a workload that genuinely compresses well is +// recognized within a few calls rather than after a whole session. +const minRatioSampleTokens = 1500 // --- Triggering (issue #28 part E) ----------------------------------------------- // diff --git a/components/offload/extract_econ_test.go b/components/offload/extract_econ_test.go index 9d02771..c5b6fb1 100644 --- a/components/offload/extract_econ_test.go +++ b/components/offload/extract_econ_test.go @@ -17,7 +17,7 @@ func TestGateSuppressesSmallOutputWhenCacheAware(t *testing.T) { if val.cached != true { t.Fatal("cache-aware ctx must price saved tokens at the cache-read rate") } - d := evaluateGate(400, defaultCompressionRatio, val, priorCallCost, false, 5) + d := evaluateGate(400, defaultCompressionRatio, val, callCost(cheapmodel.HaikuPricing(), 400), false, 5, false) if d.allow { t.Fatalf("small cached output must be suppressed: saving=$%.5f cost=$%.5f", d.expSaving, d.expCost) } @@ -33,11 +33,11 @@ func TestGateSuppressesSmallOutputWhenCacheAware(t *testing.T) { // more valuable — so the same output that loses under caching can win here. This is the // asymmetry the gate exists to exploit, so assert it on ONE fixture, not two. func TestGatePermitsOnNonCachingBackend(t *testing.T) { - size := 12000 + size := 60000 // above both break-evens (~1.8k non-caching, ~42.6k cached) cached := evaluateGate(size, defaultCompressionRatio, - savedTokenValue(&components.Ctx{CacheAware: true}), priorCallCost, false, 5) + savedTokenValue(&components.Ctx{CacheAware: true}), callCost(cheapmodel.HaikuPricing(), size), false, 5, false) fresh := evaluateGate(size, defaultCompressionRatio, - savedTokenValue(&components.Ctx{CacheAware: false}), priorCallCost, false, 5) + savedTokenValue(&components.Ctx{CacheAware: false}), callCost(cheapmodel.HaikuPricing(), size), false, 5, false) if !fresh.allow { t.Fatalf("non-caching backend must permit a %d-token output: saving=$%.5f cost=$%.5f", @@ -54,12 +54,12 @@ func TestGatePermitsOnNonCachingBackend(t *testing.T) { // 82/103 across sessions, so this is the common case, not an edge case. func TestGatePermitsHighReuseContent(t *testing.T) { val := savedTokenValue(&components.Ctx{CacheAware: true}) - // 14000 tokens: above the ~12.7k cached-recurring break-even, below the ~17.8k + // 34000 tokens: above the ~30.5k cached-RECURRING break-even, below the ~42.6k // cached-once one. This size is the gate's whole thesis in one fixture — recurrence is // what tips an otherwise-losing call into profit, so the SAME size goes both ways. - size := 14000 - once := evaluateGate(size, defaultCompressionRatio, val, priorCallCost, false, 5) - recur := evaluateGate(size, defaultCompressionRatio, val, priorCallCost, true, 5) + size := 34000 + once := evaluateGate(size, defaultCompressionRatio, val, callCost(cheapmodel.HaikuPricing(), size), false, 5, false) + recur := evaluateGate(size, defaultCompressionRatio, val, callCost(cheapmodel.HaikuPricing(), size), true, 5, false) if recur.expSaving <= once.expSaving { t.Fatalf("recurring content must be valued higher: recur=%v once=%v", recur.expSaving, once.expSaving) @@ -83,7 +83,7 @@ func TestBreakEvenSizesMatchTheDocumentedVerdict(t *testing.T) { breakEven := func(cacheAware, recurring bool) int { val := savedTokenValue(&components.Ctx{CacheAware: cacheAware}) for size := 200; size <= 400_000; size += 100 { - if evaluateGate(size, defaultCompressionRatio, val, priorCallCost, recurring, 5).allow { + if evaluateGate(size, defaultCompressionRatio, val, callCost(cheapmodel.HaikuPricing(), size), recurring, 5, false).allow { return size } } @@ -91,16 +91,17 @@ func TestBreakEvenSizesMatchTheDocumentedVerdict(t *testing.T) { } cachedRecur := breakEven(true, true) freshRecur := breakEven(false, true) - if cachedRecur < 11_000 || cachedRecur > 14_000 { - t.Errorf("cached+recurring break-even = %d tokens, expected ~12,700 "+ + if cachedRecur < 26_000 || cachedRecur > 35_000 { + t.Errorf("cached+recurring break-even = %d tokens, expected ~30,500 "+ "(docs/components/extract_llm.md states this figure)", cachedRecur) } - if freshRecur < 1_000 || freshRecur > 1_600 { - t.Errorf("fresh+recurring break-even = %d tokens, expected ~1,270", freshRecur) + if freshRecur < 1_400 || freshRecur > 2_400 { + t.Errorf("fresh+recurring break-even = %d tokens, expected ~1,800", freshRecur) } - // The 10x rate haircut must show up as a ~10x break-even gap. - if ratio := float64(cachedRecur) / float64(freshRecur); ratio < 8 || ratio > 12 { - t.Errorf("cached/fresh break-even ratio = %.1fx, expected ~10x", ratio) + // The gap is WIDER than the bare 10x rate haircut, because cost stops growing once the + // prompt hits the shown-content cap while value keeps scaling with output size. + if ratio := float64(cachedRecur) / float64(freshRecur); ratio < 12 || ratio > 30 { + t.Errorf("cached/fresh break-even ratio = %.1fx, expected ~20x", ratio) } } @@ -128,7 +129,7 @@ func TestCostModelMatchesKnownTokensTimesKnownPrice(t *testing.T) { // callCost must fall back to the prior only until a real observation exists; it must never // return zero, which would make the gate permit everything. func TestCallCostNeverZero(t *testing.T) { - if c := callCost(cheapmodel.HaikuPricing()); c <= 0 { + if c := callCost(cheapmodel.HaikuPricing(), 3000); c <= 0 { t.Fatalf("callCost must be positive, got %v", c) } } @@ -209,3 +210,112 @@ func TestRatioTrackerLearnsFromObservations(t *testing.T) { t.Fatalf("repeated misses must drive the ratio down, got %v", got) } } + +// REGRESSION (found on a live capture): a FLAT per-call cost estimate deadlocks the gate. +// The gate priced every call at the ~$0.012 Terminal-Bench average — roughly 5x the true +// cost on a workload with small outputs — so it suppressed everything; and because it +// suppressed everything, no call was ever observed and the estimate could never correct +// itself. Measured: observed $0.0024/call vs the $0.012 prior, and the gate declined calls +// that were in fact ~2.2x profitable. +// +// The fix is that cost must be ANALYTIC and SIZE-AWARE, so the estimate is right on the +// very first call with no observations at all. +func TestCallCostIsSizeAwareNotFlat(t *testing.T) { + p := cheapmodel.HaikuPricing() + small := callCost(p, 400) + mid := callCost(p, 2000) + big := callCost(p, 5000) + + if !(small < mid && mid < big) { + t.Fatalf("cost must scale with candidate size, got %v %v %v", small, mid, big) + } + // A small candidate must cost far less than the old flat prior, or the gate + // re-deadlocks on exactly the workload that exposed the bug. + if small >= priorCallCost { + t.Fatalf("a 400-token candidate must cost well under the flat prior $%.4f, got $%.5f", + priorCallCost, small) + } + // The preamble dominates a small call, so it must be included — a cost model that + // forgot it would under-price and let the gate permit everything. + if small <= p.Cost(preambleTokens, 0, 0, 0)*0.5 { + t.Fatalf("cost must include the ~%d-token preamble, got $%.5f", preambleTokens, small) + } + // Beyond the shown-content cap the prompt is truncated, so cost must stop growing — + // otherwise a huge output is priced as if the whole thing were sent. + if callCost(p, 50_000) != callCost(p, 500_000) { + t.Fatal("cost must plateau past the shown-content cap (the prompt is truncated)") + } +} + +// The gate must be decidable on the FIRST call, with no observations — that is what the +// size-aware cost model buys. A 2,000-token output (the largest actually present in the +// measured capture) does not pay even on a non-caching backend at the measured 0.12 ratio, +// and the gate must say so from cold rather than needing a warm-up; a clearly larger output +// must be permitted from cold too. Both directions, no observations, first call. +func TestGateIsDecidableFromColdOnFirstCall(t *testing.T) { + val := savedTokenValue(&components.Ctx{CacheAware: false}) + + small := 2000 + d := evaluateGate(small, defaultCompressionRatio, val, + callCost(cheapmodel.HaikuPricing(), small), false, 5, false) + if d.allow { + t.Errorf("a %d-token output should not pay at the measured 0.12 ratio: "+ + "saving=$%.5f cost=$%.5f", small, d.expSaving, d.expCost) + } + + big := 20000 // comfortably above the ~1.8k-3.4k non-caching break-even + d = evaluateGate(big, defaultCompressionRatio, val, + callCost(cheapmodel.HaikuPricing(), big), false, 5, false) + if !d.allow { + t.Errorf("a %d-token output on a non-caching backend must pay on the first call: "+ + "saving=$%.5f cost=$%.5f", big, d.expSaving, d.expCost) + } + // And it must be permitted for the right reason, not by accident. + if d.reason == "" { + t.Error("an allowed call must carry a reason") + } +} + +// REGRESSION (found on a live capture): a pessimistic ratio prior can justify itself +// forever. The ratio starts at 0.12; on a workload whose outputs sit below the resulting +// break-even the gate suppresses every call, so the tracker never observes anything and the +// prior becomes permanent. Measured: the gate forwent a genuine +$0.0094 net this way. +// +// A BOUNDED exploration budget breaks the loop. This is the same failure shape as the +// flat-cost deadlock (see TestCallCostIsSizeAwareNotFlat) — a gate that cannot revise its +// own prior is an off switch, not a gate. +func TestGateExploresThenSettles(t *testing.T) { + var r ratioTracker + explored := 0 + for i := 0; i < 20; i++ { + if r.exploring() { + explored++ + } + } + if explored != maxExploreCalls { + t.Fatalf("exploration must be bounded to %d calls, got %d", maxExploreCalls, explored) + } + + // An exploration slot must actually flip an otherwise-suppressed decision. + val := savedTokenValue(&components.Ctx{CacheAware: true}) + size := 400 // far below break-even: normally suppressed + cost := callCost(cheapmodel.HaikuPricing(), size) + suppressed := evaluateGate(size, defaultCompressionRatio, val, cost, false, 5, false) + if suppressed.allow { + t.Fatal("without an exploration slot a tiny cached output must be suppressed") + } + exploring := evaluateGate(size, defaultCompressionRatio, val, cost, false, 5, true) + if !exploring.allow { + t.Fatal("an exploration slot must permit the call so the ratio can be learned") + } + if exploring.reason == "" || exploring.reason == suppressed.reason { + t.Fatalf("exploration must be distinguishable in the reason, got %q", exploring.reason) + } + + // Once enough evidence exists, exploration stops even if slots remain unused. + var r2 ratioTracker + r2.observe(200, minRatioSampleTokens+1) + if r2.exploring() { + t.Fatal("with sufficient evidence the gate must stop exploring") + } +} diff --git a/components/offload/extract_llm.go b/components/offload/extract_llm.go index fc01668..7871d10 100644 --- a/components/offload/extract_llm.go +++ b/components/offload/extract_llm.go @@ -249,7 +249,6 @@ func (e *ExtractLLM) Offload(req *bschemas.BifrostChatRequest, rep *components.R } // Gate inputs shared by every candidate this request. val := savedTokenValue(c) - perCall := callCost(e.pricing) ratio := e.ratios.ratio() turnsSoFar := len(req.Input) extCfg := extract.DefaultCfg() @@ -297,7 +296,7 @@ func (e *ExtractLLM) Offload(req *bschemas.BifrostChatRequest, rep *components.R if e.skipFileReads != nil { skipFR = *e.skipFileReads } - var dbgTail, dbgFloor, dbgPlace, dbgReapply, dbgBigTailBlocked int + var dbgTail, dbgFloor, dbgPlace, dbgReapply, dbgBigTailBlocked, dbgMaxSz int for _, i := range tools { msg := &req.Input[i] if !schema.Rewritable(*msg) { @@ -336,6 +335,9 @@ func (e *ExtractLLM) Offload(req *bschemas.BifrostChatRequest, rep *components.R // suffix is cache-written either way, so re-deriving would buy a model call for // nothing. The model may also not run at all (throttle, timeout, floor), which would // leave the output verbatim at depth after the gate had already been lifted. + if sz > dbgMaxSz { + dbgMaxSz = sz + } if c.CacheAware && !c.TailOnly(i) { dbgTail++ if sz >= floor { @@ -387,7 +389,8 @@ func (e *ExtractLLM) Offload(req *bschemas.BifrostChatRequest, rep *components.R // Content the store has seen before in ANY session is likely to recur again, // which is what amortizes the call across turns. seenBefore := hasSeenContent(c, id) - d := evaluateGate(sz, ratio, val, perCall, seenBefore, turnsSoFar) + d := evaluateGate(sz, ratio, val, callCost(e.pricing, sz), seenBefore, turnsSoFar, + e.ratios.exploring()) if !d.allow { metrics.RecordExtractionSuppressed(d.reason) if debugExtractLLM { @@ -405,7 +408,7 @@ func (e *ExtractLLM) Offload(req *bschemas.BifrostChatRequest, rep *components.R if debugExtractLLM && len(tools) > 0 { slog.Info("cg.debug.extract_llm", "tools", len(tools), "cands", len(cands), "reapplied", dbgReapply, "skip_placeholder", dbgPlace, "skip_tail", dbgTail, - "skip_floor", dbgFloor, "big_but_not_tail", dbgBigTailBlocked, + "skip_floor", dbgFloor, "max_output_tokens", dbgMaxSz, "big_but_not_tail", dbgBigTailBlocked, "cacheAware", c.CacheAware, "maxCachedIdx", c.MaxCachedIdx, "floor", floor, "nInput", len(req.Input)) } diff --git a/docs/components/extract_llm.md b/docs/components/extract_llm.md index 33e8ba6..7e24b06 100644 --- a/docs/components/extract_llm.md +++ b/docs/components/extract_llm.md @@ -28,22 +28,68 @@ costing ~$0.012 must therefore remove a *lot* of tokens to break even: | Backend | Content | Break-even output size | |---|---|---| -| Caching | seen once | **~17,800 tokens** | -| Caching | recurring (amortized over replays) | **~12,700 tokens** | -| Non-caching | seen once | ~1,780 tokens | -| Non-caching | recurring | **~1,270 tokens** | - -Most tool outputs are nowhere near 12,700 tokens. That is why the same component **wins on a -non-caching backend and loses on a caching one**, and why the fix is not "compress harder" but -"decide per call". Since #28 the [economic gate](#economics) makes that decision automatically, -so the component is safe to leave enabled — it simply declines to spend where it cannot win. +| Caching | seen once | **~42,600 tokens** | +| Caching | recurring (amortized over replays) | **~30,500 tokens** | +| Non-caching | seen once | ~3,400 tokens | +| Non-caching | recurring | **~1,800 tokens** | + +These use the **measured** compression ratio, and that measurement is the uncomfortable part: on +real captures an accepted extraction removed only **31–254 tokens per call** on outputs of +400–2,000 tokens — an actual ratio around **0.10–0.12**, not the ~0.45 one might assume. The model +declines to cut aggressively, and correctly so: its contract is recall-first. + +Most tool outputs are nowhere near 30,500 tokens — in one measured Terminal-Bench capture the +**largest** tool output was 2,053 tokens, ~15× below the cached break-even. That is why the same +component **wins on a non-caching backend and loses on a caching one**, and why the fix is not +"compress harder" but "decide per call". Since #28 the [economic gate](#economics) makes that +decision automatically, so the component is safe to leave enabled — it simply declines to spend +where it cannot win. + +### Measured after #28 (replay of real captures, `aws/claude-haiku-4-5`) + +`forced` = pre-#28 behavior (`economic_gate: false`); `gated` = post-#28 default. Same +capture, same floor, same model — the only difference is the gate. + +**Terminal-Bench capture (20 requests):** + +| Arm | Backend | LLM calls | Cost | Tokens saved | Gross value | **NET** | Avg latency | +|---|---|---|---|---|---|---|---| +| forced | caching | 4 | $0.0086 | 1,220 | $0.0004 | **−$0.0082** | 9,726 ms | +| **gated** | caching | 3 | $0.0065 | 0 | $0 | **−$0.0065** | 6,966 ms | +| forced | non-caching | 6 | $0.0109 | 6,664 | $0.0200 | **+$0.0091** | 10,287 ms | +| **gated** | non-caching | 7 | $0.0232 | **17,286** | $0.0519 | **+$0.0287** | **4,654 ms** | + +**SWE-bench capture (19 requests):** + +| Arm | Backend | LLM calls | Cost | Tokens saved | **NET** | Avg latency | +|---|---|---|---|---|---|---| +| forced | caching | 2 | $0.0098 | 793 | **−$0.0095** | 8,173 ms | +| **gated** | caching | 2 | $0.0092 | 806 | **−$0.0089** | 5,060 ms | +| forced | non-caching | **13** | $0.0593 | 726 | **−$0.0571** | 5,150 ms | +| **gated** | non-caching | **2** | $0.0090 | **2,451** | **−$0.0016** | 3,754 ms | + +Reading these: + +- **On a non-caching backend the gate is a clear win.** Terminal-Bench: net **+$0.0287 vs + +$0.0091** (3.2×) with **2.6× more tokens saved** and **half the latency**. SWE-bench: it cuts + 13 calls to 2 and the loss from **−$0.0571 to −$0.0016** — a **97% reduction in waste**. +- **On a caching backend it is still slightly negative**, because even the gate's minimum + exploration spend exceeds what cache-read-rate savings can return on these output sizes. The + gate reduces the loss (−$0.0082 → −$0.0065; −$0.0095 → −$0.0089) and cuts latency ~30%, but it + does not turn the component positive. **This is the honest verdict: on a caching backend + `extract_llm` does not earn its place on these workloads.** +- The `reasons` breakdown shows the gate working for the documented reasons — + `allow: recurring content, amortized over reuses`, `allow: non-caching backend, saved tokens at + full rate`, `suppressed: cache-aware, saving below call cost`. !!! tip "If you only remember one thing" On a caching backend, expect `extract_llm` to suppress most candidates and contribute little; its value comes from the **result cache**, not from new LLM calls. On a **non-caching** backend it is genuinely valuable. Check `/stats` → `extract.net_value_usd` — if it is negative on your workload, remove the - component from the pipeline. + component from the pipeline. **On the caching backends measured here it IS negative even + after #28**, so the recommendation for caching traffic is `codesafe` (which has no LLM pass) + or dropping `extract_llm` from `codesmart`. ## How it works @@ -80,7 +126,9 @@ expected saving = tokens expected to remove x (1 + expected future replays) x per-token value <-- cache-read rate when cache-aware, else fresh rate -expected cost = observed mean cost of one extraction call +expected cost = analytic size-aware cost of one extraction call + (preamble + shown content + overhead, at real rates), + reconciled with the observed mean once calls exist ``` Each input is measured rather than assumed: @@ -88,8 +136,8 @@ Each input is measured rather than assumed: | Input | Source | |---|---| | Per-token value | `Ctx.CacheAware` selects the cache-read vs fresh rate (the 10× factor) | -| Expected compression ratio | **Learned** from this workload's accepted results; a conservative 0.45 until ~4k tokens of evidence. Repeated misses drive it toward 0, shutting the gate | -| Call cost | **Observed mean** of real calls (tokens × real model pricing), not a hard-coded constant. Falls back to a ~$0.012 prior only before the first call | +| Expected compression ratio | **Learned** from this workload's accepted results; a conservative **0.12** (the measured figure) until ~1.5k tokens of evidence. Repeated misses drive it toward 0, shutting the gate. Note the direction of conservatism: for a *spending* gate, conservative means under-estimating the saving | +| Call cost | **Analytic and size-aware** — `preamble (1,463 tok) + shown content + overhead`, priced at real model rates — then reconciled with the observed mean once real calls exist. A flat per-call constant is not just imprecise, it **deadlocks**: pricing every call at the $0.012 average (≈5× the true cost on small outputs) suppressed everything, so nothing was ever observed and the estimate could never correct itself. Measured: $0.0024/call actual vs the $0.012 prior | | Model pricing | `claude-haiku-4-5` list rates by default; override with `CHEAP_MODEL_PRICE_IN` / `_OUT` / `_CACHE_WRITE` / `_CACHE_READ` (dollars per MTok) | | Expected replays | Recurrence: content seen before in **any** session is expected to recur (measured 82/103 across sessions) | | Remaining horizon | Fewer expected replays late in a long session | @@ -117,6 +165,12 @@ absolute `trigger` applies — the same fail-open convention `Trigger` already u **`min_tokens` still governs when set explicitly**, so existing configs keep their behavior. +!!! note "`/compact` now resolves the context window too" + The `/compact` endpoint used to hard-code the window as unknown, which silently disabled + every fraction-based `trigger` threshold *and* this pressure-based logic on that path — so + offline replay/eval measured a different component than the one that ships. It now resolves + the window exactly as the chat path does. + ## Caching Three distinct caches, easily confused: diff --git a/docs/reference/presets.md b/docs/reference/presets.md index 75fce52..0aa7484 100644 --- a/docs/reference/presets.md +++ b/docs/reference/presets.md @@ -36,7 +36,7 @@ taken exactly from the `presets` map in `config/config.go`. `extract_llm` is the only component that spends money to save money, and on a prompt-caching backend it was measured **~8× underwater**: a token removed from a cached region saves the cache-read rate (`$0.30/MTok`), not the fresh-input rate (`$3/MTok`), so - break-even is **~12,700 tokens per output** — far above a typical tool output. + break-even is **~30,500 tokens per output** at the measured compression ratio — far above a typical tool output (the largest in one capture was 2,053). Since #28 it applies an [economic gate](../components/extract_llm.md#economics): it calls the LLM only when the expected saving exceeds the expected cost. On a caching backend that @@ -45,9 +45,15 @@ taken exactly from the `presets` map in `config/config.go`. result cache. On a **non-caching** backend the gate permits far more, because there the reduction is worth 10× more. - Check `/stats` → `extract.net_value_usd`. If it is negative on your workload, drop - `extract_llm` from the pipeline (or use `codesafe`, which never had it). `codesmart`'s - pinned `min_tokens: 3000` still governs its per-output floor, so that part is unchanged. + **Measured verdict:** on a **non-caching** backend the gate is a clear win (net + **+$0.0287 vs +$0.0091** on one capture, 2.6× more tokens saved at half the latency; on + another it cut 13 calls to 2 and reduced waste **97%**). On a **caching** backend it reduces + the loss and the latency but does **not** turn the component positive — so for caching + traffic prefer **`codesafe`** (no LLM pass) or drop `extract_llm` from `codesmart`. See + [the component's measured tables](../components/extract_llm.md#measured-after-28-replay-of-real-captures-awsclaude-haiku-4-5). + + Check `/stats` → `extract.net_value_usd` on your own workload. `codesmart`'s pinned + `min_tokens: 3000` still governs its per-output floor, so that part is unchanged. Not sure which to pick? See [Choose a preset](../how-to/choose-a-preset.md). Every component's config lives in [Components](../components.md). diff --git a/proxy/proxy.go b/proxy/proxy.go index 3c90408..39f4f4d 100644 --- a/proxy/proxy.go +++ b/proxy/proxy.go @@ -266,11 +266,22 @@ func (h *Handler) compact(w http.ResponseWriter, r *http.Request) { if q := r.URL.Query().Get("cache"); q != "" { cacheMode = q } + // Resolve the model's context window here too, exactly as the chat path does. It used + // to be hard-coded 0 ("unknown"), which silently disabled every fraction-based Trigger + // threshold AND extract_llm's context-pressure triggering on this endpoint — so + // /compact did not reflect production, and offline replay/eval measured a different + // component than the one that ships. + window := 0 + if h.opts.Windows != nil { + if w, ok := h.opts.Windows.Window(r.Context(), gjson.GetBytes(body, "model").String()); ok { + window = w + } + } out, _ := apply.BodyFull( r.Context(), pipe, h.store, provider, body, r.Header.Get("x-context-guru-session"), strings.EqualFold(r.Header.Get("x-context-guru-bypass"), "true"), - models, 0, cacheMode, + models, window, cacheMode, ) w.Header().Set("Content-Type", "application/json") w.Write(out) From a760b7adf897ed11e6e1acd8a3725132c310c3b1 Mon Sep 17 00:00:00 2001 From: Osher-Elhadad Date: Mon, 10 Aug 2026 08:30:43 +0000 Subject: [PATCH 3/5] fix(extract_llm): correct savings attribution, seen-flag, and exploration scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three high-severity review defects, plus the shipping decision. H3 (worst): /stats credited the WHOLE pipeline's savings to extract_llm. It passed snap.SavedTokens — every component's savings — valued against extract_llm's cost alone, so on a preset like codesmart the component displayed as comfortably positive while its own arithmetic proves it negative. It inverted the conclusion in the single field an operator reads. This also invalidated our own benchmark table: the previously reported "+$0.0287 net win" on non-caching traffic was produced BY the bug. With honest attribution the best result on these captures is break-even, not profit. Docs corrected. ExtractSnapshot now takes a per-saved-token RATE and applies it internally to the component's own GrossSavedTokens, so passing a pipeline-wide total is no longer expressible. Regression test fails if anyone re-wires it. H1: markSeenContent fired after the gate ALLOWED a call, so first sight reclassified itself as recurring and collected a 50% valuation bump (6 expected reuses vs 4) it had not earned — the gate over-firing, opposite in direction to the two pessimistic priors fixed earlier. It is now test-and-set on OBSERVATION, returning the prior value; a suppressed candidate still counts as seen, which is correct since recurrence is a property of the content, not of what we spent. The flag-setting path now has tests; previously only the pre-computed bool did. H2: the exploration budget was per-PROCESS. The tracker lives on a Pipeline held for proxy lifetime, so the first session spent the whole budget and every later session inherited an unrevisable prior — the self-justifying-prior failure at process scope. Now per-session (budget 3 -> 2). The learned ratio is also shrunk toward the prior and capped: minRatioSampleTokens is one medium output, so a raw n=1 mean could drop the cached break-even from ~30,500 to ~7,000 permanently. PR #37: exploration spends wall clock as well as money, and an agent on a task deadline feels the former more (#37 measured 17.8s over 2 calls saving 0 tokens). Speculative calls now stop once observed mean latency reaches 6s. SHIPPING DECISION, in code rather than prose: extract_llm is disabled by default on prompt-caching backends (allow_on_caching_backend: true overrides). Every caching workload measured is net-negative even with a correct gate — break-even ~30,500 tokens/output against a largest-observed 2,053 — so the gate could reduce the loss but never eliminate it. codesmart is the proxy default; shipping a component our own numbers say loses money, guarded only by a doc note, is not a defensible default. On caching traffic it now makes zero calls and costs nothing. Medium items taken: - Floor dropped from the cache fingerprint except in "auto" mode. It is derived from context pressure, so it rotated the key as the window filled and discarded the cross-session reuse the key exists to capture; strategyOrder reads it only on the "auto" branch, so elsewhere it cannot change the result. - PromptVersion is now DERIVED from a hash of the prompt constants. A manual constant only works while every future editor remembers to bump it, and one omission serves extractions produced under rules that no longer exist, with no symptom. semanticsVersion remains as the manual hatch for validation-gate changes. - Cross-session reuse gated on RECOVERABILITY rather than restricted to deletion-only: a cached result can be a lossy rewrite steered by another session's goal, which is acceptable only while expand can recover the original. With marker_mode summary/off (or a non-persisting store) the global cache falls back to same-session reuse, on both the read and write side. Verified deletion-only results are always global. Gates: build, test -tags cg_skeleton, -race -count=5, gofmt -l (clean), go vet. Refs #28 Signed-off-by: Osher-Elhadad --- components/offload/extract_econ.go | 105 ++++++++++++++++---- components/offload/extract_econ_test.go | 122 +++++++++++++++++++++--- components/offload/extract_seen_test.go | 51 ++++++++++ components/offload/state.go | 25 +++-- docs/components/extract_llm.md | 64 +++++++------ docs/reference/config.md | 5 + docs/reference/presets.md | 37 ++++--- internal/extract/extract.go | 12 ++- internal/extract/prompt.go | 38 ++++++-- internal/extract/resultkey_test.go | 76 +++++++++++++++ metrics/extract.go | 34 ++++++- metrics/extract_test.go | 61 +++++++++++- proxy/proxy.go | 15 ++- 13 files changed, 536 insertions(+), 109 deletions(-) create mode 100644 components/offload/extract_seen_test.go diff --git a/components/offload/extract_econ.go b/components/offload/extract_econ.go index bc4fe0f..3c57c9d 100644 --- a/components/offload/extract_econ.go +++ b/components/offload/extract_econ.go @@ -193,7 +193,7 @@ func expectedReuses(seenBefore bool, turnsSoFar int) float64 { // // Allow only when saving strictly exceeds cost. Every suppression carries a reason. func evaluateGate(sizeTokens int, ratio float64, val tokenValue, cost float64, - seenBefore bool, turnsSoFar int, explore bool) gateDecision { + seenBefore bool, turnsSoFar int, explore, allowCached bool) gateDecision { expectedRemoved := float64(sizeTokens) * ratio reuses := expectedReuses(seenBefore, turnsSoFar) @@ -201,6 +201,18 @@ func evaluateGate(sizeTokens int, ratio float64, val tokenValue, cost float64, saving := expectedRemoved * (1 + reuses) * val.perToken d := gateDecision{expSaving: saving, expCost: cost} + // Hard decline on a caching backend unless explicitly forced. This is the SHIPPING + // DECISION, in code rather than prose: the measurements in this change show the + // component net-negative on every caching workload tested, even with the gate working + // correctly (break-even ~30,500 tokens/output against a largest-observed 2,053). A + // default that ships a component our own numbers say loses money — guarded only by a + // doc note nobody reads — is not a defensible default. `allow_on_caching_backend: true` + // re-enables it for anyone whose workload genuinely has huge outputs; the gate's + // economics then apply as normal. + if val.cached && !allowCached { + d.reason = "suppressed: disabled by default on caching backends (measured net-negative)" + return d + } if saving <= cost && explore { // No trustworthy ratio yet — spend a bounded call to find out rather than // letting a pessimistic prior justify itself forever. @@ -249,10 +261,17 @@ const defaultCompressionRatio = 0.12 // the estimate down and shut the gate, which is precisely the feedback the old // fixed-threshold design lacked. type ratioTracker struct { - mu sync.Mutex - removed int64 - total int64 - explored int // calls spent learning the ratio (bounded by maxExploreCalls) + mu sync.Mutex + removed int64 + total int64 + // explored counts exploration calls PER SESSION. A process-wide counter spent its + // whole budget on the first session, after which every later session inherited an + // unrevisable prior — reintroducing the self-justifying-prior failure at process scope, + // which is the exact thing exploration exists to prevent. The tracker lives on the + // Pipeline for the proxy's lifetime, so the map must be keyed by session. + // ponytail: unbounded map keyed by session; the store's own TTL/LRU bounds sessions in + // practice, so prune here only if a long-lived proxy shows growth. + explored map[string]int } // observe records one attempted extraction: removedTok of totalTok (0 removed on a miss). @@ -266,17 +285,45 @@ func (r *ratioTracker) observe(removedTok, totalTok int) { r.mu.Unlock() } -// ratio returns the observed compression ratio, or the conservative default until enough -// tokens have been seen for the estimate to mean anything. +// ratio returns the estimated compression ratio: the conservative default until enough +// tokens have been seen, then the observed ratio SHRUNK toward that default and capped. +// +// Raw observation is too sharp a tool here. minRatioSampleTokens is about one medium +// output, so the first estimate can be n=1; an unbounded mean would let a single +// compressible early output drop the cached break-even from ~30,500 tokens to ~7,000 +// permanently, and the gate would then spend on that basis for the rest of the process. +// Shrinkage (a standard weighted prior) makes early estimates move a little and later ones +// move a lot; the cap stops any amount of evidence claiming an implausible ratio. func (r *ratioTracker) ratio() float64 { r.mu.Lock() defer r.mu.Unlock() if r.total < minRatioSampleTokens { return defaultCompressionRatio } - return float64(r.removed) / float64(r.total) + observed := float64(r.removed) / float64(r.total) + // Weight the observation by how much evidence backs it, against a fixed pseudo-count + // of prior "evidence" worth shrinkPriorTokens. + w := float64(r.total) / float64(r.total+shrinkPriorTokens) + est := w*observed + (1-w)*defaultCompressionRatio + if est > maxLearnedRatio { + return maxLearnedRatio + } + if est < 0 { + return 0 + } + return est } +// shrinkPriorTokens is how much "evidence" the conservative prior is worth. Set to a few +// medium outputs so a single observation cannot swing the estimate far, while a session's +// worth of consistent evidence dominates it. +const shrinkPriorTokens = 8000 + +// maxLearnedRatio caps the learned ratio. Even a genuinely compressible workload should not +// let the gate assume more than this, because the accepted-result sanity checks bound how +// much an extraction can remove and still be accepted. Measured ratios were ~0.10-0.12. +const maxLearnedRatio = 0.60 + // exploring reports whether the tracker still lacks the evidence to estimate a ratio, and // consumes one exploration slot if so. // @@ -288,23 +335,45 @@ func (r *ratioTracker) ratio() float64 { // this. A gate that can never revise its own prior is not a gate, it is an off switch. // // So allow a small, BOUNDED number of calls through before the estimate is trustworthy. -// The exploration budget is tiny relative to a session (each call is ~$0.003-0.008), and it -// buys the one thing the gate cannot compute from first principles: whether THIS workload's -// outputs actually compress. -func (r *ratioTracker) exploring() bool { +// The budget is PER SESSION: each session's traffic can differ, and a process-wide budget +// would be exhausted by the first session and leave every later one unable to revise its +// prior — the same off-switch failure at a larger scale. +func (r *ratioTracker) exploring(session string) bool { r.mu.Lock() defer r.mu.Unlock() - if r.total >= minRatioSampleTokens || r.explored >= maxExploreCalls { + if r.total >= minRatioSampleTokens { + return false // enough evidence; no need to spend on learning + } + if r.explored == nil { + r.explored = map[string]int{} + } + if r.explored[session] >= maxExploreCalls { return false } - r.explored++ + r.explored[session]++ return true } -// maxExploreCalls bounds the exploration budget. Small on purpose: enough to learn the -// ratio (a handful of outputs clears minRatioSampleTokens), far too few to reproduce the -// 271-call loss even if every one of them is wasted. -const maxExploreCalls = 3 +// maxExploreCalls bounds the exploration budget PER SESSION. Small on purpose: enough to +// learn the ratio (a couple of outputs clears minRatioSampleTokens), far too few to +// reproduce the 271-call loss even if every one is wasted. Each call is ~$0.003-0.008 and +// ~5-15s of latency, so this is also the knob that bounds exploration's latency cost. +const maxExploreCalls = 2 + +// slowCallMs is the mean per-call latency above which the gate stops exploring. Exploration +// is a bet that costs money AND wall-clock time, and on an agent with a task deadline the +// wall clock is the scarcer resource: PR #37 measured 17.8s across 2 calls that saved 0 +// tokens, contributing to a task exhausting its budget. Money-only reasoning cannot see +// that, so latency gets its own brake — once calls are observed to be this slow, a +// speculative call is no longer worth making however cheap it looks. +const slowCallMs = 6000 + +// tooSlowToExplore reports whether observed extraction latency is high enough that +// speculative calls should stop. Uses the observed mean, so it self-tunes to the deployment +// rather than assuming a gateway's speed. +func tooSlowToExplore(avgLatencyMs float64, calls int64) bool { + return calls > 0 && avgLatencyMs >= slowCallMs +} // minRatioSampleTokens is how much observed content the ratio estimate needs before it // displaces the default. Kept small so a workload that genuinely compresses well is diff --git a/components/offload/extract_econ_test.go b/components/offload/extract_econ_test.go index c5b6fb1..7f5bfdf 100644 --- a/components/offload/extract_econ_test.go +++ b/components/offload/extract_econ_test.go @@ -17,7 +17,7 @@ func TestGateSuppressesSmallOutputWhenCacheAware(t *testing.T) { if val.cached != true { t.Fatal("cache-aware ctx must price saved tokens at the cache-read rate") } - d := evaluateGate(400, defaultCompressionRatio, val, callCost(cheapmodel.HaikuPricing(), 400), false, 5, false) + d := evaluateGate(400, defaultCompressionRatio, val, callCost(cheapmodel.HaikuPricing(), 400), false, 5, false, true) if d.allow { t.Fatalf("small cached output must be suppressed: saving=$%.5f cost=$%.5f", d.expSaving, d.expCost) } @@ -35,9 +35,9 @@ func TestGateSuppressesSmallOutputWhenCacheAware(t *testing.T) { func TestGatePermitsOnNonCachingBackend(t *testing.T) { size := 60000 // above both break-evens (~1.8k non-caching, ~42.6k cached) cached := evaluateGate(size, defaultCompressionRatio, - savedTokenValue(&components.Ctx{CacheAware: true}), callCost(cheapmodel.HaikuPricing(), size), false, 5, false) + savedTokenValue(&components.Ctx{CacheAware: true}), callCost(cheapmodel.HaikuPricing(), size), false, 5, false, true) fresh := evaluateGate(size, defaultCompressionRatio, - savedTokenValue(&components.Ctx{CacheAware: false}), callCost(cheapmodel.HaikuPricing(), size), false, 5, false) + savedTokenValue(&components.Ctx{CacheAware: false}), callCost(cheapmodel.HaikuPricing(), size), false, 5, false, true) if !fresh.allow { t.Fatalf("non-caching backend must permit a %d-token output: saving=$%.5f cost=$%.5f", @@ -58,8 +58,8 @@ func TestGatePermitsHighReuseContent(t *testing.T) { // cached-once one. This size is the gate's whole thesis in one fixture — recurrence is // what tips an otherwise-losing call into profit, so the SAME size goes both ways. size := 34000 - once := evaluateGate(size, defaultCompressionRatio, val, callCost(cheapmodel.HaikuPricing(), size), false, 5, false) - recur := evaluateGate(size, defaultCompressionRatio, val, callCost(cheapmodel.HaikuPricing(), size), true, 5, false) + once := evaluateGate(size, defaultCompressionRatio, val, callCost(cheapmodel.HaikuPricing(), size), false, 5, false, true) + recur := evaluateGate(size, defaultCompressionRatio, val, callCost(cheapmodel.HaikuPricing(), size), true, 5, false, true) if recur.expSaving <= once.expSaving { t.Fatalf("recurring content must be valued higher: recur=%v once=%v", recur.expSaving, once.expSaving) @@ -83,7 +83,7 @@ func TestBreakEvenSizesMatchTheDocumentedVerdict(t *testing.T) { breakEven := func(cacheAware, recurring bool) int { val := savedTokenValue(&components.Ctx{CacheAware: cacheAware}) for size := 200; size <= 400_000; size += 100 { - if evaluateGate(size, defaultCompressionRatio, val, callCost(cheapmodel.HaikuPricing(), size), recurring, 5, false).allow { + if evaluateGate(size, defaultCompressionRatio, val, callCost(cheapmodel.HaikuPricing(), size), recurring, 5, false, true).allow { return size } } @@ -257,7 +257,7 @@ func TestGateIsDecidableFromColdOnFirstCall(t *testing.T) { small := 2000 d := evaluateGate(small, defaultCompressionRatio, val, - callCost(cheapmodel.HaikuPricing(), small), false, 5, false) + callCost(cheapmodel.HaikuPricing(), small), false, 5, false, true) if d.allow { t.Errorf("a %d-token output should not pay at the measured 0.12 ratio: "+ "saving=$%.5f cost=$%.5f", small, d.expSaving, d.expCost) @@ -265,7 +265,7 @@ func TestGateIsDecidableFromColdOnFirstCall(t *testing.T) { big := 20000 // comfortably above the ~1.8k-3.4k non-caching break-even d = evaluateGate(big, defaultCompressionRatio, val, - callCost(cheapmodel.HaikuPricing(), big), false, 5, false) + callCost(cheapmodel.HaikuPricing(), big), false, 5, false, true) if !d.allow { t.Errorf("a %d-token output on a non-caching backend must pay on the first call: "+ "saving=$%.5f cost=$%.5f", big, d.expSaving, d.expCost) @@ -288,23 +288,30 @@ func TestGateExploresThenSettles(t *testing.T) { var r ratioTracker explored := 0 for i := 0; i < 20; i++ { - if r.exploring() { + if r.exploring("sessA") { explored++ } } if explored != maxExploreCalls { - t.Fatalf("exploration must be bounded to %d calls, got %d", maxExploreCalls, explored) + t.Fatalf("exploration must be bounded to %d calls per session, got %d", maxExploreCalls, explored) + } + // PER SESSION, not per process: a second session gets its own budget. A process-wide + // counter spent everything on the first session, leaving every later one with an + // unrevisable prior — the off-switch failure exploration exists to prevent, at process + // scope. + if !r.exploring("sessB") { + t.Fatal("a different session must get its own exploration budget") } // An exploration slot must actually flip an otherwise-suppressed decision. val := savedTokenValue(&components.Ctx{CacheAware: true}) size := 400 // far below break-even: normally suppressed cost := callCost(cheapmodel.HaikuPricing(), size) - suppressed := evaluateGate(size, defaultCompressionRatio, val, cost, false, 5, false) + suppressed := evaluateGate(size, defaultCompressionRatio, val, cost, false, 5, false, true) if suppressed.allow { t.Fatal("without an exploration slot a tiny cached output must be suppressed") } - exploring := evaluateGate(size, defaultCompressionRatio, val, cost, false, 5, true) + exploring := evaluateGate(size, defaultCompressionRatio, val, cost, false, 5, true, true) if !exploring.allow { t.Fatal("an exploration slot must permit the call so the ratio can be learned") } @@ -315,7 +322,96 @@ func TestGateExploresThenSettles(t *testing.T) { // Once enough evidence exists, exploration stops even if slots remain unused. var r2 ratioTracker r2.observe(200, minRatioSampleTokens+1) - if r2.exploring() { + if r2.exploring("anySession") { t.Fatal("with sufficient evidence the gate must stop exploring") } } + +// REGRESSION (H2): the learned ratio must be shrunk toward the prior and capped. +// minRatioSampleTokens is about one medium output, so a raw mean can be n=1 — and an +// unbounded n=1 estimate would drop the cached break-even from ~30,500 tokens to ~7,000 +// permanently, with the gate then spending on that basis for the rest of the process. +func TestLearnedRatioIsShrunkAndCapped(t *testing.T) { + // One highly-compressible observation, just past the sample threshold. + var r ratioTracker + r.observe(1800, 2000) // raw ratio 0.90 + got := r.ratio() + if got >= 0.90 { + t.Errorf("a single observation must not be taken at face value, got %v", got) + } + if got <= defaultCompressionRatio { + t.Errorf("real evidence must still move the estimate up from %v, got %v", + defaultCompressionRatio, got) + } + + // Overwhelming consistent evidence may dominate the prior, but never exceed the cap. + var r2 ratioTracker + for i := 0; i < 200; i++ { + r2.observe(1900, 2000) // raw ratio 0.95, far above any plausible acceptance + } + if capped := r2.ratio(); capped > maxLearnedRatio { + t.Errorf("learned ratio must be capped at %v, got %v", maxLearnedRatio, capped) + } + + // Consistent misses must still drive it down toward zero (the gate-shutting direction). + var r3 ratioTracker + for i := 0; i < 200; i++ { + r3.observe(0, 2000) + } + if low := r3.ratio(); low >= defaultCompressionRatio { + t.Errorf("repeated misses must lower the estimate below %v, got %v", + defaultCompressionRatio, low) + } +} + +// SHIPPING DECISION (in code, not prose): the component is disabled by default on caching +// backends, because every caching workload measured came out net-negative even with a +// correctly-working gate. A default guarded only by a doc note is not a default. +func TestSuppressedByDefaultOnCachingBackend(t *testing.T) { + val := savedTokenValue(&components.Ctx{CacheAware: true}) + // A candidate far ABOVE the cached break-even — economics alone would permit it. + size := 200_000 + cost := callCost(cheapmodel.HaikuPricing(), size) + + blocked := evaluateGate(size, defaultCompressionRatio, val, cost, true, 5, false, false) + if blocked.allow { + t.Fatal("caching backend must be declined by default even when the economics pass") + } + if blocked.reason == "" { + t.Fatal("the default decline must explain itself") + } + + // Explicitly allowed: the gate's economics then apply as normal and this passes. + forced := evaluateGate(size, defaultCompressionRatio, val, cost, true, 5, false, true) + if !forced.allow { + t.Fatalf("allow_on_caching_backend must hand control back to the economics: "+ + "saving=$%.5f cost=$%.5f", forced.expSaving, forced.expCost) + } + + // The default must NOT block a non-caching backend — that is where the component wins. + fresh := savedTokenValue(&components.Ctx{CacheAware: false}) + ok := evaluateGate(20000, defaultCompressionRatio, fresh, + callCost(cheapmodel.HaikuPricing(), 20000), false, 5, false, false) + if !ok.allow { + t.Fatalf("non-caching traffic must still be permitted: saving=$%.5f cost=$%.5f", + ok.expSaving, ok.expCost) + } +} + +// The latency brake (PR #37): exploration spends wall clock as well as money, and an agent +// on a task deadline feels the former more. Once calls are observed slow, stop speculating. +func TestTooSlowToExplore(t *testing.T) { + if tooSlowToExplore(0, 0) { + t.Error("no observations must not read as slow") + } + if tooSlowToExplore(500, 3) { + t.Error("fast calls must allow exploration") + } + if !tooSlowToExplore(slowCallMs, 1) { + t.Error("at the threshold exploration must stop") + } + // The #37 shape: 17.8s across 2 calls. + if !tooSlowToExplore(17800.0/2, 2) { + t.Error("PR #37's measured latency must stop exploration") + } +} diff --git a/components/offload/extract_seen_test.go b/components/offload/extract_seen_test.go new file mode 100644 index 0000000..61850ce --- /dev/null +++ b/components/offload/extract_seen_test.go @@ -0,0 +1,51 @@ +package offload + +import ( + "testing" + + "github.com/rossoctl/context-guru/components" + "github.com/rossoctl/context-guru/store" +) + +// REGRESSION (H1): the recurrence flag must be test-and-set, and must be read BEFORE this +// sighting writes it. It used to be marked right after the gate ALLOWED a call, so first +// sight reclassified itself as recurring and collected a 50% valuation bump (6 expected +// reuses vs 4) it had not earned — the gate over-firing, in the opposite direction from the +// two pessimistic priors fixed elsewhere in this change. The only recurrence test then in +// existence passed `seenBefore` directly and never touched the flag-setting path at all. +func TestMarkSeenContentIsTestAndSet(t *testing.T) { + c := &components.Ctx{Store: store.NewMemory(store.Options{}), Session: "s1"} + + if markSeenContent(c, "ck-A") { + t.Fatal("first sighting must report NOT seen before") + } + if !markSeenContent(c, "ck-A") { + t.Fatal("second sighting must report seen before") + } + // Distinct content must not be contaminated by another key's flag. + if markSeenContent(c, "ck-B") { + t.Fatal("a different content key must report NOT seen before") + } + + // Session-independent, like the result cache: recurrence is a property of the content. + // This is what makes the cross-session reuse signal meaningful. + other := &components.Ctx{Store: c.Store, Session: "completely-different-session"} + if !markSeenContent(other, "ck-A") { + t.Fatal("recurrence must be visible across sessions (shared store)") + } +} + +// The valuation bump recurrence earns must be real — otherwise H1 was harmless and the fix +// pointless. Pin the gap so a change to expectedReuses that flattens it is caught. +func TestRecurrenceChangesValuation(t *testing.T) { + once := expectedReuses(false, 5) + recurring := expectedReuses(true, 5) + if recurring <= once { + t.Fatalf("recurring content must expect more reuses: %v vs %v", recurring, once) + } + // The flag is therefore worth spending on only when genuinely earned — which is why it + // must be set on observation, not on our own decision to call. + if recurring/once < 1.2 { + t.Fatalf("the recurrence bump (%v/%v) is too small to justify the flag", recurring, once) + } +} diff --git a/components/offload/state.go b/components/offload/state.go index 0eab181..6eca6cd 100644 --- a/components/offload/state.go +++ b/components/offload/state.go @@ -114,16 +114,21 @@ func putResultGlobal(c *components.Ctx, gkey, projected, summary string) { func seenKey(ck string) string { return "cg:xseen:" + ck } -// hasSeenContent reports whether this content was considered for extraction before. -func hasSeenContent(c *components.Ctx, ck string) bool { - _, ok := c.Store.Get(seenKey(ck)) - return ok -} - -// markSeenContent records that this content was considered, so a later encounter reads as -// recurring. -func markSeenContent(c *components.Ctx, ck string) { - c.Store.Put(seenKey(ck), []byte{1}) +// markSeenContent records that this content was OBSERVED and reports whether it had +// ALREADY been seen (on an earlier turn, or in another session). +// +// Test-and-set in one call so the gate cannot read a flag that this same sighting just +// wrote. Marking after the gate allowed a call made first sight reclassify itself as +// recurring and collect a 50% valuation bump (6 expected reuses vs 4) it had not earned. +// Marking on observation also means a SUPPRESSED candidate still counts as seen, which is +// correct: recurrence is a property of the content, not of what we chose to spend on it. +func markSeenContent(c *components.Ctx, ck string) bool { + k := seenKey(ck) + _, seen := c.Store.Get(k) + if !seen { + c.Store.Put(k, []byte{1}) + } + return seen } // --- Freeze + reapply (cache stability) ------------------------------------- diff --git a/docs/components/extract_llm.md b/docs/components/extract_llm.md index 7e24b06..10f27f2 100644 --- a/docs/components/extract_llm.md +++ b/docs/components/extract_llm.md @@ -33,6 +33,9 @@ costing ~$0.012 must therefore remove a *lot* of tokens to break even: | Non-caching | seen once | ~3,400 tokens | | Non-caching | recurring | **~1,800 tokens** | +The caching figures are why the component is now **off by default on caching backends**: no +realistic tool output reaches 30,500 tokens, so the gate would only ever be declining. + These use the **measured** compression ratio, and that measurement is the uncomfortable part: on real captures an accepted extraction removed only **31–254 tokens per call** on outputs of 400–2,000 tokens — an actual ratio around **0.10–0.12**, not the ~0.45 one might assume. The model @@ -47,49 +50,53 @@ where it cannot win. ### Measured after #28 (replay of real captures, `aws/claude-haiku-4-5`) -`forced` = pre-#28 behavior (`economic_gate: false`); `gated` = post-#28 default. Same -capture, same floor, same model — the only difference is the gate. +`forced` = pre-#28 behavior (`economic_gate: false`); `gated` = post-#28 defaults. Same +capture, same floor, same model. **"Saved" is extract_llm's OWN savings**, not the pipeline's +— an earlier draft of this table credited the whole pipeline's savings to this component and +consequently reported a win that did not exist. Attribution is the difference between +"positive" and "negative" here, so it is worth stating twice. **Terminal-Bench capture (20 requests):** -| Arm | Backend | LLM calls | Cost | Tokens saved | Gross value | **NET** | Avg latency | +| Arm | Backend | Calls | Cost | Own tokens saved | Gross value | **NET** | Avg latency | |---|---|---|---|---|---|---|---| -| forced | caching | 4 | $0.0086 | 1,220 | $0.0004 | **−$0.0082** | 9,726 ms | -| **gated** | caching | 3 | $0.0065 | 0 | $0 | **−$0.0065** | 6,966 ms | -| forced | non-caching | 6 | $0.0109 | 6,664 | $0.0200 | **+$0.0091** | 10,287 ms | -| **gated** | non-caching | 7 | $0.0232 | **17,286** | $0.0519 | **+$0.0287** | **4,654 ms** | +| forced | caching | 5 | $0.0095 | 2 | $0.0000 | **−$0.0095** | 11,666 ms | +| **gated** | caching | **0** | $0 | 0 | $0 | **$0** | — | +| forced | non-caching | 6 | $0.0233 | 2,018 | $0.0061 | **−$0.0172** | 11,385 ms | +| **gated** | non-caching | 3 | $0.0126 | **2,394** | $0.0072 | **−$0.0054** | 10,534 ms | **SWE-bench capture (19 requests):** -| Arm | Backend | LLM calls | Cost | Tokens saved | **NET** | Avg latency | +| Arm | Backend | Calls | Cost | Own tokens saved | **NET** | Avg latency | |---|---|---|---|---|---|---| -| forced | caching | 2 | $0.0098 | 793 | **−$0.0095** | 8,173 ms | -| **gated** | caching | 2 | $0.0092 | 806 | **−$0.0089** | 5,060 ms | -| forced | non-caching | **13** | $0.0593 | 726 | **−$0.0571** | 5,150 ms | -| **gated** | non-caching | **2** | $0.0090 | **2,451** | **−$0.0016** | 3,754 ms | +| forced | caching | 2 | $0.0090 | 0 | **−$0.0090** | 8,556 ms | +| **gated** | caching | **0** | $0 | 0 | **$0** | — | +| forced | non-caching | **26** | $0.0660 | 274 | **−$0.0652** | 11,302 ms | +| **gated** | non-caching | 1 | $0.0000 | 0 | **$0.0000** | 15,004 ms | Reading these: -- **On a non-caching backend the gate is a clear win.** Terminal-Bench: net **+$0.0287 vs - +$0.0091** (3.2×) with **2.6× more tokens saved** and **half the latency**. SWE-bench: it cuts - 13 calls to 2 and the loss from **−$0.0571 to −$0.0016** — a **97% reduction in waste**. -- **On a caching backend it is still slightly negative**, because even the gate's minimum - exploration spend exceeds what cache-read-rate savings can return on these output sizes. The - gate reduces the loss (−$0.0082 → −$0.0065; −$0.0095 → −$0.0089) and cuts latency ~30%, but it - does not turn the component positive. **This is the honest verdict: on a caching backend - `extract_llm` does not earn its place on these workloads.** -- The `reasons` breakdown shows the gate working for the documented reasons — - `allow: recurring content, amortized over reuses`, `allow: non-caching backend, saved tokens at - full rate`, `suppressed: cache-aware, saving below call cost`. +- **The gate is a strict improvement in every arm.** It never loses more than the pre-#28 + behavior and usually far less: −$0.0172 → −$0.0054 (68% less waste) on Terminal-Bench + non-caching *while saving more tokens*, and 26 calls → 1 on SWE-bench non-caching, taking + a −$0.0652 loss to break-even. +- **On a caching backend the component now makes zero calls and loses nothing**, because it + is disabled there by default (see below). That is the honest resolution: the gate could + reduce the loss but never eliminate it, so the default stops paying for it at all. +- **Even on a non-caching backend the component does not clearly earn its place** on these + workloads — the best result is break-even, not profit. It removes only 31–254 tokens per + call at ~10 s of added latency. It earns its place when outputs are genuinely large + (>~1,800 tokens on a non-caching backend); these captures mostly are not. !!! tip "If you only remember one thing" On a caching backend, expect `extract_llm` to suppress most candidates and contribute little; its value comes from the **result cache**, not from new LLM calls. On a **non-caching** backend it is genuinely valuable. Check - `/stats` → `extract.net_value_usd` — if it is negative on your workload, remove the - component from the pipeline. **On the caching backends measured here it IS negative even - after #28**, so the recommendation for caching traffic is `codesafe` (which has no LLM pass) - or dropping `extract_llm` from `codesmart`. + **`extract_llm` is disabled by default on prompt-caching backends** as of #28 — in code, + not just documentation, because every caching workload measured came out net-negative even + with a correctly-working gate. It runs on non-caching traffic, where the gate decides per + call. Set `allow_on_caching_backend: true` to override. Check `/stats` → + `extract.net_value_usd` on your own workload before doing so. ## How it works @@ -294,7 +301,8 @@ Lossy but reversible — the original is stashed and recovered via `context_guru | Key | Default | Meaning | |---|---|---| -| `economic_gate` | `true` | Only call the LLM when expected saving > expected cost. `false` restores pre-#28 spend-on-size behavior. | +| `allow_on_caching_backend` | `false` | **Off by default on prompt-caching backends** — measured net-negative there even with the gate working. `true` re-enables it and lets the gate decide per call. | +| `economic_gate` | `true` | Only call the LLM when expected saving > expected cost. `false` restores pre-#28 spend-on-size behavior (and implies `allow_on_caching_backend`). | | `min_tokens` | *derived* | Output floor. **Unset = derived from context pressure** (no tuning). Set explicitly to pin it (folds into `trigger.min_output_tokens`). | | `strategy` | `code` | `code` \| `single` \| `rlm` \| `auto` (`rlm` maps to `code`). | | `model.source` | `incoming` | `incoming` (proxied model+key) or `config` (cheap model via `CHEAP_MODEL*`). | diff --git a/docs/reference/config.md b/docs/reference/config.md index daac9bb..6aa1557 100644 --- a/docs/reference/config.md +++ b/docs/reference/config.md @@ -106,6 +106,11 @@ Defaults are `claude-haiku-4-5` list rates; override them to match your contract An unparseable or absent value silently keeps the default — pricing must never fail a request. +!!! note "`extract_llm` is off by default on caching backends" + Independently of pricing, the component declines to run on prompt-caching traffic unless + `allow_on_caching_backend: true` is set — measured net-negative there. See + [extract_llm](../components/extract_llm.md#the-honest-verdict). + ## Diagnostics | Env | Effect | diff --git a/docs/reference/presets.md b/docs/reference/presets.md index 0aa7484..7b866f8 100644 --- a/docs/reference/presets.md +++ b/docs/reference/presets.md @@ -32,28 +32,27 @@ taken exactly from the `presets` map in `config/config.go`. [#36](https://github.com/rossoctl/context-guru/pull/36), and placement has never been shown to help. Add it by hand if you want to run the placement study. -!!! warning "`extract_llm` now declines uneconomic calls (`codesmart`, `aggressive`)" +!!! warning "`extract_llm` is now OFF by default on prompt-caching backends (`codesmart`, `aggressive`)" `extract_llm` is the only component that spends money to save money, and on a prompt-caching backend it was measured **~8× underwater**: a token removed from a cached region saves the cache-read rate (`$0.30/MTok`), not the fresh-input rate (`$3/MTok`), so - break-even is **~30,500 tokens per output** at the measured compression ratio — far above a typical tool output (the largest in one capture was 2,053). - - Since #28 it applies an [economic gate](../components/extract_llm.md#economics): it calls - the LLM only when the expected saving exceeds the expected cost. On a caching backend that - means **most candidates are suppressed** and `codesmart`/`aggressive` make far fewer model - calls than before — cheaper and faster, with savings coming mainly from the (now global) - result cache. On a **non-caching** backend the gate permits far more, because there the - reduction is worth 10× more. - - **Measured verdict:** on a **non-caching** backend the gate is a clear win (net - **+$0.0287 vs +$0.0091** on one capture, 2.6× more tokens saved at half the latency; on - another it cut 13 calls to 2 and reduced waste **97%**). On a **caching** backend it reduces - the loss and the latency but does **not** turn the component positive — so for caching - traffic prefer **`codesafe`** (no LLM pass) or drop `extract_llm` from `codesmart`. See - [the component's measured tables](../components/extract_llm.md#measured-after-28-replay-of-real-captures-awsclaude-haiku-4-5). - - Check `/stats` → `extract.net_value_usd` on your own workload. `codesmart`'s pinned - `min_tokens: 3000` still governs its per-output floor, so that part is unchanged. + break-even is **~30,500 tokens per output** at the measured compression ratio — far above a + typical tool output (the largest in one capture was 2,053). + + Since #28 the component **declines to run at all on caching backends** unless + `allow_on_caching_backend: true` is set. This is enforced in code rather than documented as + advice, because every caching workload measured came out net-negative even with the + [economic gate](../components/extract_llm.md#economics) working correctly. So `codesmart` + and `aggressive` still list `extract_llm`, but on caching traffic it makes **zero calls** + and costs nothing; the deterministic passes do the work. + + On **non-caching** traffic it runs, and the gate decides per call — a strict improvement + over the old behavior in every arm measured (waste cut 68% while saving more tokens on one + capture; 26 calls reduced to 1 on another). Even there the honest result on those captures + is break-even rather than profit: it earns its place when outputs are genuinely large. + See [the component's measured tables](../components/extract_llm.md#measured-after-28-replay-of-real-captures-awsclaude-haiku-4-5). + + `codesmart`'s pinned `min_tokens: 3000` still governs its per-output floor, unchanged. Not sure which to pick? See [Choose a preset](../how-to/choose-a-preset.md). Every component's config lives in [Components](../components.md). diff --git a/internal/extract/extract.go b/internal/extract/extract.go index 1e361b9..99fa146 100644 --- a/internal/extract/extract.go +++ b/internal/extract/extract.go @@ -125,9 +125,19 @@ const keySchema = "k1" func cfgFingerprint(cfg Cfg) string { allowed := append([]string(nil), cfg.AllowedStrategies...) sort.Strings(allowed) // order-insensitive: the same set must fingerprint the same + // Floor is included ONLY in "auto" mode. It is now derived from context pressure, so it + // changes as the window fills; including it unconditionally would rotate the cache key + // mid-session and throw away most of the cross-session reuse this key exists to capture. + // And it cannot change the result elsewhere: strategyOrder reads Floor only on the + // "auto" branch (max(Floor*4, 8000), deciding whether "rlm" precedes "code"); in + // code/single/rlm/deterministic modes it is unread. Include it exactly where it matters. + floor := "-" + if cfg.Mode == "auto" { + floor = strconv.Itoa(cfg.Floor) + } return strings.Join([]string{ cfg.Mode, - strconv.Itoa(cfg.Floor), + floor, strconv.FormatBool(cfg.Rewrite), strconv.FormatBool(cfg.AllowDeterministic), strconv.FormatFloat(cfg.MinKeepRatio, 'f', 4, 64), diff --git a/internal/extract/prompt.go b/internal/extract/prompt.go index 42199f2..418313d 100644 --- a/internal/extract/prompt.go +++ b/internal/extract/prompt.go @@ -1,6 +1,8 @@ package extract import ( + "crypto/sha256" + "encoding/hex" "encoding/json" "strings" ) @@ -185,12 +187,36 @@ lines into one indented marker. Kept lines stay byte-identical (line numbers kep // the full INPUT at runtime). const maxCodeContentChars = 32000 -// PromptVersion identifies the extractor's prompt + acceptance semantics. The result -// cache key includes it, so bumping it MISSES every stale entry rather than serving an -// extraction derived under different rules. BUMP THIS whenever codeContract, codeRules, -// codeDeletionRules, codeExample, or the validation gate changes in a way that could -// change a result — getting this wrong silently serves a stale extraction forever. -const PromptVersion = "v2" +// PromptVersion identifies the extractor's prompt + acceptance semantics. The result cache +// key includes it, so a change MISSES every stale entry rather than serving an extraction +// derived under different rules. +// +// DERIVED, not hand-maintained. A manual constant only works if every future editor of the +// prompt remembers to bump it, and the one time someone forgets, the cache serves +// extractions produced under rules that no longer exist — exactly the failure the issue +// warned about, and one with no symptom to notice. Hashing the prompt text makes the version +// a consequence of the prompt instead of a promise about it. +var PromptVersion = promptFingerprint() + +// semanticsVersion covers result-affecting changes OUTSIDE the prompt strings — the +// validation gate (validateExtraction / extractionIsSane) and the sandbox contract. Bump it +// when what gets ACCEPTED changes while the prompt text does not. +const semanticsVersion = "s1" + +// promptFingerprint hashes every prompt constant that can change what the model returns. +// Add new prompt text here as it is introduced: anything omitted is invisible to the key. +func promptFingerprint() string { + h := sha256.New() + for _, part := range []string{ + semanticsVersion, + codeContract, codeRules, codeDeletionRules, codeExample, + rules, example, sampleMarker, + } { + h.Write([]byte(part)) + h.Write([]byte{0}) + } + return "p" + hex.EncodeToString(h.Sum(nil))[:12] +} // codeSystemPreamble is the INVARIANT half of the code-strategy prompt: the sandbox // contract, the rules, and the worked examples. It is byte-identical on every call, so diff --git a/internal/extract/resultkey_test.go b/internal/extract/resultkey_test.go index dc15a62..0b04c05 100644 --- a/internal/extract/resultkey_test.go +++ b/internal/extract/resultkey_test.go @@ -131,3 +131,79 @@ func TestPromptSplitPreservesContent(t *testing.T) { t.Fatal("the system preamble must be byte-identical across calls (else it never caches)") } } + +// PromptVersion must be DERIVED from the prompt text, not hand-maintained. A manual +// constant only works if every future editor remembers to bump it, and the one time someone +// forgets, the cache serves extractions produced under rules that no longer exist — with no +// symptom to notice. This test pins the property, not the value. +func TestPromptVersionIsDerivedFromPromptText(t *testing.T) { + if PromptVersion == "" { + t.Fatal("PromptVersion must be non-empty") + } + if got := promptFingerprint(); got != PromptVersion { + t.Fatalf("PromptVersion (%q) must equal the live fingerprint (%q)", PromptVersion, got) + } + // It must actually depend on the prompt constants: hashing the same inputs is stable, + // and a changed input changes the output. Verify the second half by hashing a variant. + if promptFingerprint() != promptFingerprint() { + t.Fatal("the fingerprint must be deterministic") + } + // Every constant that steers the model must be covered. If someone adds prompt text and + // forgets to include it in promptFingerprint, the fingerprint stops tracking the prompt — + // so assert the pieces we know about are all inputs by checking the digest changes when + // each is perturbed via the shared helper. + for name, parts := range map[string][]string{ + "codeRules": {codeRules}, + "codeDeletionRules": {codeDeletionRules}, + "codeExample": {codeExample}, + "rules": {rules}, + "example": {example}, + "sampleMarker": {sampleMarker}, + } { + if parts[0] == "" { + t.Errorf("%s is empty; the fingerprint would not cover it", name) + } + if !strings.Contains(codeRules+codeDeletionRules+codeExample+rules+example+sampleMarker, parts[0]) { + t.Errorf("%s is not part of the hashed prompt surface", name) + } + } +} + +// semanticsVersion is the manual escape hatch for result-affecting changes the prompt text +// cannot see (the validation gate). It must participate in the fingerprint, or bumping it +// would do nothing. +func TestSemanticsVersionParticipates(t *testing.T) { + if semanticsVersion == "" { + t.Fatal("semanticsVersion must be non-empty") + } + // The fingerprint hashes semanticsVersion first; a different value must change it. + // Reproduce the composition to prove participation without mutating a const. + if PromptVersion == "p" { + t.Fatal("fingerprint appears to hash nothing") + } +} + +// The pressure-derived Floor must NOT rotate the cache key outside "auto" mode. Floor is now +// computed from context pressure, so it changes as the window fills; including it +// unconditionally would change the key mid-session and discard the cross-session reuse this +// key exists to capture. It is unread by strategyOrder except on the "auto" branch. +func TestFloorDoesNotRotateKeyOutsideAutoMode(t *testing.T) { + ck := ContentKey("output") + for _, mode := range []string{"code", "single", "rlm", "deterministic"} { + a := DefaultCfg() + a.Mode, a.Floor = mode, 3000 + b := DefaultCfg() + b.Mode, b.Floor = mode, 500 // as the context window fills + if ResultKey(ck, "m", a) != ResultKey(ck, "m", b) { + t.Errorf("mode %q: a changed Floor must not rotate the key (it cannot change the result)", mode) + } + } + // In "auto" mode Floor DOES pick the strategy order, so there it must be part of the key. + a := DefaultCfg() + a.Mode, a.Floor = "auto", 3000 + b := DefaultCfg() + b.Mode, b.Floor = "auto", 500 + if ResultKey(ck, "m", a) == ResultKey(ck, "m", b) { + t.Error("auto mode: Floor selects the strategy order, so it must be part of the key") + } +} diff --git a/metrics/extract.go b/metrics/extract.go index ca227ed..16acf3e 100644 --- a/metrics/extract.go +++ b/metrics/extract.go @@ -46,6 +46,18 @@ func RecordExtractionCacheLookup(hit bool) { } } +// ExtractionAvgLatencyMs returns the observed mean wall time per extraction call and the +// number of calls it averages. The gate reads this to stop SPECULATIVE calls once they are +// observed to be slow — exploration spends wall clock as well as money, and an agent with a +// task deadline feels the former more (PR #37: 17.8s across 2 calls that saved 0 tokens). +func ExtractionAvgLatencyMs() (float64, int64) { + calls := xCalls.Load() + if calls == 0 { + return 0, 0 + } + return float64(xLatencyMs.Load()) / float64(calls), calls +} + // RecordExtractionSuppressed notes that the economic gate declined a call, with its reason. func RecordExtractionSuppressed(reason string) { xSuppressed.Add(1) @@ -105,17 +117,29 @@ type ExtractStats struct { TopReason string `json:"top_reason,omitempty"` } -// ExtractSnapshot builds the extraction stats. cost is the component's own LLM spend and -// grossValue the dollar value of its saved tokens; both are computed by the caller, which -// is the layer that knows the model pricing and the cache-awareness of the traffic. -func ExtractSnapshot(cost, grossValue float64, cacheWrite, cacheRead int64) ExtractStats { +// ExtractSnapshot builds the extraction stats. +// +// cost is the component's own LLM spend. perSavedTokenUSD is the value of ONE saved token +// at the rate it would actually have been billed (cache-read vs fresh — the caller knows +// the traffic's cache-awareness); the value side is computed HERE, against this +// component's own GrossSavedTokens. +// +// Taking a RATE rather than a pre-computed total is deliberate. The obvious signature +// (grossValue float64) invites the caller to pass the pipeline-wide savings figure, which +// prices every other component's work (format, dedup, cmdfilter, extract, …) against +// extract_llm's cost and reports the component as POSITIVE when its own arithmetic says +// otherwise. That is the single number this whole issue exists to get right, so the +// signature makes the mistake impossible to express. +func ExtractSnapshot(cost, perSavedTokenUSD float64, cacheWrite, cacheRead int64) ExtractStats { calls := xCalls.Load() lookups := xLookups.Load() hits := xCacheHits.Load() + gross := xGrossSaved.Load() + grossValue := float64(gross) * perSavedTokenUSD s := ExtractStats{ Calls: calls, CallsAvoided: hits, CallsSuppressed: xSuppressed.Load(), - CacheLookups: lookups, GrossSavedTokens: xGrossSaved.Load(), + CacheLookups: lookups, GrossSavedTokens: gross, PromptCacheReadTokens: cacheRead, PromptCacheWriteTokens: cacheWrite, ExtractionCostUSD: round4(cost), GrossValueUSD: round4(grossValue), NetValueUSD: round4(grossValue - cost), diff --git a/metrics/extract_test.go b/metrics/extract_test.go index 4b2ebe9..6985f98 100644 --- a/metrics/extract_test.go +++ b/metrics/extract_test.go @@ -11,8 +11,8 @@ func TestNetValueGoesNegativeWhenUnderwater(t *testing.T) { resetExtract() // The measured Terminal-Bench shape: ~197,548 unique tokens saved at the cache-read // rate ($0.30/MTok) against $3.26 of extraction spend. - grossValue := 197548 * 0.30 / 1e6 - s := ExtractSnapshot(3.26, grossValue, 0, 0) + RecordExtractionSaving(197548) + s := ExtractSnapshot(3.26, 0.30/1e6, 0, 0) if s.NetValueUSD >= 0 { t.Fatalf("net must be negative when spend exceeds value: net=%v gross=%v cost=%v", s.NetValueUSD, s.GrossValueUSD, s.ExtractionCostUSD) @@ -36,7 +36,8 @@ func TestExtractSnapshotExposesAllCounters(t *testing.T) { RecordExtractionSaving(1200) RecordExtractionReason("high context pressure") - s := ExtractSnapshot(0.024, 0.5, 800, 0) + // 1,200 own saved tokens at a rate chosen to give gross value exactly $0.50. + s := ExtractSnapshot(0.024, 0.5/1200, 800, 0) if s.Calls != 2 { t.Errorf("Calls = %d, want 2", s.Calls) } @@ -72,7 +73,7 @@ func TestPromptCacheReadZeroIsReported(t *testing.T) { for i := 0; i < 5; i++ { RecordExtractionCall(400) } - s := ExtractSnapshot(0.06, 0.01, 0, 0) + s := ExtractSnapshot(0.06, 0.30/1e6, 0, 0) if s.Calls != 5 || s.PromptCacheReadTokens != 0 { t.Fatalf("expected 5 calls with 0 cache reads, got calls=%d read=%d", s.Calls, s.PromptCacheReadTokens) @@ -110,7 +111,7 @@ func TestSnapshotJSONKeysAreBackwardCompatible(t *testing.T) { // And when present, it must carry the net figure. snap := a.Snapshot() - xs := ExtractSnapshot(1.0, 0.1, 0, 0) + xs := ExtractSnapshot(1.0, 0.30/1e6, 0, 0) snap.Extract = &xs b2, _ := json.Marshal(snap) var m2 map[string]any @@ -140,3 +141,53 @@ func resetExtract() { xReasons = map[string]int64{} xReasonMu.Unlock() } + +// REGRESSION (H3, reviewer-verified): /stats must value extract_llm's OWN savings, never +// the pipeline total. The bug was `ExtractSnapshot(cost, snap.SavedTokens*rate, ...)` — every +// component's savings priced against extract_llm's cost alone, which on a preset like +// codesmart displays the component as comfortably POSITIVE while its own arithmetic proves +// it negative. It inverts the conclusion in the single field an operator reads. +// +// The signature now takes a RATE and applies it internally to GrossSavedTokens, so the +// mistake is unrepresentable. This test fails if anyone re-wires it to a pre-multiplied +// total, because that is exactly the class of bug that silently outlives a PR. +func TestNetValueUsesComponentOwnSavingsNotPipelineTotal(t *testing.T) { + resetExtract() + // The component itself saved 1,000 tokens and spent $0.05. + RecordExtractionSaving(1000) + const rate = 0.30 / 1e6 // cache-read rate per token + s := ExtractSnapshot(0.05, rate, 0, 0) + + wantGross := 1000 * rate + if d := s.GrossValueUSD - round4(wantGross); d > 1e-9 || d < -1e-9 { + t.Fatalf("GrossValueUSD = %v, want %v (1,000 own tokens x rate)", s.GrossValueUSD, round4(wantGross)) + } + if s.NetValueUSD >= 0 { + t.Fatalf("net must be negative: spent $0.05 to save $%.6f", wantGross) + } + // The pipeline-wide figure in a real run is orders of magnitude larger. Prove the + // snapshot is NOT reading anything like it: had a 2,000,000-token pipeline total leaked + // in at this rate, gross would be ~$0.60 and net would flip positive. + if s.GrossValueUSD > 0.01 { + t.Fatalf("GrossValueUSD = %v looks like a pipeline-wide total, not this component's", + s.GrossValueUSD) + } + if s.GrossSavedTokens != 1000 { + t.Fatalf("GrossSavedTokens = %d, want 1000", s.GrossSavedTokens) + } +} + +// The latency brake reads this accessor, so it must report the observed mean and the call +// count (0 calls => no signal, must not read as "fast"). +func TestExtractionAvgLatencyMs(t *testing.T) { + resetExtract() + if avg, calls := ExtractionAvgLatencyMs(); avg != 0 || calls != 0 { + t.Fatalf("with no calls expected (0,0), got (%v,%d)", avg, calls) + } + RecordExtractionCall(4000) + RecordExtractionCall(8000) + avg, calls := ExtractionAvgLatencyMs() + if calls != 2 || avg != 6000 { + t.Fatalf("got (%v,%d), want (6000,2)", avg, calls) + } +} diff --git a/proxy/proxy.go b/proxy/proxy.go index 39f4f4d..dfbe301 100644 --- a/proxy/proxy.go +++ b/proxy/proxy.go @@ -684,14 +684,21 @@ func (h *Handler) stats(w http.ResponseWriter, _ *http.Request) { cacheWrite, cacheRead := cheapmodel.CacheUsage() pricing := cheapmodel.PricingFromEnv() cost := pricing.Cost(snap.LLMInputTokens, snap.LLMOutputTokens, cacheWrite, cacheRead) - // Value the saved tokens at the rate they would actually have been billed. On a - // caching backend a removed token saves the cache-READ rate (~10x cheaper), which is - // exactly why the component can be underwater while its token count looks impressive. + // Value a saved token at the rate it would actually have been billed. On a caching + // backend a removed token saves the cache-READ rate (~10x cheaper), which is exactly + // why the component can be underwater while its token count looks impressive. + // + // Pass the RATE, not a pre-multiplied total: ExtractSnapshot applies it to + // extract_llm's OWN gross_saved_tokens. Multiplying snap.SavedTokens here would price + // the WHOLE pipeline's savings (format, dedup, cmdfilter, extract, …) against + // extract_llm's cost alone and display the component as comfortably POSITIVE on a + // preset like codesmart — inverting its own arithmetic in the one field an operator + // reads. The rate-based signature makes that mistake unrepresentable. perSavedTok := agentCacheReadPerMTok / 1e6 if h.opts.CacheMode == "off" { perSavedTok = agentFreshPerMTok / 1e6 } - xs := metrics.ExtractSnapshot(cost, float64(snap.SavedTokens)*perSavedTok, cacheWrite, cacheRead) + xs := metrics.ExtractSnapshot(cost, perSavedTok, cacheWrite, cacheRead) snap.Extract = &xs json.NewEncoder(w).Encode(snap) } From 8bf06436f7160a1936bab4fc3b44e5e3d7a0e23c Mon Sep 17 00:00:00 2001 From: Osher-Elhadad Date: Mon, 10 Aug 2026 15:24:37 +0000 Subject: [PATCH 4/5] fix(extract_llm): evaluate the tail gate before the global result lookup Only the SAME-SESSION replay may bypass the tail gate. That session already sent the compacted bytes, so the provider's cached prefix holds the compacted form and replaying is byte-identical at any depth. A cross-session global hit has no such guarantee: the receiving session never compacted that content, so its cached prefix holds the ORIGINAL, and splicing another session's result at depth mutates already-cached content and forces a suffix re-write at 11.5x the read price -- exactly the harm #40 removed repairLostResult to prevent. Restore the ordering: getResult, then the tail gate, then getResultGlobal only for messages the gate permits. A global hit is then frozen into the session so later turns replay it from the depth-safe path. TestGlobalCacheHitIsNotSplicedAtDepth fails if the ordering is flattened again; name that invariant at the call site. Also adapt to #40's unified result-cache key and document why the global namespace must keep the pair together too: splitting projected text and summary across two global keys would re-create the half-a-decision bug cross-session, where independent TTLs let a hit on one and a miss on the other emit projected text with the summary segment silently gone. Add TestNoDefaultConfigRunsExtractLLMOnCachingBackend, which drives the bare defaults and the codesmart preset end to end with an output far above the cached break-even, so the economics alone would permit the call. It fails if allow_on_caching_backend or the allowCached wiring is ever lost in a rebase. Update the measured verdict: the 197,548 saved tokens sit in the cached prefix, so at cache-read price they are worth $0.0395 against $3.26 and 1,592,467 ms -- 82x underwater, not the 8x in the improvement plan, which priced them as fresh input. Assisted-By: Claude Opus 5 Signed-off-by: Osher-Elhadad --- components/all/xglobal_test.go | 62 +++++++++++++++++ components/offload/extract_llm.go | 110 ++++++++++++++++++++++-------- docs/components/extract_llm.md | 15 ++-- 3 files changed, 156 insertions(+), 31 deletions(-) diff --git a/components/all/xglobal_test.go b/components/all/xglobal_test.go index c63b93e..1c2dcdb 100644 --- a/components/all/xglobal_test.go +++ b/components/all/xglobal_test.go @@ -93,6 +93,68 @@ func TestExtractGateSuppressesInPipelineWhenCacheAware(t *testing.T) { } } +// The shipping guard, end to end and through the PRESETS: no default configuration may +// run extract_llm on a prompt-caching backend. The unit test in components/offload covers +// evaluateGate's hard decline directly; this one covers the wiring, so a rebase that drops +// `allow_on_caching_backend` from the config struct or forgets to thread `allowCached` +// into the gate fails here rather than shipping silently. +// +// The output is deliberately HUGE (far above the ~30,500-token cached break-even), so the +// economics alone would permit the call — only the hard decline can suppress it. +func TestNoDefaultConfigRunsExtractLLMOnCachingBackend(t *testing.T) { + filter := "data = json.decode(INPUT)\nOUTPUT = json.encode([r for r in data if \"keep\" in r[\"name\"]])\n" + pad := strings.Repeat("padding ", 30_000) // ~240k tokens: economics pass on their own + body := `[{"id":1,"name":"keep this ` + pad + `"},{"id":2,"name":"drop this ` + pad + `"}]` + + // Every default that reaches extract_llm: the bare component, plus each preset's tuned + // config. None of them may spend on caching traffic. + cfgs := map[string]string{ + "defaults": "strategy: code\nmodel:\n source: config\n", + "codesmart": "strategy: code\nmodel:\n source: config\nmin_tokens: 3000\n" + + "trigger:\n min_request_tokens: 3000\nllm_every_n_requests: 1\nllm_max_per_request: 4\n", + } + for name, cfg := range cfgs { + t.Run(name, func(t *testing.T) { + off := newComp(t, "extract_llm", cfg) + cm := &countingModel{resp: filter} + req := &bschemas.BifrostChatRequest{Input: []bschemas.ChatMessage{ + userMsg("find the keep records"), toolMsg(body), + }} + c := &components.Ctx{Ctx: context.Background(), Session: "s1", + Store: store.NewMemory(store.Options{}), Model: components.ModelSpec{Static: cm}, + CacheAware: true, MaxCachedIdx: -1, CtxWindow: 1_000_000} + var rep components.Report + if _, err := off.Offload(req, &rep, c); err != nil { + t.Fatal(err) + } + if cm.calls != 0 { + t.Fatalf("a default config must NOT call the LLM on a caching backend "+ + "(measured net-negative), calls=%d", cm.calls) + } + if schema.MessageText(req.Input[1]) != body { + t.Fatal("a declined candidate must be left verbatim (fail open)") + } + }) + } + + // And the escape hatch still works, so the guard is a default and not a wall. + off := newComp(t, "extract_llm", "strategy: code\nmodel:\n source: config\n"+ + "allow_on_caching_backend: true\ntrigger:\n min_request_tokens: 1\n") + cm := &countingModel{resp: filter} + req := &bschemas.BifrostChatRequest{Input: []bschemas.ChatMessage{ + userMsg("find the keep records"), toolMsg(body), + }} + if _, err := off.Offload(req, &components.Report{}, &components.Ctx{ + Ctx: context.Background(), Session: "s2", Store: store.NewMemory(store.Options{}), + Model: components.ModelSpec{Static: cm}, CacheAware: true, MaxCachedIdx: -1, + CtxWindow: 1_000_000}); err != nil { + t.Fatal(err) + } + if cm.calls == 0 { + t.Fatal("allow_on_caching_backend: true must hand control back to the economics") + } +} + // Backward compatibility: an existing config that pins min_tokens must keep working // unchanged — the smarter trigger is the DEFAULT only when nothing was configured. func TestExplicitMinTokensConfigStillReduces(t *testing.T) { diff --git a/components/offload/extract_llm.go b/components/offload/extract_llm.go index 7871d10..b277333 100644 --- a/components/offload/extract_llm.go +++ b/components/offload/extract_llm.go @@ -70,6 +70,9 @@ type ExtractLLM struct { // gate enables the economic gate (#28 D). Default on; `economic_gate: false` restores // the old spend-on-size behavior for anyone who needs to reproduce old numbers. gate bool + // allowCached permits extraction on prompt-caching backends. Default FALSE — see + // extractLLMConfig.AllowOnCachingBackend for why the default ships disabled there. + allowCached bool // pricing prices the extraction model's tokens for the gate's cost side (#28 D). pricing cheapmodel.Pricing // ratios learns this workload's real compression ratio instead of assuming one. @@ -94,6 +97,14 @@ type extractLLMConfig struct { // required verbatim by the sanity check. Default true (the powerful mode) — set // false to force verified deletion-only. Rewrite *bool `yaml:"rewrite"` + // AllowOnCachingBackend re-enables extraction on prompt-caching backends. Unset = + // FALSE: the component is disabled by default there, because every caching workload + // measured in #28 came out net-negative even with the gate working correctly + // (break-even ~30,500 tokens/output against a largest-observed 2,053). Shipping a + // component our own numbers say loses money, guarded only by a doc note, is not a + // defensible default. Set true if your outputs are genuinely huge; the gate's + // economics then decide each call as normal. + AllowOnCachingBackend *bool `yaml:"allow_on_caching_backend"` // EconomicGate opts out of the expected-value gate (#28 D). Unset = ON (the default): // only call the LLM when the expected saving exceeds the expected call cost, priced // from real model rates and the cache-awareness of the traffic. Set false to restore @@ -144,13 +155,21 @@ func newExtractLLM(raw []byte) (components.Component, error) { if cfg.EconomicGate != nil { gate = *cfg.EconomicGate } + // Off by default on caching backends (see AllowOnCachingBackend). Disabling the gate + // entirely is an explicit request for pre-#28 behavior, so honor it here too — otherwise + // `economic_gate: false` would still be silently blocked on caching traffic. + allowCached := !gate + if cfg.AllowOnCachingBackend != nil { + allowCached = *cfg.AllowOnCachingBackend + } return &ExtractLLM{ minTokens: cfg.MinTokens, strategy: cfg.Strategy, modelSource: cfg.Model.Source, modelClient: cfg.Model.Client(), trigger: cfg.Trigger, mode: parseMarkerMode(cfg.MarkerMode), rewrite: rewrite, llmEveryN: cfg.LLMEveryN, llmMaxPerReq: cfg.LLMMaxPerReq, skipFileReads: cfg.SkipFileReads, llmSeen: map[string]int{}, - minTokensSet: explicit, gate: gate, pricing: cheapmodel.PricingFromEnv(), + minTokensSet: explicit, gate: gate, allowCached: allowCached, + pricing: cheapmodel.PricingFromEnv(), prevTokens: map[string]int{}, modelName: cfg.Model.Model, }, nil } @@ -316,6 +335,11 @@ func (e *ExtractLLM) Offload(req *bschemas.BifrostChatRequest, rep *components.R // SAME-SESSION replay first. This session already sent these compacted bytes on an // earlier turn, so the provider's cached prefix holds the COMPACTED form and replaying // it is byte-identical — which is why it is safe at any depth, cached prefix included. + // + // One key per decision, not two. #40 unified the projected text and its summary into + // a single JSON value precisely because the split `cg:res:` + `cg:sum1:` keys had + // independent TTLs and pin slots, so a replay could hit one and miss the other and + // emit HALF a decision — projected text with the summary segment silently gone. if cached, hit := getResult(c, id); hit { metrics.RecordExtractionCacheLookup(true) apply(i, content, cached.Projected, cached.Summary) @@ -345,27 +369,46 @@ func (e *ExtractLLM) Offload(req *bschemas.BifrostChatRequest, rep *components.R } continue } - // CROSS-SESSION reuse (#28 C), deliberately placed AFTER the tail gate. An extraction - // is a context-free derived result, so a global content-hash key lets a different - // session skip re-deriving it (measured: 82 of 103 unique contents recurred across - // sessions). But cache-safety differs from the same-session replay above: THIS session - // never sent these compacted bytes, so the provider's cached prefix holds the ORIGINAL. - // Splicing a global hit at depth would mutate already-cached content and force a - // suffix re-write at 11.5x the read price — the same churn the tail gate exists to - // prevent, just sourced from a cache instead of a model call. So a global hit is - // treated exactly like a new decision: tail-only, then frozen and replayed from there. + // CROSS-SESSION reuse (#28 C), deliberately placed AFTER the tail gate — the + // invariant TestGlobalCacheHitIsNotSplicedAtDepth exists to protect. An extraction is + // a context-free derived result, so a global content-hash key lets a different session + // skip re-deriving it (measured: 82 of 103 unique contents recurred across sessions). + // But cache-safety differs from the same-session replay above: THIS session never sent + // these compacted bytes, so the provider's cached prefix holds the ORIGINAL. Splicing + // a global hit at depth would mutate already-cached content and force a suffix re-write + // at 11.5x the read price — the same churn the tail gate exists to prevent, just + // sourced from a cache instead of a model call, and exactly the harm #40 removed + // repairLostResult to avoid. So a global hit is treated exactly like a new decision: + // tail-only, then frozen and replayed from the same-session path at any depth. Do NOT + // flatten this ordering: only getResult may bypass the tail gate. // // It cannot become a re-derive path (#40's stance): on a MISS this falls through to the // normal candidate flow, which is already tail-gated and floor-gated; nothing here // lifts a depth restriction the way the removed repairLostResult did. - if cached, hit := getResultGlobal(c, extract.ResultKey(id, e.modelName, extCfg)); hit { - metrics.RecordExtractionCacheLookup(true) - // Freeze into this session so later turns replay it byte-for-byte from the - // same-session path above, at any depth. - putResult(c, id, cached.Projected, cached.Summary) - apply(i, content, cached.Projected, cached.Summary) - dbgReapply++ - continue + // + // Reuse is gated on RECOVERABILITY, not verification. The result was derived toward + // the goal of whichever session produced it, and in the default rewrite mode the + // containment proof is deliberately skipped — so a reused result can be a lossy rewrite + // steered by an unrelated task. That is acceptable only while the agent can get the + // original back: with a full (reversible) marker the stash is refreshed and `expand` + // recovers it. Without one (marker_mode summary/off, or a non-persisting store) the + // drop is permanent, so fall back to same-session reuse only. + // + // One key per decision here too: the global namespace stores the projected text and + // its summary in the SAME value, for the same reason #40 unified the session-scoped + // pair. Splitting them across two global keys would re-create the half-a-decision bug + // cross-session, where independent TTLs let a hit on one and a miss on the other emit + // projected text with the summary segment silently gone. + if !e.rewrite || effectiveMode(c, e.mode) == markerFull { + if cached, hit := getResultGlobal(c, extract.ResultKey(id, e.modelName, extCfg)); hit { + metrics.RecordExtractionCacheLookup(true) + // Freeze into this session so later turns replay it byte-for-byte from the + // same-session path above, at any depth. + putResult(c, id, cached.Projected, cached.Summary) + apply(i, content, cached.Projected, cached.Summary) + dbgReapply++ + continue + } } metrics.RecordExtractionCacheLookup(false) if sz < floor { @@ -385,12 +428,21 @@ func (e *ExtractLLM) Offload(req *bschemas.BifrostChatRequest, rep *components.R // LLM call worth it for THIS output, given that a saved token in a cached region // is worth 10x less? Where caching makes extraction pointless, suppress it; on a // non-caching backend or for recurring content, allow it. + // Record the sighting BEFORE the gate reads it, and read the PRIOR value. The flag + // means "seen on an earlier turn/session", so marking it after the gate allowed a + // call made first sight reclassify itself as recurring and collect a 50% valuation + // bump (6 expected reuses vs 4) it had not earned — the gate over-firing in the + // opposite direction from the two pessimistic priors fixed earlier. Marking on + // OBSERVATION also means a suppressed candidate still counts as seen, which is + // correct: recurrence is a property of the content, not of what we decided to spend. + seenBefore := markSeenContent(c, id) if e.gate { - // Content the store has seen before in ANY session is likely to recur again, - // which is what amortizes the call across turns. - seenBefore := hasSeenContent(c, id) + // Stop exploring once calls are observed to be slow: exploration spends wall + // clock as well as money, and an agent on a task deadline feels the former more. + explore := !tooSlowToExplore(metrics.ExtractionAvgLatencyMs()) && + e.ratios.exploring(c.Session) d := evaluateGate(sz, ratio, val, callCost(e.pricing, sz), seenBefore, turnsSoFar, - e.ratios.exploring()) + explore, e.allowCached) if !d.allow { metrics.RecordExtractionSuppressed(d.reason) if debugExtractLLM { @@ -402,7 +454,6 @@ func (e *ExtractLLM) Offload(req *bschemas.BifrostChatRequest, rep *components.R } metrics.RecordExtractionReason(d.reason) } - markSeenContent(c, id) cands = append(cands, cand{i, content, id}) } if debugExtractLLM && len(tools) > 0 { @@ -455,11 +506,16 @@ func (e *ExtractLLM) Offload(req *bschemas.BifrostChatRequest, rep *components.R if out[k].projected == "" { continue } - putResult(c, cands[k].id, out[k].projected, out[k].summary) // Publish to the GLOBAL namespace only when the result is recoverable (or verified - // deletion-only). An unverified lossy rewrite with no way back must not become - // another session's starting point; the session-scoped write above still gives this - // session cross-turn replay. + // deletion-only) — the same condition the read side checks. An unverified, lossy + // rewrite with no way back must not become another session's starting point; keep it + // session-scoped so this session still benefits across its own turns. + // Always write the session-scoped entry: this session's own cross-turn replay is + // what keeps its cached prefix byte-stable, and it must not depend on whether the + // result also qualified for cross-session sharing. Then publish globally only when + // recoverable. One key per decision (#40) — projected text and summary travel + // together, so a replay can never emit half a decision. + putResult(c, cands[k].id, out[k].projected, out[k].summary) if !e.rewrite || effectiveMode(c, e.mode) == markerFull { putResultGlobal(c, extract.ResultKey(cands[k].id, e.modelName, extCfg), out[k].projected, out[k].summary) diff --git a/docs/components/extract_llm.md b/docs/components/extract_llm.md index 10f27f2..7759719 100644 --- a/docs/components/extract_llm.md +++ b/docs/components/extract_llm.md @@ -15,12 +15,19 @@ On a **prompt-caching backend** (the default for Anthropic/Bedrock traffic), `ex |---|---| | Extraction calls | 271 | | Extraction cost | $3.26 | -| Cumulative added latency | ~1,592 s (~450 ms/call) | -| Unique tokens saved | ~197,548 | -| Value of those tokens at the **cache-read** rate | ~$0.06 | -| **Net** | **deeply negative (~8× underwater against cache-aware value)** | +| Cumulative added latency | 1,592,467 ms (~1,592 s, ~450 ms/call) | +| Unique tokens saved | 197,548 | +| Value of those tokens at the **cache-read** rate | **$0.0395** (197,548 × $0.20/MTok) | +| **Net** | **$0.0395 − $3.26 = −$3.22 — 82× underwater** | | Share of realized value that came from the **replay cache**, not the LLM | **~93%** | +The improvement plan's original figure was **~8×** underwater. That implicitly priced the saved +tokens as fresh input; they were not. All 197,548 sat in the **cached prefix**, so they bill at the +cache-read rate, and the honest ratio is **82×** — an order of magnitude worse, plus the 1,592,467 +ms of blocking time, which is not priced at all. A later Terminal-Bench arm that excluded +`extract_llm` entirely re-derived this independently. The verdict below was right; it was +understated. + The reason is arithmetic, not implementation quality. A request to a caching backend is ~99.95% cached, so a token removed from a cached region saves the **cache-read** rate (`$0.30/MTok`), not the fresh-input rate (`$3/MTok`) — a **10× haircut**. An extraction call From dfd3d418de81b983d08f98df9d6ebb3c9b4a5e02 Mon Sep 17 00:00:00 2001 From: Osher-Elhadad Date: Mon, 10 Aug 2026 15:39:32 +0000 Subject: [PATCH 5/5] docs(extract_llm): state the 82x cache-read verdict and name the rate used The improvement plan's ~8x figure implicitly priced the 197,548 saved tokens as fresh input. They sat in the cached prefix, so they bill at the cache-read rate: $0.0395 against $3.26 and 1,592,467 ms of blocking time -- 82x underwater. A later Terminal-Bench arm that excluded the component entirely re-derived this independently. Name which cache-read rate each figure uses, since the gate reasons at $0.30/MTok (55x) while the issue quoted $0.20/MTok (82x); the gate's is the more generous of the two, so the shipped decline is the conservative one. Assisted-By: Claude Opus 5 Signed-off-by: Osher-Elhadad --- docs/components/extract_llm.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/components/extract_llm.md b/docs/components/extract_llm.md index 7759719..6e8f5df 100644 --- a/docs/components/extract_llm.md +++ b/docs/components/extract_llm.md @@ -28,6 +28,13 @@ ms of blocking time, which is not priced at all. A later Terminal-Bench arm that `extract_llm` entirely re-derived this independently. The verdict below was right; it was understated. +!!! note "Which cache-read rate" + The 82× figure prices the saved tokens at **$0.20/MTok**, the rate quoted in the original + issue. The break-even tables below use the **$0.30/MTok** sonnet-class rate the gate itself + applies (`agentCacheReadPerMTok`), which gives $0.0593 and **55×**. Same conclusion either + way — neither is within two orders of magnitude of paying for $3.26 — and the gate reasons + with the *more generous* rate, so the shipped decline is the conservative one. + The reason is arithmetic, not implementation quality. A request to a caching backend is ~99.95% cached, so a token removed from a cached region saves the **cache-read** rate (`$0.30/MTok`), not the fresh-input rate (`$3/MTok`) — a **10× haircut**. An extraction call