From c6e445068684f0fe269bd89927732463ac875021 Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Sun, 23 Aug 2026 00:50:26 +0100 Subject: [PATCH 1/3] test(anthropic): pin tool translation and upstream error shapes Cover four zero-coverage functions on the /v1/messages route Claude Code drives on nearly every call: openAIToolsFromAnthropic, openAIToolChoiceFromAnthropic, responseTranslationError.Error, and writeAnthropicProviderUpstreamError. Add table-driven tests for every tool_choice form (auto, any, tool-by-name, unrecognized, absent, non-map, nil), a multi-tool mapping test that checks field order, description omission, and malformed-entry skipping, a full request-to-response round trip through a forced tool call, and a test proving no-tools/no-tool-choice omits both keys rather than sending an empty array. Add direct tests for the Anthropic-shaped upstream error envelope (status passthrough, error.code, decoded error.upstream) and for the translation-failure error prefix. Every test was confirmed to fail against a real mutation of the target function before being kept; each mutation and its revert are recorded in the test's own doc comment. --- routes_messages_test.go | 66 ++++++++++++++ translate_anthropic_test.go | 172 ++++++++++++++++++++++++++++++++++++ 2 files changed, 238 insertions(+) diff --git a/routes_messages_test.go b/routes_messages_test.go index 7246954..5571d36 100644 --- a/routes_messages_test.go +++ b/routes_messages_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "net/http" "net/http/httptest" "strings" @@ -679,3 +680,68 @@ func TestHandleMessages_AnthropicProvider_Passthrough_HeaderAllowlist(t *testing t.Errorf("upstream x-api-key = %q, want the gateway's own configured key sk-ant-up, never the client's sk-alice", apiKeyValues[0]) } } + +// TestResponseTranslationError_ErrorPrefixesTranslateResponse proves +// (*responseTranslationError).Error prefixes the wrapped error's own +// message with "translate response: " — the text a log line built from +// this error carries, distinguishing a translation failure (the upstream +// already answered and was billed) from a connectivity failure in the +// same log stream. +// +// MUTATION PROVEN: changing the method body to `return e.err.Error()` +// (dropping the prefix) made this test fail — confirmed via `go test -run +// TestResponseTranslationError_ErrorPrefixesTranslateResponse`, then +// reverted with `git checkout -- routes_messages.go`. +func TestResponseTranslationError_ErrorPrefixesTranslateResponse(t *testing.T) { + err := &responseTranslationError{err: errors.New("boom")} + if got, want := err.Error(), "translate response: boom"; got != want { + t.Errorf("Error() = %q, want %q", got, want) + } +} + +// TestWriteAnthropicProviderUpstreamError_EmbedsStatusTypeAndDecodedBody +// proves writeAnthropicProviderUpstreamError writes perr's own status +// through as the HTTP status (not a fixed one), wraps it in the Anthropic +// {"type":"error","error":{...}} envelope with error.type "upstream_error" +// and error.code as perr's status stringified, and embeds perr.body — +// decoded as JSON when it parses as one — under error.upstream +// (decodeUpstreamErrorBody, routes_unified.go, shared with this +// function's OpenAI-shaped counterpart writeProviderUpstreamError). +// +// MUTATION PROVEN: replacing `w.WriteHeader(perr.status)` with a +// hardcoded `w.WriteHeader(http.StatusInternalServerError)` made the +// status assertion fail — confirmed, then reverted. Separately proven: +// removing `"upstream": decodeUpstreamErrorBody(perr)` from the envelope +// made the upstream-body assertion fail — confirmed, then reverted. +func TestWriteAnthropicProviderUpstreamError_EmbedsStatusTypeAndDecodedBody(t *testing.T) { + perr := &providerHTTPError{status: http.StatusServiceUnavailable, body: []byte(`{"error":{"message":"overloaded"}}`)} + rec := httptest.NewRecorder() + + writeAnthropicProviderUpstreamError(rec, "anthropic", perr) + + if rec.Code != http.StatusServiceUnavailable { + t.Fatalf("status = %d, want 503 (perr's own status)", rec.Code) + } + var got map[string]any + if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil { + t.Fatalf("response is not valid JSON: %v, body=%s", err, rec.Body.String()) + } + if got["type"] != "error" { + t.Errorf(`top-level "type" = %v, want "error"`, got["type"]) + } + errObj, _ := got["error"].(map[string]any) + if errObj["type"] != "upstream_error" { + t.Errorf(`error.type = %v, want "upstream_error"`, errObj["type"]) + } + if errObj["message"] != "anthropic upstream error" { + t.Errorf(`error.message = %v, want "anthropic upstream error"`, errObj["message"]) + } + if errObj["code"] != "503" { + t.Errorf(`error.code = %v, want "503"`, errObj["code"]) + } + upstream, _ := errObj["upstream"].(map[string]any) + inner, _ := upstream["error"].(map[string]any) + if inner["message"] != "overloaded" { + t.Errorf(`error.upstream decoded body = %v, want the parsed provider body {"error":{"message":"overloaded"}}`, errObj["upstream"]) + } +} diff --git a/translate_anthropic_test.go b/translate_anthropic_test.go index a378311..2604568 100644 --- a/translate_anthropic_test.go +++ b/translate_anthropic_test.go @@ -856,3 +856,175 @@ func TestAnthropicUsagePayload_TotalInputTokens_FoldsCacheCounters(t *testing.T) t.Errorf("totalInputTokens() = %d, want %d", got, want) } } + +// --- openAIToolsFromAnthropic / openAIToolChoiceFromAnthropic: the +// Anthropic tools[]/tool_choice request translators, on the newest, +// heaviest-traffic path (/v1/messages -> a non-anthropic provider). Claude +// Code sends a tools[] array on nearly every call. Both functions were +// previously ZERO-COVERED: every existing openAIRequestFromAnthropic test +// above never set "tools" or "tool_choice" on its request map. --- + +// TestOpenAIToolsFromAnthropic_MapsFieldsAndSkipsMalformedEntries proves +// each Anthropic tool{name, description, input_schema} maps IN ORDER to +// an OpenAI {"type":"function","function":{"name","description", +// "parameters"}} entry, that "description" is OMITTED (not set to a zero +// value) when the source tool carries none, and that a non-map entry in +// toolsRaw is skipped rather than panicking or producing a garbage entry. +// +// MUTATION PROVEN: changing `"parameters": tm["input_schema"]` to +// `"parameters": tm["name"]` made this test fail (parameters carried the +// tool's name string instead of its JSON schema) — confirmed via `go test +// -run TestOpenAIToolsFromAnthropic`, then reverted with `git checkout -- +// translate_anthropic.go`. +func TestOpenAIToolsFromAnthropic_MapsFieldsAndSkipsMalformedEntries(t *testing.T) { + schema := map[string]any{"type": "object", "properties": map[string]any{"city": map[string]any{"type": "string"}}} + toolsRaw := []any{ + map[string]any{"name": "get_weather", "description": "Get the weather", "input_schema": schema}, + map[string]any{"name": "no_desc_tool", "input_schema": map[string]any{"type": "object"}}, + "not a map", // malformed entry: must be skipped, not panic + map[string]any{"name": "empty_schema_tool", "input_schema": map[string]any{}}, + } + + out := openAIToolsFromAnthropic(toolsRaw) + if len(out) != 3 { + t.Fatalf("len(out) = %d, want 3 (the malformed entry must be skipped)", len(out)) + } + + first, _ := out[0].(map[string]any) + if first["type"] != "function" { + t.Errorf(`out[0]["type"] = %v, want "function"`, first["type"]) + } + fn, _ := first["function"].(map[string]any) + assert.Equal(t, "get_weather", fn["name"]) + assert.Equal(t, "Get the weather", fn["description"]) + assert.Equal(t, schema, fn["parameters"]) + + second, _ := out[1].(map[string]any) + fn2, _ := second["function"].(map[string]any) + if _, hasDesc := fn2["description"]; hasDesc { + t.Errorf(`function["description"] present (%v) for a tool with no description, want the key omitted`, fn2["description"]) + } + + third, _ := out[2].(map[string]any) + fn3, _ := third["function"].(map[string]any) + assert.Equal(t, "empty_schema_tool", fn3["name"]) + assert.Equal(t, map[string]any{}, fn3["parameters"]) +} + +// TestOpenAIToolChoiceFromAnthropic_AllForms proves each of Anthropic's +// tool_choice shapes maps to OpenAI's, per the function's own doc +// comment: "auto"->"auto", "any"->"required" (OpenAI's force-some-tool +// value), {"type":"tool","name":n}->{"type":"function","function": +// {"name":n}}, and an unrecognized type, an absent type, a non-map value, +// or nil all map to a Go nil — the signal openAIRequestFromAnthropic's own +// caller-side `if choice := ...; choice != nil` uses to decide whether to +// set "tool_choice" on the OpenAI request at all. +// +// MUTATION PROVEN: changing the "any" case to `return "auto"` (collapsing +// it into the "auto" case, so a forced tool call silently became merely +// advisory) made this test fail — confirmed, then reverted. +func TestOpenAIToolChoiceFromAnthropic_AllForms(t *testing.T) { + cases := []struct { + tc any + want any + name string + }{ + {name: "auto", tc: map[string]any{"type": "auto"}, want: "auto"}, + {name: "any becomes OpenAI's required", tc: map[string]any{"type": "any"}, want: "required"}, + {name: "tool-by-name", tc: map[string]any{"type": "tool", "name": "get_weather"}, want: map[string]any{"type": "function", "function": map[string]any{"name": "get_weather"}}}, + {name: "unrecognized type is nil", tc: map[string]any{"type": "bogus"}, want: nil}, + {name: "absent type is nil", tc: map[string]any{}, want: nil}, + {name: "non-map value is nil", tc: "auto", want: nil}, + {name: "nil value is nil", tc: nil, want: nil}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got := openAIToolChoiceFromAnthropic(c.tc) + assert.Equal(t, c.want, got) + }) + } +} + +// TestOpenAIRequestFromAnthropic_ToolsAndToolChoiceRoundTrip is the +// end-to-end regression for openAIToolsFromAnthropic and +// openAIToolChoiceFromAnthropic wired through their only real caller, +// openAIRequestFromAnthropic: an Anthropic /v1/messages request carrying a +// multi-tool tools[] array and a tool_choice forcing one specific tool +// translates to a request shape a real OpenAI chat-completions endpoint +// accepts, and the tool_calls[] such an endpoint would answer with +// translates back to Anthropic's own tool_use content-block shape — the +// full round trip Claude Code actually drives on this route. +func TestOpenAIRequestFromAnthropic_ToolsAndToolChoiceRoundTrip(t *testing.T) { + req := map[string]any{ + "model": "claude-x", + "tools": []any{ + map[string]any{"name": "get_weather", "description": "Get the weather for a city", "input_schema": map[string]any{ + "type": "object", "properties": map[string]any{"city": map[string]any{"type": "string"}}, "required": []any{"city"}, + }}, + map[string]any{"name": "get_time", "input_schema": map[string]any{"type": "object"}}, + }, + "tool_choice": map[string]any{"type": "tool", "name": "get_weather"}, + "messages": []any{map[string]any{"role": "user", "content": "What's the weather in London?"}}, + } + + out, _, err := openAIRequestFromAnthropic(req) + if err != nil { + t.Fatalf("openAIRequestFromAnthropic: %v", err) + } + + tools, _ := out["tools"].([]any) + if len(tools) != 2 { + t.Fatalf(`out["tools"] = %v, want 2 entries`, out["tools"]) + } + firstFn, _ := tools[0].(map[string]any)["function"].(map[string]any) + assert.Equal(t, "get_weather", firstFn["name"]) + secondFn, _ := tools[1].(map[string]any)["function"].(map[string]any) + assert.Equal(t, "get_time", secondFn["name"]) + + wantChoice := map[string]any{"type": "function", "function": map[string]any{"name": "get_weather"}} + assert.Equal(t, wantChoice, out["tool_choice"]) + + // A real OpenAI-shaped response answering that forced tool call... + const body = `{"id":"chatcmpl-1","choices":[{"index":0,"message":{"content":null,"tool_calls":[{"id":"call_1","function":{"name":"get_weather","arguments":"{\"city\":\"London\"}"}}]},"finish_reason":"tool_calls"}],"usage":{"prompt_tokens":10,"completion_tokens":5}}` + anthropicOut, err := anthropicResponseFromOpenAI([]byte(body), "claude-x") + if err != nil { + t.Fatalf("anthropicResponseFromOpenAI: %v", err) + } + // ...must translate back to Anthropic's tool_use shape. + assert.Equal(t, "tool_use", anthropicOut["stop_reason"]) + content, _ := anthropicOut["content"].([]any) + if len(content) != 1 { + t.Fatalf("content = %v, want one tool_use block", anthropicOut["content"]) + } + block, _ := content[0].(map[string]any) + assert.Equal(t, "get_weather", block["name"]) + input, _ := block["input"].(map[string]any) + assert.Equal(t, "London", input["city"]) +} + +// TestOpenAIRequestFromAnthropic_NoTools_OmitsToolsAndToolChoiceFields +// proves a request that never set Anthropic's "tools"/"tool_choice" +// fields produces an OpenAI request with NEITHER key present at all — not +// an empty "tools":[] array, which some OpenAI-compatible upstreams treat +// differently from the field's outright absence. +// +// MUTATION PROVEN: changing `if toolsRaw, ok := req["tools"].([]any); ok` +// to always assign `out["tools"] = openAIToolsFromAnthropic(toolsRaw)` +// regardless of ok made this test fail (an empty "tools" key appeared) — +// confirmed, then reverted. +func TestOpenAIRequestFromAnthropic_NoTools_OmitsToolsAndToolChoiceFields(t *testing.T) { + req := map[string]any{ + "model": "claude-x", + "messages": []any{map[string]any{"role": "user", "content": "hi"}}, + } + out, _, err := openAIRequestFromAnthropic(req) + if err != nil { + t.Fatalf("openAIRequestFromAnthropic: %v", err) + } + if _, ok := out["tools"]; ok { + t.Errorf(`out["tools"] = %v, want the key absent when no tools were requested`, out["tools"]) + } + if _, ok := out["tool_choice"]; ok { + t.Errorf(`out["tool_choice"] = %v, want the key absent when no tool_choice was requested`, out["tool_choice"]) + } +} From a45bd291ea132d5147a5a2cacc6d4d43f7e83a29 Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Sun, 23 Aug 2026 00:53:43 +0100 Subject: [PATCH 2/3] test(limits): pin store fail-open/fail-closed policy and latch skip Cover failPolicyIncrBy (0% covered), and add direct tests for storeIncrBy and storeGet against a real store, not only the nil-store fallback path the existing suite exercised. Each direct test proves four branches: a nil store always uses the in-process fallback, a configured store's success returns the store's own value rather than the fallback's, a store error records the failure and falls through to the fail-open/fail-closed policy, and an open store-down latch skips the network call entirely until it expires. Add a direct test for logSpawnDrop (0% covered), proving it logs once per storeErrorLogEvery like its sibling logStoreError, sharing the same rate-limit gate by design. Every test was confirmed to fail against a real mutation of the target function before being kept; each mutation and its revert are recorded in the test's own doc comment. --- limits_test.go | 236 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 236 insertions(+) diff --git a/limits_test.go b/limits_test.go index 5362e11..bbd8b71 100644 --- a/limits_test.go +++ b/limits_test.go @@ -6,6 +6,7 @@ import ( "fmt" "math" "net/http" + "strings" "testing" "time" @@ -1099,6 +1100,241 @@ func TestLimiter_FailPolicyGet(t *testing.T) { } } +// TestLimiter_FailPolicyIncrBy mirrors TestLimiter_FailPolicyGet for the +// write side: failOpen=true counts n into the in-process fallback and +// returns its real post-increment value; failOpen=false refuses outright +// (0, false) WITHOUT touching the fallback at all — a caller must not +// double-count once the store recovers by having silently counted +// locally on the refused path too. +// +// MUTATION PROVEN: removing the `if !l.failOpen { return 0, false }` +// guard (falling straight through to the fallback increment regardless of +// failOpen) made the "failOpen false" case return ok=true and leave a +// nonzero fallback counter instead of refusing untouched — confirmed via +// `go test -run TestLimiter_FailPolicyIncrBy`, then reverted with `git +// checkout -- limits.go`. +func TestLimiter_FailPolicyIncrBy(t *testing.T) { + cases := []struct { + name string + failOpen bool + wantOK bool + wantV int64 + }{ + {name: "failOpen true counts in the fallback", failOpen: true, wantOK: true, wantV: 3}, + {name: "failOpen false refuses without touching the fallback", failOpen: false, wantOK: false, wantV: 0}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + l := newLimiter(nil, c.failOpen) + v, ok := l.failPolicyIncrBy("k", 3, time.Minute) + require.Equal(t, c.wantOK, ok) + assert.Equal(t, c.wantV, v) + + got, err := l.fallback.get("k") + if err != nil { + t.Fatalf("fallback.get: %v", err) + } + wantFallback := int64(0) + if c.failOpen { + wantFallback = 3 + } + if got != wantFallback { + t.Errorf("fallback[k] = %d, want %d", got, wantFallback) + } + }) + } +} + +// fixedStore is a counterStore stub whose incrBy/get always succeed with a +// fixed value, independent of key/n/ttl — used to prove storeIncrBy/ +// storeGet return the STORE's own reported value on success, rather than +// silently substituting the in-process fallback's. +type fixedStore struct { + incrVal int64 + getVal int64 +} + +func (s *fixedStore) incrBy(string, int64, time.Duration) (int64, error) { return s.incrVal, nil } +func (s *fixedStore) get(string) (int64, error) { return s.getVal, nil } +func (s *fixedStore) getMulti([]string) ([]int64, error) { return nil, nil } +func (s *fixedStore) incrMulti([]counterIncr) ([]int64, error) { return nil, nil } +func (s *fixedStore) incrAndGetMulti([]counterIncr, []string) ([]int64, []int64, error) { + return nil, nil, nil +} + +// TestLimiter_StoreIncrBy_DirectPaths exercises storeIncrBy's four +// branches directly. storeIncrBy has no production caller since +// checkAndCount/account moved to the batched storeIncrMulti (its own doc +// comment) — its only caller, incrCounter, is itself only called from +// tests for direct counter seeding, which is why the existing suite only +// ever drove it through the nil-store path (30% baseline coverage): a nil +// store always uses the in-process fallback; a configured store's +// successful call returns the STORE's own value; a store error records +// the failure and falls through to the fail-open/fail-closed policy; and +// once the store-down latch opens from a prior failure, the store is +// never called again until the latch expires. +// +// MUTATION PROVEN: deleting the `if l.storeLatched() { return +// l.failPolicyIncrBy(key, n, ttl) }` branch made the "latched skips the +// network call" subtest fail (the erroring store's incrBy was invoked a +// second time instead of being skipped) — confirmed via `go test -run +// TestLimiter_StoreIncrBy_DirectPaths/latched`, then reverted. +func TestLimiter_StoreIncrBy_DirectPaths(t *testing.T) { + t.Run("nil store uses the fallback", func(t *testing.T) { + l := newLimiter(nil, true) + v, ok := l.storeIncrBy("k", 5, time.Minute) + require.True(t, ok) + assert.Equal(t, int64(5), v) + got, err := l.fallback.get("k") + if err != nil { + t.Fatalf("fallback.get: %v", err) + } + if got != 5 { + t.Errorf("fallback[k] = %d, want 5", got) + } + }) + + t.Run("configured store success returns the store's own value", func(t *testing.T) { + store := &fixedStore{incrVal: 42} + l := newLimiter(store, true) + v, ok := l.storeIncrBy("k", 5, time.Minute) + require.True(t, ok) + assert.Equal(t, int64(42), v) + }) + + t.Run("store error falls through to the fail-open policy", func(t *testing.T) { + store := &erroringStore{err: errors.New("boom")} + l := newLimiter(store, true) + v, ok := l.storeIncrBy("k", 5, time.Minute) + require.True(t, ok) + assert.Equal(t, int64(5), v) + got, err := l.fallback.get("k") + if err != nil { + t.Fatalf("fallback.get: %v", err) + } + if got != 5 { + t.Errorf("fallback[k] = %d, want 5 (fail-open must count in the fallback)", got) + } + }) + + t.Run("store error with failOpen false refuses", func(t *testing.T) { + store := &erroringStore{err: errors.New("boom")} + l := newLimiter(store, false) + _, ok := l.storeIncrBy("k", 5, time.Minute) + require.False(t, ok) + }) + + t.Run("latched skips the network call entirely", func(t *testing.T) { + store := &countingErrorStore{err: errors.New("boom")} + l := newLimiter(store, true) + now := time.Date(2026, 8, 20, 10, 0, 0, 0, time.UTC) + l.nowFn = func() time.Time { return now } + + l.storeIncrBy("k", 1, time.Minute) // first call: reaches the store, opens the latch + if store.incrCalls != 1 { + t.Fatalf("incrCalls after first call = %d, want 1", store.incrCalls) + } + l.storeIncrBy("k", 1, time.Minute) // second call: within the latch window + if store.incrCalls != 1 { + t.Errorf("incrCalls after second call = %d, want still 1 (the latch must skip the store)", store.incrCalls) + } + }) +} + +// TestLimiter_StoreGet_DirectPaths mirrors +// TestLimiter_StoreIncrBy_DirectPaths for the read side. The round-3 perf +// fusion moved checkAndCount's own reads onto storeGetMulti/ +// incrAndGetMulti, so a direct call is storeGet's only remaining exercise +// of its store!=nil branches (70% baseline coverage, from other tests +// that only drove it through account/currentUsage's own indirect paths). +// +// MUTATION PROVEN: deleting the `if l.storeLatched() { return +// l.failPolicyGet(key) }` branch made the "latched skips the network +// call" subtest fail (the erroring store's get was invoked a second time) +// — confirmed, then reverted. +func TestLimiter_StoreGet_DirectPaths(t *testing.T) { + t.Run("nil store uses the fallback", func(t *testing.T) { + l := newLimiter(nil, true) + if _, err := l.fallback.incrBy("k", 7, time.Minute); err != nil { + t.Fatalf("fallback.incrBy: %v", err) + } + v, ok := l.storeGet("k") + require.True(t, ok) + assert.Equal(t, int64(7), v) + }) + + t.Run("configured store success returns the store's own value", func(t *testing.T) { + store := &fixedStore{getVal: 99} + l := newLimiter(store, true) + v, ok := l.storeGet("k") + require.True(t, ok) + assert.Equal(t, int64(99), v) + }) + + t.Run("store error with failOpen false refuses", func(t *testing.T) { + store := &erroringStore{err: errors.New("boom")} + l := newLimiter(store, false) + _, ok := l.storeGet("k") + require.False(t, ok) + }) + + t.Run("latched skips the network call entirely", func(t *testing.T) { + store := &countingErrorStore{err: errors.New("boom")} + l := newLimiter(store, true) + now := time.Date(2026, 8, 20, 10, 0, 0, 0, time.UTC) + l.nowFn = func() time.Time { return now } + + l.storeGet("k") // first call: reaches the store, opens the latch + if store.getCalls != 1 { + t.Fatalf("getCalls after first call = %d, want 1", store.getCalls) + } + l.storeGet("k") // second call: within the latch window + if store.getCalls != 1 { + t.Errorf("getCalls after second call = %d, want still 1 (the latch must skip the store)", store.getCalls) + } + }) +} + +// TestLimiter_LogSpawnDrop_RateLimited proves logSpawnDrop logs one line +// describing the dropped telemetry write, rate-limited to once per +// storeErrorLogEvery regardless of how many drops happen in between — +// mirroring TestLimiter_LogsStoreErrorOncePerRateLimit, since logSpawnDrop +// deliberately shares lastLogAt with logStoreError (logSpawnDrop's own +// doc comment: "a stressed store... is the same 'the store is struggling' +// story"). +// +// MUTATION PROVEN: removing the `if shouldLog { ... }` guard around the +// l.logf call (logging unconditionally on every drop) made the +// "still inside the window" assertion fail (2 lines logged after the +// second call instead of 1) — confirmed via `go test -run +// TestLimiter_LogSpawnDrop_RateLimited`, then reverted. +func TestLimiter_LogSpawnDrop_RateLimited(t *testing.T) { + l := newLimiter(nil, true) + now := time.Date(2026, 8, 20, 10, 0, 0, 0, time.UTC) + l.nowFn = func() time.Time { return now } + var logged []string + l.logf = func(format string, args ...any) { logged = append(logged, fmt.Sprintf(format, args...)) } + + l.logSpawnDrop() + if len(logged) != 1 { + t.Fatalf("logged = %d lines after the first drop, want 1", len(logged)) + } + if !strings.Contains(logged[0], "limit telemetry write dropped") { + t.Errorf("logged[0] = %q, want it to mention the telemetry write drop", logged[0]) + } + + l.logSpawnDrop() // still inside the rate-limit window + if len(logged) != 1 { + t.Fatalf("logged = %d lines after a second drop inside the window, want still 1", len(logged)) + } + + now = now.Add(storeErrorLogEvery) + l.logSpawnDrop() + if len(logged) != 2 { + t.Fatalf("logged = %d lines after the rate-limit window elapsed, want 2", len(logged)) + } +} + // --- perf review, 2026-08-21: single-batch counter writes --- // countingIncrStore is a counterStore stub whose incrMulti/getMulti/ From 49505227db112c5aeeee1d9fad4c9a534cb09b9e Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Sun, 23 Aug 2026 00:55:12 +0100 Subject: [PATCH 3/3] test(cache): pin streaming requests as never cacheable Cover an undefended half of runMeteredCall's own cacheable guard (routes_unified.go): "!streaming" was asserted nowhere end to end. Existing streaming tests only checked chunk delivery and mid-stream accounting, never the cache header or store state. Add a test that drives a real streaming chat-completion request through a Gateway with caching enabled and proves no X-Llmgw-Cache header appears, and that the cache holds no entry under the key a non-streaming request for the same body would use. Confirmed to fail against a real mutation (dropping "!streaming" from the cacheable expression) before being kept; the mutation and its revert are recorded in the test's own doc comment. --- routes_unified_test.go | 57 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/routes_unified_test.go b/routes_unified_test.go index e005a80..42eb7ce 100644 --- a/routes_unified_test.go +++ b/routes_unified_test.go @@ -1319,6 +1319,63 @@ func TestHandleChat_CacheOversizeBody_SkipsStore_StillServesEveryRequest(t *test } } +// TestHandleChat_StreamingRequest_NeverCachedOrServedFromCache is the +// direct regression for runMeteredCall's own `cacheable := !streaming && +// ...` guard (routes_unified.go): a streaming chat-completion request +// must never get an X-Llmgw-Cache header at all (it is never looked up +// nor stored — the whole cacheable block is skipped), and nothing the +// streaming call did may have populated the cache under that body's own +// key. +// +// MUTATION PROVEN: changing `cacheable := !streaming && g.cache != nil && +// groupCacheEnabled(grp)` to drop the `!streaming` term made this test +// fail — the streaming response picked up an X-Llmgw-Cache: miss header +// — confirmed via `go test -run +// TestHandleChat_StreamingRequest_NeverCachedOrServedFromCache`, then +// reverted with `git checkout -- routes_unified.go`. +func TestHandleChat_StreamingRequest_NeverCachedOrServedFromCache(t *testing.T) { + var upstreamCalls int + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + upstreamCalls++ + w.Header().Set("Content-Type", "text/event-stream") + w.WriteHeader(http.StatusOK) + fl, ok := w.(http.Flusher) + if !ok { + t.Fatal("fake upstream ResponseWriter does not support Flush") + } + _, _ = w.Write([]byte("data: {\"id\":\"c1\",\"object\":\"chat.completion.chunk\",\"choices\":[{\"delta\":{\"content\":\"hi\"}}]}\n\n")) + fl.Flush() + _, _ = w.Write([]byte("data: [DONE]\n\n")) + fl.Flush() + })) + defer srv.Close() + + gw := newCacheTestGateway(t, srv, nil, defaultCacheMaxBodyBytes) + body := map[string]any{"model": "gpt-test", "stream": true, "messages": []any{map[string]any{"role": "user", "content": "hi"}}} + + req := newUnifiedRequest(t, http.MethodPost, "/v1/chat/completions", "sk-alice", body) + rw := newRecordingWriter() + gw.ServeHTTP(rw, req) + + if rw.status != http.StatusOK { + t.Fatalf("status = %d, want 200", rw.status) + } + if got := rw.Header().Get("X-Llmgw-Cache"); got != "" { + t.Errorf(`X-Llmgw-Cache = %q for a streaming request, want no header at all (never cacheable)`, got) + } + if upstreamCalls != 1 { + t.Fatalf("upstreamCalls = %d, want 1", upstreamCalls) + } + + // Nothing the streaming call did may have populated the cache under + // the key a non-streaming request with the same provider/model/body + // would have used. + key := cacheKey("openai", "gpt-test", "gpt-test", cacheEndpointChat, body) + if _, hit := gw.cache.lookup(key); hit { + t.Error("cache has an entry for the streaming request's key, want none stored") + } +} + // TestHandleChat_CacheRedisDown_MissesAndServesNormallyWithLog proves // spec §2's "redis errors during cache ops → treat as miss ... never fail // the request because the cache is down": cfg.Redis points at an address