Skip to content

Top-10 improvements: thinking/signature replay, Gemini fixes, retry policy, request controls, embeddings - #14

Closed
jkyberneees wants to merge 8 commits into
mainfrom
claude/top-10-improvements-5l14eg
Closed

jkyberneees wants to merge 8 commits into
mainfrom
claude/top-10-improvements-5l14eg

Conversation

@jkyberneees

Copy link
Copy Markdown
Contributor

Fixes the bugs and fills the feature gaps from the top-10 audit, plus everything found by four independent adversarial reviewers. Every behavior change was test-first: the test failed, then the fix landed. One commit per area so it can be reviewed in order.

Numbers: unit-suite coverage went from 92.2% to 99.0%. The -race suite runs in about 35s, down from about 108s, because ListModels no longer retries parse errors with real backoff. golangci-lint reports 0 issues, and go vet -tags e2e plus the Windows and darwin builds are clean. I have not run the live e2e suite.

Bugs fixed (top-10 audit)

  1. Anthropic thinking requests would 400.
    • max_tokens is now always larger than budget_tokens. An unset MaxTokens becomes budget + 8192. A preset larger than half an explicit MaxTokens is clamped to half of it, never below 1024. An explicit budget that cannot fit returns a ConfigError.
    • temperature and top_p are omitted while thinking is on.
  2. Gemini thoughtSignature was dropped. It is now kept per function call (ToolCall.Signature) and on text parts, and sent back on the same parts. Gemini 3 rejects tool turns that lack it.
  3. Anthropic multi-block and redacted thinking was lost.
    • The new ChatResult.ThinkingBlocks / Message.ThinkingBlocks keep every thinking and redacted_thinking block in order, each with its own signature.
    • Streamed signatures are no longer concatenated across blocks.
    • An empty-text signed block still sends the "thinking" key.
  4. Gemini finish reasons were wrong.
    • STOP plus function calls now maps to tool_calls.
    • A stream that ends without any completion signal is now a premature close on every format; Gemini is no longer exempt.
    • A prompt blocked by promptFeedback finishes with content_filter instead of being retried 8 times.
  5. Delta.ToolIndex was inconsistent. It is now the call's position in ChatResult.ToolCalls on every format. Before, Anthropic used the content-block index and Responses used output_index.
  6. No overall budget on buffered calls, and no retry configuration.
    • WithRetryPolicy(RetryPolicy{MaxAttempts, MaxBackoff}) configures the ladder.
    • The request timeout is now the budget for the whole call, retries included, on every entry point.
    • The three copied retry loops (chat, TTS, STT) are now one shared helper, withRetry.
  7. Global mutable settings. WithStreamIdleTimeout and WithLearnObserver set these per SDK. SetStreamIdleTimeout is now atomic, so calling it while streams run is race-free.
  8. Usage meant different things per provider.
    • Gemini cached tokens are now reported, and Gemini thinking tokens count in CompletionTokens.
    • A DeepSeek cache miss stays in PromptTokens; it is no longer counted as a cache write.
    • A gateway that reports both Anthropic and OpenAI cache fields is not double-counted.
    • Moonshot's top-level cached_tokens is understood.
    • New Usage.InputTokens() and TotalTokens().
  • Also fixed:
    • The Gemini model id is path-escaped in the URL.
    • A Responses incomplete turn with reason content_filter maps to content_filter.
    • The Responses stream error event is handled.
    • A request that cannot be built is no longer retried 8 times.

Features added

  • Request controls:
    • ToolChoice (auto / none / required / a named tool), ParallelToolCalls, Seed.
    • ResponseFormat for json_object / json_schema. OpenAI uses response_format or text.format; Gemini uses responseMimeType + responseJsonSchema; Anthropic uses a forced tool whose input is folded back into Content, both buffered and streamed.
    • Everything is validated before any network call.
  • Passthrough: WithHeaders, applied last on every request (an empty value removes a header), and ChatRequest.Extra for top-level body fields. The keys the SDK validates are reserved.
  • SDK.Embed: OpenAI-compatible /embeddings and Gemini batchEmbedContents. Results come back in input order, with batching and count/index checks.
  • Tool results: Message.IsError, and image Parts on tool results, mapped per format.
  • Replay helper: ChatResult.AssistantMessage() builds the assistant turn with every replay field.
  • ListModels:
    • Concurrent cache misses share one upstream fetch, and a panic in that fetch can never block later calls.
    • A malformed body is not retried.
    • A listing longer than 100 pages returns ErrModelListTruncated instead of being silently truncated.

Security hardening (from review)

  • Redirects to another host are no longer followed. Go strips only Authorization across hosts, so x-api-key and custom headers would otherwise leak.
  • Provider and ProviderConfig now hide the API key and header values under %v, %+v and %#v.
  • CR/LF in STT Filename/MIMEType is rejected, to stop header injection.

Behavior changes to review

  • Buffered timeout: buffered Call, Speak, Transcribe and Embed now stop at the request timeout in total (default 120s). Before, each of up to 8 attempts got the full timeout.
  • DeepSeek usage: a cache miss is now in PromptTokens, not CacheCreationTokens. This is intentional but differs from the old odek-parity test, which I updated.
  • Gemini streams: a stream with no finishReason is now an error, as on every other format.
  • DeepSeek JSON: Quirks.NoJSONSchema is set for DeepSeek, so a json_schema response format fails fast. That DeepSeek supports only json_object comes from provider docs and has not been checked against the live API.
  • New struct fields were added; Thinking on the internal Anthropic block is now a pointer. No exported field was removed.

Review process

Four adversarial reviewers each worked in an isolated worktree: three Sonnet (streaming and thinking, retry and concurrency, request controls and wire formats) and one Haiku (invariants and security). Every finding they proved was fixed test-first in 4b60ce2 or bd73bba.

Not acted on, and documented instead:

  • Signatures are provider-specific, so don't replay them across providers.
  • Gemini 2.x rejects JSON mode combined with tools.
  • Extra merges only top-level keys (shallow).
  • The TTS audio/mpeg MIME fallback predates this PR; left as is.

The new surface and invariants 8–10 are documented in README.md and AGENTS.md.

🤖 Generated with Claude Code

https://claude.ai/code/session_01N76SRxxjJ4FgZGWmv4sGPS


Generated by Claude Code

claude added 8 commits October 8, 2026 10:42
…stream completion

- Anthropic: max_tokens always exceeds budget_tokens (unset → budget+8192,
  preset clamped below explicit cap, impossible explicit budget →
  ConfigError); temperature/top_p omitted under extended thinking
- Anthropic: every thinking and redacted_thinking block is captured in
  order (ChatResult.ThinkingBlocks) and replayed verbatim; streamed
  signatures are per block, never concatenated across blocks
- Gemini: thoughtSignature captured per function call (ToolCall.Signature)
  and on text parts (ThinkingSignature) and replayed on the same parts
- Gemini: a function-call turn finishes as tool_calls, like every format
- Streams that end at EOF without any completion signal are a premature
  close on every format, Gemini included; unmapped finish reasons still
  count as completion
- Delta.ToolIndex is the call's position in ChatResult.ToolCalls on every
  format (was the Anthropic content-block / Responses output index)
- Gemini model ids are path-escaped in the request URL
- ChatResult.AssistantMessage() builds the replay turn with every field

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N76SRxxjJ4FgZGWmv4sGPS
…rver, ListModels hardening

- RetryPolicy{MaxAttempts, MaxBackoff} via WithRetryPolicy, honored by
  Call, CallStream, Speak and Transcribe; one shared withRetry ladder
  replaces the three copied loops (chat, tts, stt)
- Buffered Call, Speak and Transcribe share the request timeout as a
  whole-call budget, retries included (as streaming already did) — a
  failing provider can no longer hold a caller for timeout x attempts
- Backoff shift is clamped so long ladders never overflow to zero delay
- WithStreamIdleTimeout and WithLearnObserver: per-SDK settings that take
  precedence over the process-wide defaults; SetStreamIdleTimeout is now
  atomic (race-free while streams run)
- ListModels: malformed bodies are terminal (no retry), the page cap
  returns ErrModelListTruncated instead of a silently truncated listing
  (cap raised to 100 pages), and concurrent cache misses share one
  upstream fetch; a follower never inherits its leader's cancellation

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N76SRxxjJ4FgZGWmv4sGPS
- Gemini: cachedContentTokenCount moves to CacheReadTokens (CacheReported
  set); thoughts are added to CompletionTokens so it includes reasoning on
  every format
- DeepSeek: a cache miss is ordinary uncached input, not a cache write;
  hit/miss fields are authoritative and never double-counted with an
  echoed cached_tokens
- Anthropic: cumulative message_delta usage updates input and cache
  volumes when present
- Usage.InputTokens() / TotalTokens(); CachedTokens documented as a
  diagnostic subset of CacheReadTokens (never summed)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N76SRxxjJ4FgZGWmv4sGPS
…ls, Seed

- ToolChoice{auto,none,required,tool}: OpenAI/Responses tool_choice,
  Anthropic tool_choice (auto/none/any/tool), Gemini
  toolConfig.functionCallingConfig (AUTO/NONE/ANY + allowedFunctionNames)
- ResponseFormat{text,json_object,json_schema}: OpenAI response_format,
  Responses text.format, Gemini responseMimeType + responseJsonSchema;
  Anthropic emulates it with a forced synthetic tool whose input is
  folded back into Content on buffered and streaming paths
- ParallelToolCalls: OpenAI/Responses parallel_tool_calls (only with
  tools), Anthropic disable_parallel_tool_use
- Seed: OpenAI chat completions and Gemini generationConfig.seed
- Controls are validated at the SDK boundary (ConfigError), including
  Anthropic forced tool use under extended thinking

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N76SRxxjJ4FgZGWmv4sGPS
- ProviderConfig.Headers / WithHeaders: applied last on every request
  (chat, streaming, models, speech, transcription); an empty value removes
  an SDK header (e.g. Authorization for api-key gateways); values never
  appear in String() or errors; the map is copied
- ChatRequest.Extra: top-level body fields merged over the SDK's own on
  every format (Responses included); "stream" is reserved and
  non-JSON values fail fast with a ConfigError

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N76SRxxjJ4FgZGWmv4sGPS
…items

- SDK.Embed (EmbedRequest/EmbedResult): OpenAI-compatible /embeddings and
  Gemini batchEmbedContents; vectors returned in input order with count
  and index checks; shared retry ladder, budget, headers and error types;
  Anthropic (no endpoint) is a ConfigError
- Message.IsError for tool results: Anthropic is_error, Gemini
  {"error": …}; ConfigError outside the tool role
- Tool results accept text+image Parts: Anthropic tool_result blocks,
  Responses input_text/input_image output, Gemini inlineData in the same
  turn, chat completions images in a follow-up user message
- Responses: every reasoning item becomes a ThinkingBlock and is replayed
  in order (was: last encrypted_content only); top-level stream "error"
  events fail the stream with the provider message
- Unauthenticated errors name the env vars actually consulted (custom
  providers without EnvKeys no longer suggest a variable nobody reads)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N76SRxxjJ4FgZGWmv4sGPS
Found by four independent reviewers (each finding reproduced RED first):
- ListModels single-flight: a panicking fetch no longer wedges the
  provider (flight always released, followers get an error); an upstream
  timeout is the shared result instead of re-leading per follower
- Anthropic: a signed thinking block with empty text keeps the required
  "thinking" key; preset budgets clamp to half an explicit MaxTokens
  (never a one-token answer); disable_parallel_tool_use never rides a
  "none" choice; JSON mode rejects Tools and non-object schemas;
  sparse message_delta usage never zeroes output tokens
- Gemini: promptFeedback.blockReason completes as content_filter (was
  retried 8x as a premature close); seeds must fit in 32 bits
- Responses: incomplete+content_filter maps to content_filter
- Usage: gateway Anthropic+OpenAI cache fields never double-count;
  Moonshot top-level cached_tokens is understood
- DeepSeek: Quirks.NoJSONSchema fails json_schema fast (json_object only)
- Embed: Gemini models/ prefix accepted; batches of 100 (Gemini) / 2048
  (OpenAI); gateways omitting index are trusted in order
- Security: cross-host redirects are never followed (x-api-key and custom
  headers would leak); Provider/ProviderConfig redact under %v/%+v/%#v;
  Extra cannot replace validated keys (messages, tools, …); STT
  Filename/MIMEType control characters are rejected (header injection)
- withRetry reports the context error on an interrupted last attempt;
  a mid-stream buffered fallback stays within the call's budget
- README.md and AGENTS.md describe every new option, control and
  invariant

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N76SRxxjJ4FgZGWmv4sGPS
- Responses: an incomplete turn with incomplete_details content_filter now
  finishes content_filter (was length / tool_calls)
- A request that cannot be built (*ConfigError from the transport layer)
  is terminal instead of being retried 8 times; base URLs that do not
  parse are rejected at wiring time
- Dead branches removed (Responses failed-with-message, streaming learn
  retry without an APIError, an unreachable exhaustion return,
  assistant-role Parts in the OpenAI builder)
- Edge tests for body read failures, learn triggers on the final attempt,
  429-then-definitive ladders, long backoff ladders, same-host redirects,
  follower cancellation, Responses finish/usage/stream edges, content-part
  validation, model CreatedAt, speech transport errors

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N76SRxxjJ4FgZGWmv4sGPS

Copy link
Copy Markdown
Contributor Author

Superseded by #15: same commits, with the branch renamed to fix/top-10-improvements to follow the repo's fix/ convention.


Generated by Claude Code

@jkyberneees jkyberneees closed this Oct 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants