Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions components/component.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,14 @@ type PrefixUsage struct {
CacheWrite int
Fresh int
Output int
// ViaTool records that the answer arrived as a tool_use for the proxy's own structured-answer
// tool rather than as reply TEXT. Reported for the same reason CacheRead is: a caller that
// declares a verdict tool in order to get a schema-shaped answer cannot otherwise tell whether it
// got one. A sweep whose tool is never used and a sweep that works look IDENTICAL in every other
// counter -- both return parsed verdicts and real savings -- because the prose parser accepts
// both. Measured: a review of PR #137 found 0 of 5 live asks using the declared tool while every
// verdict arrived through the text path, and no counter in this repo could show that.
ViaTool bool
}

// PrefixAsker completes `ask` as a trailing user message appended to the EXACT body this session
Expand Down
17 changes: 17 additions & 0 deletions components/offload/extract_sweep.go
Original file line number Diff line number Diff line change
Expand Up @@ -694,6 +694,23 @@ func (e *ExtractSweep) adjudicate(req *bschemas.BifrostChatRequest, c *component
} else if !fellBack {
r.event("sweep_prefix_cache_read_ok")
}
// HOW THE ANSWER ARRIVED: the proxy's structured-answer tool, or reply prose. Split because
// nothing else in this component can tell the two apart. ParseVerdicts reads a tool_use `input`
// and a JSON array in text identically -- by design, so that declaring the tool is additive --
// which means a run where the declared tool is never touched produces the same verdicts, the same
// savings and the same gate counts as one where it is used every time. A review of PR #137
// measured exactly that divergence live (0 of 5 asks used the tool, all 5 answered in prose)
// against that PR's claim of 6 of 6, and no published counter could adjudicate between them.
//
// Only for the PREFIX ask. The fallback calls Complete(), which has no tool to declare, so
// counting it as prose would report a shape that was never on offer.
if !fellBack {
if usage.ViaTool {
r.event("sweep_answered_via_tool")
} else {
r.event("sweep_answered_via_prose")
}
}
for range items {
r.event("sweep_adjudicated")
}
Expand Down
81 changes: 77 additions & 4 deletions components/offload/extract_sweep_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,13 @@ type fakeAsker struct {
reply string
cacheRead int
err error
calls int64
lastAsk atomic.Value
lastSess atomic.Value
// viaTool reports the reply as having arrived through the proxy's structured-answer tool rather
// than as text, which is the one thing about a reply the parser CANNOT infer: it reads a
// tool_use input and a JSON array in prose identically.
viaTool bool
calls int64
lastAsk atomic.Value
lastSess atomic.Value
}

func (f *fakeAsker) Ask(_ context.Context, session, ask string) (string, components.PrefixUsage, error) {
Expand All @@ -36,7 +40,8 @@ func (f *fakeAsker) Ask(_ context.Context, session, ask string) (string, compone
if f.err != nil {
return "", components.PrefixUsage{}, f.err
}
return f.reply, components.PrefixUsage{CacheRead: f.cacheRead, Fresh: 40, Output: 90}, nil
return f.reply, components.PrefixUsage{CacheRead: f.cacheRead, Fresh: 40, Output: 90,
ViaTool: f.viaTool}, nil
}

func (f *fakeAsker) ask() string { s, _ := f.lastAsk.Load().(string); return s }
Expand Down Expand Up @@ -832,3 +837,71 @@ func TestSweepSendsTheInventoryAndNotTheOutputs(t *testing.T) {
}
}
}

// The verdict tool's whole purpose is that the model ANSWERS with it, and until this counter existed
// nothing could say whether it did. extract.ParseVerdicts reads a tool_use `input` and a JSON array in
// reply text identically -- deliberately, so that declaring the tool is additive -- with the
// consequence that a run where the declared tool is never touched produces the same verdicts, the same
// savings and the same gates as one where it is used every time. That is not hypothetical: a review of
// PR #137 measured 0 of 5 live asks using the tool while the PR claimed 6 of 6, and no published
// figure in this repo could adjudicate between the two readings.
func TestSweepCountsWhetherTheAnswerCameViaTheToolOrProse(t *testing.T) {
verdicts := `[{"i":0,"needed_by":"none","quote":"","verdict":"keep"}]`
for _, tc := range []struct {
name string
viaTool bool
want string
notWant string
}{
{"tool_use", true, "sweep_answered_via_tool", "sweep_answered_via_prose"},
{"prose", false, "sweep_answered_via_prose", "sweep_answered_via_tool"},
} {
t.Run(tc.name, func(t *testing.T) {
asker := &fakeAsker{reply: verdicts, cacheRead: 19595, viaTool: tc.viaTool}
e := newSweepSmall(t, "")
rep := &components.Report{}
if _, err := e.Offload(sweepReq(), rep,
preExpiryCtx("s-"+tc.name, asker, store.NewMemory(store.Options{}))); err != nil {
t.Fatal(err)
}
// PRECONDITION. Without it a turn that never reached the ask at all would pass both
// assertions below by producing neither event, which is exactly the vacuous shape this
// repo's convention exists to catch.
if rep.Events["sweep_prefix_cache_read_ok"] != 1 {
t.Fatalf("the prefix ask never happened, so no reply shape was observed "+
"(gates: %v events: %v)", rep.Gates, rep.Events)
}
if rep.Events[tc.want] != 1 {
t.Errorf("%s: %s = %d, want 1 (events: %v)", tc.name, tc.want,
rep.Events[tc.want], rep.Events)
}
if rep.Events[tc.notWant] != 0 {
t.Errorf("%s: %s = %d, want 0 -- the two shapes must not both be counted "+
"(events: %v)", tc.name, tc.notWant, rep.Events[tc.notWant], rep.Events)
}
})
}
}

// The FALLBACK is neither shape. fallbackAsk calls Model.Complete(), which carries no tool to declare,
// so counting its reply as prose would report a shape that was never on offer -- and would silently
// inflate the prose count with calls that could not have used the tool even in principle.
func TestSweepDoesNotAttributeAReplyShapeToTheFallback(t *testing.T) {
// cacheRead 0 sends this down the fallback fork, which is the only way in without a real
// non-Anthropic route.
asker := &fakeAsker{reply: `[{"i":0,"needed_by":"none","quote":"","verdict":"keep"}]`, cacheRead: 0}
e := newSweepSmall(t, "")
rep := &components.Report{}
if _, err := e.Offload(sweepReq(), rep,
preExpiryCtx("s-fb", asker, store.NewMemory(store.Options{}))); err != nil {
t.Fatal(err)
}
if rep.Gates["sweep_prefix_cache_read_ZERO"] != 1 {
t.Fatalf("the fallback fork was never taken, so this asserts nothing "+
"(gates: %v events: %v)", rep.Gates, rep.Events)
}
if rep.Events["sweep_answered_via_tool"] != 0 || rep.Events["sweep_answered_via_prose"] != 0 {
t.Errorf("the fallback was attributed a reply shape it could not have had (events: %v)",
rep.Events)
}
}
71 changes: 67 additions & 4 deletions docs/components/extract_llm_sweep.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,73 @@ transcript as of the previous turn, so the newest tool output is invisible to it
content has had no turns in which to be superseded — and it keeps a large model call off the agent's
critical path.

Three construction facts, each measured and each a test: `tool_choice: none` is **not** in the cache key
so forcing it is free, and **necessary**, or the prefix's tools make the model answer with a `tool_use`;
`tools` **are** in the key, so stripping them reads a different, smaller entry; and the route rejects
assistant prefill, which an appended user message satisfies by construction.
Two construction facts, each measured and each a test: `tools` **are** in the cache key, so stripping
them reads a different, smaller entry; and the route rejects assistant prefill, which an appended user
message satisfies by construction.

### `tool_choice`, and why the answer is a declared tool rather than a suppressed one

This page previously stated the inverse of what is now measured — that forcing `tool_choice: none` was
"free, and **necessary**, or the prefix's tools make the model answer with a `tool_use`". The second
half of that sentence was **right about the mechanism and wrong about the remedy**, and the fix is not
to suppress the tool call but to declare a tool worth calling.

The ask carries the agent's own tools, because `tools` are in the cache key and stripping them costs
the read. Three arms, same transcript, same ask model (`aws/claude-sonnet-5` over LOCA S2L, three tasks,
three passes each, run sequentially):

| arm | `tool_choice` | verdict tool declared | asks replied | unusable | answered via `tool_use` | verdict coverage |
|---|---|---|---|---|---|---|
| A | `{"type":"none"}` | no | 20 | **6 (30.0%)** | 0 | 71.5% |
| B | omitted | **no** | 24 | **14 (58.3%)** | 0 | 41.2% |
| C | omitted | **yes** | 77 | **7 (9.1%)** | **43 (55.8%)** | 90.9% |

Fisher two-tailed: A vs C `p = 0.0245`, B vs C `p = 0.0000`, A vs B `p = 0.0755`.

Arm **B is the one that had never been run**, and it is the reason the tool is declared rather than the
`tool_choice` simply being dropped. Removing the suppression without offering an answer tool is
**worse than either other arm**: the model, now free to call something and offered only the agent's
tools and `context_guru_expand`, calls one of those. Directly observed on the wire by logging every
reply's content blocks — `thinking,tool_use:context_guru_expand` with **no text block at all**, which
the text-only extraction reads as the empty string and files as unusable. Arm B also lost 5 asks to the
90 s `llmCallTimeout` against 0 in arm A and 1 in arm C.

So the three shapes are: suppress the call and the model argues its verdicts in prose (arm A's failures);
allow the call with nothing worth calling and the answer is lost into somebody else's tool (arm B);
allow it and declare the right tool, and 55.8% of replies come back schema-shaped, at cache-read price,
with the rest still read by the unchanged prose parser (arm C).

`tool_choice` is not free in every form either. Naming a tool wrote a **second** cache entry (8,378
against the 8,268 already cached), so `tool_choice` does participate in the key when it names a tool,
even though `none` does not. Omitting it entirely is what reads the prefix for free.

The residual 9.1% in arm C is dominated by a reply with a `thinking` block and no answer at all. That
is a separate defect, present in every arm, and it is tracked apart from this component's contract.

### The verdict tool is advertised only where it can be used

`context_guru_adjudicate` is appended to the `tools` array of every request on the **Anthropic** route
whose pipeline **contains `extract_llm_sweep`** — and nowhere else. It costs a measured 946 bytes at the
head of the cacheable prefix, so both conditions matter:

- **Pipeline membership.** A preset with no sweep can never adjudicate. `off` is the control arm of every
published comparison in this repo, and injecting there perturbed the baseline of all of them.
- **Provider.** `prefixAskerFor` returns nil for anything but Anthropic and `cheapmodel/openai.go` has no
`CompletePrefixed` at all, so on the OpenAI route the definition is unreachable by construction.

Neither condition varies per turn — pipeline membership is fixed per **config document**, the provider
by the route — so the prefix stays byte-stable for every request under a given config and never flaps.
That is the distinction the cache argument actually requires: it forbids gating on something that
changes turn to turn, not on something fixed for the config. (Not "fixed at config load": `tenancy.go`
rebuilds a tenant's `*Pipeline` when the config document changes, mid-session. A config change that adds
or drops this component has already invalidated the prefix for much bigger reasons than 946 bytes.)

Because the model is told not to call it and sometimes still would, a call the **agent** makes is
answered on the response path before the client is written to — **except** when the model calls it
alongside a *client* tool in the same assistant turn. The response loop cannot continue a turn whose
other `tool_use` only the client can execute, so it hands that round over whole and
`adjudicate.AnswerStrayCalls` repairs it on the next request: the agent pays one turn, the session is
fine. `/stats` publishes `adjudicate_stray` either way — measured 0 across all three passes of arm C.

## The trigger: pre-expiry, not cold

Expand Down
1 change: 1 addition & 0 deletions docs/reference/routes.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ consumer that reads by key keeps working. The tables below group the `Snapshot`
| `adjusted_saved` | `saved − wasted`. May be negative. |
| `components` | Per-component rollup (see below). |
| `top_passthrough` | Components that ran but never *changed* a request — dead weight. A component that mutated without saving content tokens (`cachesplit`, `cacheinject`) is **not** listed here. |
| `adjudicate_stray` | Times the **agent** called `context_guru_adjudicate`, the verdict tool the proxy injects for [`extract_llm_sweep`](../components/extract_llm_sweep.md) and tells the model not to call. Counted whether the call was answered on the response path or repaired on the next request. Most cost the agent nothing, because the response path answers them in band before the client is written to; one co-called with a *client* tool in the same assistant turn costs one turn, because the loop hands that round over whole and the next request's repair fixes it. Either way this is a *description-health* number rather than an error: non-zero means the "do not call this yourself" wording has stopped working. Measured 0 over three benchmark passes. Also exported as `cg_adjudicate_stray_total`. |
| `top_discarded` | Components whose changes the **writeback layer threw away** at least once. Any entry needs investigating: the component ran, mutated, and had no effect on the wire. |

Per-component (`components.<name>` and `potential_components.<name>`):
Expand Down
27 changes: 22 additions & 5 deletions expand/response.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,26 +16,43 @@ type Call struct {
// true if the model also called some OTHER tool — in that case the host cannot
// safely auto-continue (the client must resolve the other tools), so the loop
// bails and returns the response as-is.
func ResponseCalls(provider string, resp []byte) (calls []Call, otherTools bool) {
// otherProxyTools are additional names the caller owns. otherTools means "a tool the CLIENT
// implements", which is what makes the response loop hand the turn over untouched — so a second
// proxy-injected tool counted as "other" caused the loop to bail and stream that proxy tool_use to
// the client. Variadic so the existing callers, which advertise only expand, are unchanged.
func ResponseCalls(provider string, resp []byte, otherProxyTools ...string) (calls []Call, otherTools bool) {
ours := func(name string) bool {
if name == ToolName {
return true
}
for _, t := range otherProxyTools {
if t != "" && name == t {
return true
}
}
return false
}
switch provider {
case "anthropic":
gjson.GetBytes(resp, "content").ForEach(func(_, blk gjson.Result) bool {
if blk.Get("type").String() != "tool_use" {
return true
}
if blk.Get("name").String() == ToolName {
switch name := blk.Get("name").String(); {
case name == ToolName:
calls = append(calls, Call{CallID: blk.Get("id").String(), HashID: blk.Get("input.id").String()})
} else {
case !ours(name):
otherTools = true
}
return true
})
default: // openai and compatibles
gjson.GetBytes(resp, "choices.0.message.tool_calls").ForEach(func(_, tc gjson.Result) bool {
if tc.Get("function.name").String() == ToolName {
switch name := tc.Get("function.name").String(); {
case name == ToolName:
hash := gjson.Get(tc.Get("function.arguments").String(), "id").String()
calls = append(calls, Call{CallID: tc.Get("id").String(), HashID: hash})
} else {
case !ours(name):
otherTools = true
}
return true
Expand Down
Loading
Loading