fix(anthropicapi): carry top_p/user in typed fields + architecture-review quick wins

.gitattributes

@@ -0,0 +1,4 @@
+# Generated files: collapse in diffs and GitHub language stats.
+# Regenerate with `make swagger` / `make docs-openapi`.
+cmd/gomodel/docs/docs.go linguist-generated=true
+docs/openapi.json linguist-generated=true

CLAUDE.md

@@ -130,6 +130,6 @@ Full reference: `.env.template` and `config/config.yaml`
 - **HTTP client:** `HTTP_TIMEOUT` (600s), `HTTP_RESPONSE_HEADER_TIMEOUT` (600s)
 - **Resilience:** Configured via `config/config.yaml` - global `resilience.retry.*` and `resilience.circuit_breaker.*` defaults with optional per-provider overrides under `providers.<name>.resilience.retry.*` and `providers.<name>.resilience.circuit_breaker.*`. Retry defaults: `max_retries` (3), `initial_backoff` (1s), `max_backoff` (30s), `backoff_factor` (2.0), `jitter_factor` (0.1). Circuit breaker defaults: `failure_threshold` (5), `success_threshold` (2), `timeout` (30s)
 - **Metrics:** `METRICS_ENABLED` (false), `METRICS_ENDPOINT` (/metrics)
-- **Guardrails:** Configured via `config/config.yaml` only (except `GUARDRAILS_ENABLED` env var)
+- **Guardrails:** Definitions are persisted in the `guardrail_definitions` store and managed via the admin API/dashboard; `config/config.yaml` entries are validated and upserted into that store at startup (a seed, not the source of truth). `GUARDRAILS_ENABLED` env var gates the feature.
 - **Providers:** `OPENAI_API_KEY`, `ANTHROPIC_API_KEY`, `GEMINI_API_KEY`, `USE_GOOGLE_GEMINI_NATIVE_API` (true by default; false uses Gemini's OpenAI-compatible chat API), `XAI_API_KEY`, `GROQ_API_KEY`, `OPENROUTER_API_KEY`, `ZAI_API_KEY`, `ZAI_BASE_URL` (optional Z.ai endpoint override), `MINIMAX_API_KEY`, `MINIMAX_BASE_URL` (optional MiniMax endpoint override), `XIAOMI_API_KEY`, `XIAOMI_BASE_URL` (optional Xiaomi MiMo endpoint override), `OPENCODE_GO_API_KEY`, `OPENCODE_GO_BASE_URL` (optional OpenCode Go/Zen endpoint override; default `https://opencode.ai/zen/go/v1`), `OPENCODE_GO_MESSAGES_MODELS` (optional comma-separated model IDs routed to the Anthropic-native `/messages` endpoint instead of `/chat/completions`; default `qwen3.7-max`), `BAILIAN_API_KEY`, `BAILIAN_BASE_URL` (optional Bailian base URL for region switching; default `https://dashscope.aliyuncs.com/compatible-mode/v1`), `AZURE_API_KEY`, `AZURE_BASE_URL` (Azure OpenAI deployment base URL), `AZURE_API_VERSION` (optional Azure API version), `ORACLE_API_KEY` (Oracle API key), `ORACLE_BASE_URL` (Oracle OpenAI-compatible base URL), `<PROVIDER>[_SUFFIX]_MODELS` (comma-separated configured model list for any provider type), `OLLAMA_BASE_URL`, `VLLM_BASE_URL`, `VLLM_API_KEY` (optional upstream vLLM bearer token)
 - **Provider model metadata:** `providers.<name>.models` accepts either model IDs (strings) or `{id, metadata}` objects. When `metadata` is supplied (`display_name`, `context_window`, `max_output_tokens`, `modes`, `capabilities`, `pricing`, …) it is merged onto the remote ai-model-list entry during enrichment, with operator values winning per-field. Primary use case: advertising context windows, capabilities, and pricing for local models (Ollama) and other custom endpoints whose IDs are not in the upstream registry.

docs/dev/possible-refactoring.md

@@ -4,6 +4,8 @@ Ordered by lowest effort and lowest risk first.
 
 ## 1. Remove dead `CacheTypeBoth`
 
+Status: done (2026-07-04)
+
 Effort: very low
 Risk: very low
 

internal/anthropicapi/request.go

@@ -60,9 +60,13 @@ func ToChatRequest(req *MessagesRequest) (*core.ChatRequest, error) {
 		Messages:    messages,
 		MaxTokens:   &maxTokens,
 		Temperature: req.Temperature,
+		TopP:        req.TopP,
 		Stream:      req.Stream,
 		Reasoning:   thinkingToReasoning(req.Thinking),
 	}
+	if req.Metadata != nil && strings.TrimSpace(req.Metadata.UserID) != "" {
+		chat.User = req.Metadata.UserID
+	}
 	if req.Stream {
 		chat.StreamOptions = &core.StreamOptions{IncludeUsage: true}
 	}
@@ -433,28 +437,20 @@ func thinkingToReasoning(thinking *Thinking) *core.Reasoning {
 }
 
 // buildExtraFields carries Anthropic request fields that have a portable
-// OpenAI-compatible equivalent through as extra fields.
+// OpenAI-compatible equivalent but no typed core.ChatRequest field. Fields
+// with typed equivalents (top_p, user) are set directly on the ChatRequest in
+// ToChatRequest so internal consumers of the typed fields see them too.
 //
 // top_k is deliberately not carried: it is not a valid OpenAI Chat Completions
 // parameter, and the OpenAI-family providers forward request fields verbatim
 // and reject unknown ones with a 400. Carrying it would make any request with
 // top_k fail when routed to those providers, so it is dropped (see ADR-0007).
-// stop, top_p, and user are all portable across OpenAI-compatible providers.
 func buildExtraFields(req *MessagesRequest) core.UnknownJSONFields {
 	fields := map[string]json.RawMessage{}
-	add := func(key string, value any) {
-		if raw, err := json.Marshal(value); err == nil {
-			fields[key] = raw
-		}
-	}
 	if len(req.StopSequences) > 0 {
-		add("stop", req.StopSequences)
-	}
-	if req.TopP != nil {
-		add("top_p", *req.TopP)
-	}
-	if req.Metadata != nil && strings.TrimSpace(req.Metadata.UserID) != "" {
-		add("user", req.Metadata.UserID)
+		if raw, err := json.Marshal(req.StopSequences); err == nil {
+			fields["stop"] = raw
+		}
 	}
 	return core.UnknownJSONFieldsFromMap(fields)
 }

internal/anthropicapi/request_test.go

@@ -365,14 +365,21 @@ func TestToChatRequestExtraFields(t *testing.T) {
 	if err != nil {
 		t.Fatalf("ToChatRequest: %v", err)
 	}
-	for key, want := range map[string]string{
-		"stop":  `["STOP"]`,
-		"top_p": `0.9`,
-		"user":  `"u-123"`,
-	} {
-		raw := chat.ExtraFields.Lookup(key)
-		if string(raw) != want {
-			t.Errorf("ExtraFields[%q] = %s, want %s", key, raw, want)
+	if raw := chat.ExtraFields.Lookup("stop"); string(raw) != `["STOP"]` {
+		t.Errorf("ExtraFields[stop] = %s, want [\"STOP\"]", raw)
+	}
+	// top_p and user have typed ChatRequest fields; they must land there so
+	// internal consumers of the typed fields (Responses lowering, provider
+	// adapters) see them, and must not also ride in ExtraFields.
+	if chat.TopP == nil || *chat.TopP != 0.9 {
+		t.Errorf("TopP = %v, want 0.9", chat.TopP)
+	}
+	if chat.User != "u-123" {
+		t.Errorf("User = %q, want u-123", chat.User)
+	}
+	for _, key := range []string{"top_p", "user"} {
+		if raw := chat.ExtraFields.Lookup(key); len(raw) > 0 {
+			t.Errorf("ExtraFields[%q] = %s, want typed field only", key, raw)
 		}
 	}
 	// top_k has no portable OpenAI-compatible equivalent and OpenAI-family

internal/llmclient/circuit_breaker.go

@@ -68,12 +68,6 @@ func (cb *circuitBreaker) acquire() (bool, bool) {
 	return true, false
 }
 
-// Allow reports whether any request may proceed.
-func (cb *circuitBreaker) Allow() bool {
-	allowed, _ := cb.acquire()
-	return allowed
-}
-
 // RecordSuccess records a successful request
 func (cb *circuitBreaker) RecordSuccess() {
 	cb.mu.Lock()

internal/providers/auth_headers.go

@@ -26,6 +26,21 @@ type AuthHeaderConfig struct {
 	OptionalAPIKey bool
 }
 
+// IsValidClientRequestID reports whether id may be forwarded as a client
+// request-ID header value: upstreams that accept one (OpenAI, OpenRouter,
+// Azure) require printable ASCII and reject oversized values with a 400.
+func IsValidClientRequestID(id string) bool {
+	if len(id) > 512 {
+		return false
+	}
+	for i := 0; i < len(id); i++ {
+		if id[i] < 0x20 || id[i] > 0x7E {
+			return false
+		}
+	}
+	return true
+}
+
 // SetAuthHeaders applies cfg to req for the given API key. It is safe to use
 // directly as an llmclient header hook or as CompatibleProviderConfig.SetHeaders.
 func SetAuthHeaders(req *http.Request, apiKey string, cfg AuthHeaderConfig) {

internal/providers/auth_headers_test.go

@@ -3,6 +3,7 @@ package providers
 import (
 	"context"
 	"net/http"
+	"strings"
 	"testing"
 
 	"gomodel/internal/core"
@@ -107,3 +108,81 @@ func TestSetAuthHeaders(t *testing.T) {
 		})
 	}
 }
+
+func TestIsValidClientRequestID(t *testing.T) {
+	tests := []struct {
+		name  string
+		id    string
+		valid bool
+	}{
+		{
+			name:  "valid UUID",
+			id:    "123e4567-e89b-12d3-a456-426614174000",
+			valid: true,
+		},
+		{
+			name:  "valid short ID",
+			id:    "req-123",
+			valid: true,
+		},
+		{
+			name:  "valid empty string",
+			id:    "",
+			valid: true,
+		},
+		{
+			name:  "valid 512 chars",
+			id:    strings.Repeat("a", 512),
+			valid: true,
+		},
+		{
+			name:  "invalid - 513 chars (too long)",
+			id:    strings.Repeat("a", 513),
+			valid: false,
+		},
+		{
+			name:  "invalid - non-ASCII character",
+			id:    "req-123-日本語",
+			valid: false,
+		},
+		{
+			name:  "invalid - control character",
+			id:    "req-123\n456",
+			valid: false,
+		},
+		{
+			name:  "invalid - NUL byte",
+			id:    "req-123\x00",
+			valid: false,
+		},
+		{
+			name:  "invalid - tab",
+			id:    "req\t123",
+			valid: false,
+		},
+		{
+			name:  "invalid - DEL",
+			id:    "req-123\x7f",
+			valid: false,
+		},
+		{
+			name:  "invalid - emoji",
+			id:    "req-123-🎉",
+			valid: false,
+		},
+		{
+			name:  "valid - all printable ASCII",
+			id:    "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_.",
+			valid: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := IsValidClientRequestID(tt.id)
+			if got != tt.valid {
+				t.Errorf("IsValidClientRequestID(%q) = %v, want %v", tt.id, got, tt.valid)
+			}
+		})
+	}
+}

internal/providers/azure/azure.go

@@ -165,22 +165,10 @@ func setHeaders(req *http.Request, apiKey string) {
 	providers.SetAuthHeaders(req, apiKey, providers.AuthHeaderConfig{
 		AuthHeader:        "api-key",
 		RequestIDHeader:   "X-Client-Request-Id",
-		ValidateRequestID: isValidClientRequestID,
+		ValidateRequestID: providers.IsValidClientRequestID,
 	})
 }
 
-func isValidClientRequestID(id string) bool {
-	if len(id) > 512 {
-		return false
-	}
-	for i := 0; i < len(id); i++ {
-		if id[i] > 127 {
-			return false
-		}
-	}
-	return true
-}
-
 func resourceRootBaseURL(baseURL string) string {
 	parsed, err := url.Parse(strings.TrimSpace(baseURL))
 	if err != nil {

internal/providers/openai/openai.go

@@ -64,29 +64,16 @@ func NewWithHTTPClient(apiKey string, httpClient *http.Client, hooks llmclient.H
 
 // setHeaders sets the required headers for OpenAI API requests.
 // OpenAI requires the request ID to be ASCII-only and at most 512 bytes,
-// otherwise it returns 400, so forwarding is gated by isValidClientRequestID.
+// otherwise it returns 400, so forwarding is gated by
+// providers.IsValidClientRequestID.
 func setHeaders(req *http.Request, apiKey string) {
 	providers.SetAuthHeaders(req, apiKey, providers.AuthHeaderConfig{
 		AuthScheme:        "Bearer ",
 		RequestIDHeader:   "X-Client-Request-Id",
-		ValidateRequestID: isValidClientRequestID,
+		ValidateRequestID: providers.IsValidClientRequestID,
 	})
 }
 
-// isValidClientRequestID checks if the request ID is valid for OpenAI's X-Client-Request-Id header.
-// OpenAI requires: ASCII characters only, max 512 characters.
-func isValidClientRequestID(id string) bool {
-	if len(id) > 512 {
-		return false
-	}
-	for i := 0; i < len(id); i++ {
-		if id[i] > 127 {
-			return false
-		}
-	}
-	return true
-}
-
 // isOSeriesModel reports whether the model is an OpenAI o-series model
 // (o1, o3, o4) that requires max_completion_tokens instead of max_tokens
 // and does not support the temperature parameter.

internal/providers/openai/openai_test.go

@@ -2089,64 +2089,6 @@ func TestChatCompletion_ReasoningModel_PreservesToolConfiguration(t *testing.T)
 	}
 }
 
-func TestIsValidClientRequestID(t *testing.T) {
-	tests := []struct {
-		name  string
-		id    string
-		valid bool
-	}{
-		{
-			name:  "valid UUID",
-			id:    "123e4567-e89b-12d3-a456-426614174000",
-			valid: true,
-		},
-		{
-			name:  "valid short ID",
-			id:    "req-123",
-			valid: true,
-		},
-		{
-			name:  "valid empty string",
-			id:    "",
-			valid: true,
-		},
-		{
-			name:  "valid 512 chars",
-			id:    strings.Repeat("a", 512),
-			valid: true,
-		},
-		{
-			name:  "invalid - 513 chars (too long)",
-			id:    strings.Repeat("a", 513),
-			valid: false,
-		},
-		{
-			name:  "invalid - non-ASCII character",
-			id:    "req-123-日本語",
-			valid: false,
-		},
-		{
-			name:  "invalid - emoji",
-			id:    "req-123-🎉",
-			valid: false,
-		},
-		{
-			name:  "valid - all printable ASCII",
-			id:    "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_.",
-			valid: true,
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			got := isValidClientRequestID(tt.id)
-			if got != tt.valid {
-				t.Errorf("isValidClientRequestID(%q) = %v, want %v", tt.id, got, tt.valid)
-			}
-		})
-	}
-}
-
 func TestPassthrough(t *testing.T) {
 	var gotPath string
 	var gotAuth string

internal/providers/openrouter/openrouter.go

@@ -79,7 +79,7 @@ func setHeaders(req *http.Request, apiKey string) {
 	providers.SetAuthHeaders(req, apiKey, providers.AuthHeaderConfig{
 		AuthScheme:        "Bearer ",
 		RequestIDHeader:   "X-Client-Request-Id",
-		ValidateRequestID: isValidClientRequestID,
+		ValidateRequestID: providers.IsValidClientRequestID,
 	})
 }
 
@@ -99,15 +99,3 @@ func headerValue(headers http.Header, key string) string {
 	}
 	return ""
 }
-
-func isValidClientRequestID(id string) bool {
-	if len(id) > 512 {
-		return false
-	}
-	for i := 0; i < len(id); i++ {
-		if id[i] > 127 {
-			return false
-		}
-	}
-	return true
-}

internal/responsecache/semantic.go

@@ -634,7 +634,6 @@ func WithGuardrailsHash(ctx context.Context, hash string) context.Context {
 const (
 	CacheTypeExact      = "exact"
 	CacheTypeSemantic   = "semantic"
-	CacheTypeBoth       = "both"
 	CacheHeaderExact    = "HIT (exact)"
 	CacheHeaderSemantic = "HIT (semantic)"
 )