fix(#3996): classify Gemini API 400s into safe, actionable categories - #4017
Draft
aheritier wants to merge 3 commits into
Draft
fix(#3996): classify Gemini API 400s into safe, actionable categories#4017aheritier wants to merge 3 commits into
aheritier wants to merge 3 commits into
Conversation
Diagnose the observed google/gemini-2.5-flash-image HTTP 400 without dumping raw requests/prompts/schemas. RequestShape summarizes a built GenerateContentConfig as booleans/counts/fixed enums only (modalities, tool/toolconfig presence, structured-output presence, thinking config, API surface), safe to log at Debug and to hand to bug reports. Track apiSurface (gemini_api/vertex_ai/gateway) on Client so the shape can distinguish backends. Replace the old per-tool debug log in CreateChatCompletionStream, which logged full Tool structs including function names/descriptions/parameter schemas, with the bounded shape. Trace the Split Diff local-only classification from the actual request-building boundary rather than just this package: the regression scans this package (where the request is serialized) plus pkg/tools (where the []tools.Tool list handed to every provider is built) and pkg/runtime (where messages/tools are assembled before any provider call) for any reference to the TUI-local Split Diff toggle, and finds none. MCP and other dynamically registered tools are outside the reach of a static scan; that boundary is documented on the test.
Introduce chat.SanitizeDisplayName and chat.TruncateUTF8Bytes with the canonical MaxSanitizedFieldBytes (128) field bound. Provider-supplied display names are untrusted model output: the sanitizer neutralizes control characters, path separators, traversal-like sequences, and angle brackets (so a name can never forge an XML/tag boundary in prompts), and the truncation helper enforces byte bounds without splitting multi-byte runes. These helpers are shared infrastructure: the Gemini 400 classifier uses them to bound diagnostic fields, and later generated-media commits use them for every persisted or displayed metadata field.
google.golang.org/genai's newAPIError returns APIError by value (its Error() method has a value receiver), so wrapGeminiError's previous errors.AsType[*genai.APIError] type parameter never matched. Every Gemini API error — including the observed empty-body 400 — passed through unwrapped: no *modelerrors.StatusError, no status code for the retry loop, no classification. Fix the type parameter to the value type. Add a bounded RequestRejectionCategory, derived only from keyword matches against Google's own documented request field names in the SDK's Message (never Details or request content), and APIRejectionError to render an action-oriented message in its place. This message reaches the user through the existing modelerrors.FormatError -> runtime ErrorEvent -> TUI AddErrorMessage seam unchanged — no new plumbing needed — replacing what was previously just "HTTP 400: " with no actionable content. Preserve a bounded, sanitized rendering of the SDK's own Message alongside the category/hint instead of replacing it outright: general model-error classification (pkg/modelerrors) identifies context overflow and retryable transient function-response errors (issue #2683) by matching known phrases against err.Error(), and dropping Message broke both for Gemini 400s. An auth-shaped 400 (e.g. "API key not valid") isn't affected by either classifier — a *StatusError short-circuits before modelerrors' phrase-based fallback ever inspects it — but dropping Message there still cost the user the provider's own informative text, leaving only a generic "request shape" hint; that is a loss of visible context, not a classifier failure. sanitizeAPIErrorMessage bounds the result, collapses control characters/newlines to a single line, rewrites '{'/'}' to '('/')' so a JSON-object-shaped envelope embedded in Message can't later be re-parsed by modelerrors.StatusError.Error and have its fields substitute for this package's own category/hint, and redacts any recognizable credential via portcullis.Redact before it is echoed; Details (the raw response body) is still never surfaced. Truncate the sanitized message via pkg/chat.TruncateUTF8Bytes rather than duplicating a local copy of it: that helper is already available in pkg/chat, so there is no standalone-build reason left to keep a package-local truncateUTF8Bytes. Cover the classification chain (overflow, transient retry, auth) and the JSON-envelope-neutralization/redaction/bounding contract with new tests; pkg/chat.TruncateUTF8Bytes's own multi-byte boundary conditions are covered by its existing tests in pkg/chat.
aheritier
added
area/providers/gemini
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Classifies Gemini API 400 responses into safe, actionable categories and surfaces consistent display names and diagnostics.
Why
Turns opaque Gemini request failures into useful user-facing guidance without exposing unsafe provider details.
Validation
task test; Gemini classification, diagnostics, wrapping, and display-name tests.Test instructions
Run:
task testSend an intentionally invalid Gemini request. Expected: the failure is reported in a safe, actionable category with a useful display name and diagnostic guidance, rather than exposing a raw or misleading HTTP 400 response.