mirror of
https://github.com/coder/coder.git
synced 2026-10-16 14:48:56 +08:00
fix(coderd/x/chatd): only retry without drop_block when it's rejected (#30821)
In #29792, once a provider and model had accepted `drop_block`, any 400 on a request with it retried once without it, since I didn't know what error a provider returns for an unsupported beta. That turned out to hide real errors. On dogfood, a chat replaying some very tall screenshots hit Anthropic's many-image limit: ``` messages.9.content.43.image.source.base64.data: At least one of the image dimensions exceed max allowed size for many-image requests: 3000 pixels ``` The retry without `drop_block` then hit the stale thinking block, so the chat failed with the binding error instead, and the image error only showed up in a warn log. To fix, we'll only retry without `drop_block` when the 400 is a known rejection of the field or its beta, and otherwise return the error as is and keep the hint. The catch is that if a provider rejects the beta with an error we don't recognise, chats on that model will see it until a restart clears the hint, rather than us recovering. Claude Code (as of 2.1.300) does the same on Anthropic's own API, Claude Platform on AWS included: it only retries without the beta when the 400 names it or the `block_binding` field. On Bedrock, Vertex and Mantle it also retries on any 400 it doesn't otherwise recognise, which we could copy if that catch turns out to matter. The oversized screenshots are a separate problem, since chatd doesn't resize images at all.
This commit is contained in:
@@ -5,6 +5,7 @@ import (
|
||||
"errors"
|
||||
"maps"
|
||||
"net/http"
|
||||
"slices"
|
||||
"strings"
|
||||
"sync"
|
||||
|
||||
@@ -23,16 +24,32 @@ const (
|
||||
thinkingBindingDropBlockPath = "thinking.block_binding.prefix_mismatch_behavior"
|
||||
)
|
||||
|
||||
// Anthropic reports "block_binding: Extra inputs are not permitted" or names
|
||||
// the rejected beta in an "Unexpected value(s)" error. Bedrock reports
|
||||
// unsupported betas as "invalid beta flag".
|
||||
var dropBlockRejections = []string{"block_binding", thinkingBindingBeta, "invalid beta flag"}
|
||||
|
||||
// isThinkingBindingError reports whether Anthropic rejected a replayed
|
||||
// signed thinking block because the system prompt or tools changed since
|
||||
// the block was produced.
|
||||
func isThinkingBindingError(err error) bool {
|
||||
text, ok := badRequestText(err)
|
||||
return ok && strings.Contains(text, "bound to a different conversation")
|
||||
}
|
||||
|
||||
func isDropBlockRejectedError(err error) bool {
|
||||
text, ok := badRequestText(err)
|
||||
return ok && slices.ContainsFunc(dropBlockRejections, func(pattern string) bool {
|
||||
return strings.Contains(text, pattern)
|
||||
})
|
||||
}
|
||||
|
||||
func badRequestText(err error) (string, bool) {
|
||||
var pe *fantasy.ProviderError
|
||||
if !errors.As(err, &pe) || pe.StatusCode != http.StatusBadRequest {
|
||||
return false
|
||||
return "", false
|
||||
}
|
||||
text := strings.ToLower(pe.Message + string(pe.ResponseBody))
|
||||
return strings.Contains(text, "bound to a different conversation")
|
||||
return strings.ToLower(pe.Message + string(pe.ResponseBody)), true
|
||||
}
|
||||
|
||||
type thinkingDropBlockKey struct {
|
||||
@@ -65,14 +82,10 @@ func (p *Server) withThinkingDropBlock(
|
||||
})
|
||||
}
|
||||
|
||||
// thinkingDropBlockModel makes Anthropic drop stale signed thinking blocks
|
||||
// instead of rejecting the request. chatd rebuilds the system prompt and
|
||||
// tools for every request, and some models bind signed thinking blocks to
|
||||
// that prefix. A hint for each provider and model picks whether the first
|
||||
// request sends the drop_block control. A 400 before any output retries
|
||||
// once with the other choice, and a successful retry updates the hint. The
|
||||
// hint can be wrong, for example when BYOK users of one provider send
|
||||
// different keys, so it never removes the fallback.
|
||||
// thinkingDropBlockModel makes Anthropic drop stale signed thinking blocks.
|
||||
// A hint for each provider and model picks whether to send drop_block first.
|
||||
// When hinted, only a drop_block rejection retries, so other errors are not
|
||||
// hidden behind a binding error.
|
||||
type thinkingDropBlockModel struct {
|
||||
fantasy.LanguageModel
|
||||
logger slog.Logger
|
||||
@@ -88,10 +101,8 @@ func (m *thinkingDropBlockModel) Stream(ctx context.Context, call fantasy.Call)
|
||||
retryable, updateHint := isThinkingBindingError, func() { m.hints.Store(m.key, struct{}{}) }
|
||||
_, hinted := m.hints.Load(m.key)
|
||||
if hinted {
|
||||
// The error a provider returns for an unsupported beta is
|
||||
// unknown, so any 400 retries without drop_block.
|
||||
first, fallback = fallback, first
|
||||
retryable, updateHint = isBadRequest, func() { m.hints.Delete(m.key) }
|
||||
retryable, updateHint = isDropBlockRejectedError, func() { m.hints.Delete(m.key) }
|
||||
}
|
||||
stream, err := m.LanguageModel.Stream(ctx, first)
|
||||
if err != nil {
|
||||
@@ -123,11 +134,6 @@ func (m *thinkingDropBlockModel) Stream(ctx context.Context, call fantasy.Call)
|
||||
}, nil
|
||||
}
|
||||
|
||||
func isBadRequest(err error) bool {
|
||||
var pe *fantasy.ProviderError
|
||||
return errors.As(err, &pe) && pe.StatusCode == http.StatusBadRequest
|
||||
}
|
||||
|
||||
// retryStream streams the fallback request and calls updateHint when the
|
||||
// fallback starts without an error.
|
||||
func (m *thinkingDropBlockModel) retryStream(ctx context.Context, fallback fantasy.Call, updateHint func(), yield func(fantasy.StreamPart) bool) {
|
||||
|
||||
@@ -2,6 +2,7 @@ package chatd
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"charm.land/fantasy"
|
||||
@@ -54,3 +55,79 @@ func TestThinkingDropBlockModel_RetriesWithEffort(t *testing.T) {
|
||||
require.JSONEq(t, `{"type":"adaptive","block_binding":{"prefix_mismatch_behavior":"drop_block"}}`, string(retry.Thinking))
|
||||
require.Equal(t, thinkingBindingBeta, retry.Header.Get("Anthropic-Beta"))
|
||||
}
|
||||
|
||||
func TestThinkingDropBlockModel_HintedRetriesOnlyDropBlockRejections(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
rejection string
|
||||
wantDropBlock []bool
|
||||
wantErr bool
|
||||
wantHint bool
|
||||
}{
|
||||
{
|
||||
name: "OtherError",
|
||||
rejection: "messages.9.content.43.image.source.base64.data: At least one of the image dimensions exceed max allowed size for many-image requests: 3000 pixels",
|
||||
wantDropBlock: []bool{true},
|
||||
wantErr: true,
|
||||
wantHint: true,
|
||||
},
|
||||
{
|
||||
name: "BetaRejected",
|
||||
rejection: "Unexpected value(s) `thinking-binding-controls-2026-08-01` for the `anthropic-beta` header. Please consult our documentation at platform.claude.com/docs or try again without the header.",
|
||||
wantDropBlock: []bool{true, false},
|
||||
},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ctx := testutil.Context(t, testutil.WaitShort)
|
||||
requests := make(chan chattest.AnthropicRequest, 2)
|
||||
url := chattest.NewAnthropic(t, func(req *chattest.AnthropicRequest) chattest.AnthropicResponse {
|
||||
requests <- *req
|
||||
if strings.Contains(string(req.Thinking), thinkingBindingDropBlock) {
|
||||
return chattest.AnthropicResponse{Error: &chattest.ErrorResponse{
|
||||
StatusCode: http.StatusBadRequest,
|
||||
Type: "invalid_request_error",
|
||||
Message: tc.rejection,
|
||||
}}
|
||||
}
|
||||
return chattest.AnthropicStreamingResponse(chattest.AnthropicTextChunks("ok")...)
|
||||
})
|
||||
client, err := fantasyanthropic.New(fantasyanthropic.WithAPIKey("test-key"), fantasyanthropic.WithBaseURL(url))
|
||||
require.NoError(t, err)
|
||||
lm, err := client.LanguageModel(ctx, "claude-opus-5-5")
|
||||
require.NoError(t, err)
|
||||
|
||||
server := &Server{logger: slogtest.Make(t, nil)}
|
||||
key := thinkingDropBlockKey{providerID: uuid.New(), model: "claude-opus-5-5"}
|
||||
server.thinkingDropBlock.Store(key, struct{}{})
|
||||
model := server.withThinkingDropBlock(chatprovider.NewModel(lm, nil), key.providerID, codersdk.ChatModelCallConfig{}, uuid.New())
|
||||
stream, err := model.LanguageModel().Stream(ctx, fantasy.Call{
|
||||
Prompt: fantasy.Prompt{fantasy.NewUserMessage("hello")},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
var streamErr error
|
||||
for part := range stream {
|
||||
if part.Type == fantasy.StreamPartTypeError {
|
||||
streamErr = part.Error
|
||||
}
|
||||
}
|
||||
|
||||
if tc.wantErr {
|
||||
require.ErrorContains(t, streamErr, tc.rejection)
|
||||
} else {
|
||||
require.NoError(t, streamErr)
|
||||
}
|
||||
var dropBlock []bool
|
||||
for len(requests) > 0 {
|
||||
req := <-requests
|
||||
dropBlock = append(dropBlock, strings.Contains(string(req.Thinking), thinkingBindingDropBlock))
|
||||
}
|
||||
require.Equal(t, tc.wantDropBlock, dropBlock)
|
||||
_, hinted := server.thinkingDropBlock.Load(key)
|
||||
require.Equal(t, tc.wantHint, hinted)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user