mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-14 08:18:19 +00:00
fix(media): route claude-cli PDFs as document blocks, scope disable_tools
read_image and read_document failed when only claude-cli was configured because the fallback chains only listed 'anthropic'. Adding claude-cli as fallback (PR #802) surfaced a deeper bug: buildStreamJSONInput hardcoded 'type: image' for every content block, so PDFs routed through claude-cli were rejected by the Anthropic API with a MIME mismatch. - providers/claude_cli_session.go: buildStreamJSONInput now picks the Anthropic block type from MIME — application/pdf -> document, image/* -> image. - tools/read_image.go, read_document.go: add claude-cli to the fallback priority and model defaults (empty string lets the provider pick its own default model). - tools/read_image.go, read_document_resolve.go: scope disable_tools to claude-cli only instead of leaking a CLI-specific flag into the shared Options map every provider in the chain receives. - providers/claude_cli_session_test.go: table test covering png/pdf/ mixed/unknown MIME routing plus the empty-text edge case. Closes #801.
This commit is contained in:
1 parent
abc6d62c95
commit
934ab7e485
5 files changed
+178
-16
No files matched your search
@@ -206,13 +206,24 @@ func sessionFileExists(workDir string, sessionID uuid.UUID) bool {
|
||||
return err == nil
|
||||
}
|
||||
|
||||
// buildStreamJSONInput creates stream-json stdin for vision (images + text).
|
||||
// buildStreamJSONInput creates stream-json stdin for multimodal input
|
||||
// (images/documents + text). The content block type is chosen from MIME:
|
||||
// - application/pdf → "document" (Anthropic PDF support, Claude 3.5+)
|
||||
// - image/* → "image"
|
||||
//
|
||||
// MIME types that fit neither are emitted as "image" for backwards
|
||||
// compatibility; the upstream Anthropic API will reject them explicitly
|
||||
// rather than silently misroute.
|
||||
func buildStreamJSONInput(text string, images []ImageContent) *bytes.Reader {
|
||||
var contentBlocks []map[string]any
|
||||
|
||||
for _, img := range images {
|
||||
blockType := "image"
|
||||
if img.MimeType == "application/pdf" {
|
||||
blockType = "document"
|
||||
}
|
||||
contentBlocks = append(contentBlocks, map[string]any{
|
||||
"type": "image",
|
||||
"type": blockType,
|
||||
"source": map[string]any{
|
||||
"type": "base64",
|
||||
"media_type": img.MimeType,
|
||||
|
||||
@@ -0,0 +1,125 @@
|
||||
package providers
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"io"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// TestBuildStreamJSONInput_MimeRouting verifies that buildStreamJSONInput
|
||||
// picks the correct Anthropic content block type based on MIME:
|
||||
// - application/pdf → "document"
|
||||
// - image/* → "image"
|
||||
//
|
||||
// Regression guard: earlier versions hardcoded "image" for every block,
|
||||
// causing PDF passthrough to fail because the Anthropic API rejects
|
||||
// image blocks with non-image MIME types.
|
||||
func TestBuildStreamJSONInput_MimeRouting(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
images []ImageContent
|
||||
wantTypes []string
|
||||
}{
|
||||
{
|
||||
name: "png image → image block",
|
||||
images: []ImageContent{
|
||||
{MimeType: "image/png", Data: "abc"},
|
||||
},
|
||||
wantTypes: []string{"image"},
|
||||
},
|
||||
{
|
||||
name: "pdf → document block",
|
||||
images: []ImageContent{
|
||||
{MimeType: "application/pdf", Data: "abc"},
|
||||
},
|
||||
wantTypes: []string{"document"},
|
||||
},
|
||||
{
|
||||
name: "mixed png + pdf → image then document",
|
||||
images: []ImageContent{
|
||||
{MimeType: "image/jpeg", Data: "xxx"},
|
||||
{MimeType: "application/pdf", Data: "yyy"},
|
||||
},
|
||||
wantTypes: []string{"image", "document"},
|
||||
},
|
||||
{
|
||||
name: "unknown MIME falls back to image",
|
||||
images: []ImageContent{
|
||||
{MimeType: "application/octet-stream", Data: "zzz"},
|
||||
},
|
||||
wantTypes: []string{"image"},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
r := buildStreamJSONInput("describe", tc.images)
|
||||
raw, err := io.ReadAll(r)
|
||||
if err != nil {
|
||||
t.Fatalf("read stdin: %v", err)
|
||||
}
|
||||
|
||||
var msg struct {
|
||||
Message struct {
|
||||
Content []map[string]any `json:"content"`
|
||||
} `json:"message"`
|
||||
}
|
||||
if err := json.Unmarshal(raw, &msg); err != nil {
|
||||
t.Fatalf("parse stream-json: %v\nraw: %s", err, raw)
|
||||
}
|
||||
|
||||
// Expect N media blocks + 1 trailing text block.
|
||||
wantLen := len(tc.wantTypes) + 1
|
||||
if got := len(msg.Message.Content); got != wantLen {
|
||||
t.Fatalf("content blocks = %d, want %d\nraw: %s", got, wantLen, raw)
|
||||
}
|
||||
|
||||
for i, wantType := range tc.wantTypes {
|
||||
gotType, _ := msg.Message.Content[i]["type"].(string)
|
||||
if gotType != wantType {
|
||||
t.Errorf("block[%d].type = %q, want %q", i, gotType, wantType)
|
||||
}
|
||||
source, _ := msg.Message.Content[i]["source"].(map[string]any)
|
||||
if source == nil {
|
||||
t.Errorf("block[%d].source is nil", i)
|
||||
continue
|
||||
}
|
||||
if gotMime, _ := source["media_type"].(string); gotMime != tc.images[i].MimeType {
|
||||
t.Errorf("block[%d].source.media_type = %q, want %q", i, gotMime, tc.images[i].MimeType)
|
||||
}
|
||||
}
|
||||
|
||||
// Trailing text block.
|
||||
last := msg.Message.Content[wantLen-1]
|
||||
if last["type"] != "text" {
|
||||
t.Errorf("trailing block type = %v, want text", last["type"])
|
||||
}
|
||||
if last["text"] != "describe" {
|
||||
t.Errorf("trailing block text = %v, want describe", last["text"])
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestBuildStreamJSONInput_NoText covers the edge case where the caller
|
||||
// passes images with an empty prompt — no text block should be emitted.
|
||||
func TestBuildStreamJSONInput_NoText(t *testing.T) {
|
||||
r := buildStreamJSONInput("", []ImageContent{
|
||||
{MimeType: "image/png", Data: "abc"},
|
||||
})
|
||||
raw, err := io.ReadAll(r)
|
||||
if err != nil {
|
||||
t.Fatalf("read stdin: %v", err)
|
||||
}
|
||||
var msg struct {
|
||||
Message struct {
|
||||
Content []map[string]any `json:"content"`
|
||||
} `json:"message"`
|
||||
}
|
||||
if err := json.Unmarshal(raw, &msg); err != nil {
|
||||
t.Fatalf("parse stream-json: %v", err)
|
||||
}
|
||||
if len(msg.Message.Content) != 1 {
|
||||
t.Errorf("content blocks = %d, want 1 (image only)", len(msg.Message.Content))
|
||||
}
|
||||
}
|
||||
@@ -49,14 +49,17 @@ func MediaDocRefsFromCtx(ctx context.Context) []providers.MediaRef {
|
||||
const documentMaxBytes = 20 * 1024 * 1024
|
||||
|
||||
// documentProviderPriority is the order in which providers are tried for document analysis.
|
||||
// Gemini has best native PDF support (50MB, 258 tokens/page).
|
||||
// "alibaba" is included as an alias for dashscope (common DB registration name).
|
||||
var documentProviderPriority = []string{"gemini", "anthropic", "openrouter", "dashscope"}
|
||||
// Gemini has best native PDF support (50MB, 258 tokens/page). claude-cli is
|
||||
// included so installations with only Claude CLI configured can still analyze
|
||||
// PDFs via the CLI bridge (document content block in stream-json).
|
||||
var documentProviderPriority = []string{"gemini", "anthropic", "claude-cli", "openrouter", "dashscope"}
|
||||
|
||||
// documentModelDefaults maps provider names to preferred document-capable models.
|
||||
// Empty string lets the provider pick its own default model.
|
||||
var documentModelDefaults = map[string]string{
|
||||
"gemini": "gemini-2.5-flash",
|
||||
"openrouter": "google/gemini-2.5-flash",
|
||||
"claude-cli": "",
|
||||
"dashscope": "qwen-vl-max",
|
||||
}
|
||||
|
||||
|
||||
@@ -85,6 +85,19 @@ func (t *ReadDocumentTool) callProvider(ctx context.Context, cp credentialProvid
|
||||
}
|
||||
|
||||
slog.Info("read_document: using chat API", "provider", providerName, "model", model, "doc_size", len(data))
|
||||
|
||||
opts := map[string]any{
|
||||
"max_tokens": 16384,
|
||||
"temperature": 0.2,
|
||||
}
|
||||
// Scope disable_tools to claude-cli only — it's a CLI-bridge-specific
|
||||
// option that skips loading the built-in MCP toolset for one-shot calls.
|
||||
// Other providers silently ignore unknown keys today, but leaking
|
||||
// provider-specific flags into the shared Options map couples layers.
|
||||
if providerName == "claude-cli" {
|
||||
opts["disable_tools"] = true
|
||||
}
|
||||
|
||||
resp, err := p.Chat(ctx, providers.ChatRequest{
|
||||
Messages: []providers.Message{
|
||||
{
|
||||
@@ -93,11 +106,8 @@ func (t *ReadDocumentTool) callProvider(ctx context.Context, cp credentialProvid
|
||||
Images: []providers.ImageContent{{MimeType: mime, Data: base64.StdEncoding.EncodeToString(data)}},
|
||||
},
|
||||
},
|
||||
Model: model,
|
||||
Options: map[string]any{
|
||||
"max_tokens": 16384,
|
||||
"temperature": 0.2,
|
||||
},
|
||||
Model: model,
|
||||
Options: opts,
|
||||
})
|
||||
if err != nil {
|
||||
return nil, nil, fmt.Errorf("chat call: %w", err)
|
||||
|
||||
@@ -30,13 +30,17 @@ func MediaImagesFromCtx(ctx context.Context) []providers.ImageContent {
|
||||
// --- ReadImageTool ---
|
||||
|
||||
// visionProviderPriority is the order in which providers are tried for vision.
|
||||
var visionProviderPriority = []string{"openrouter", "gemini", "anthropic", "dashscope"}
|
||||
// claude-cli follows anthropic so installations with a native Anthropic API key
|
||||
// keep using the faster direct API, while claude-cli-only setups still resolve.
|
||||
var visionProviderPriority = []string{"openrouter", "gemini", "anthropic", "claude-cli", "dashscope"}
|
||||
|
||||
// visionModelDefaults maps provider names to preferred vision models.
|
||||
// Empty string lets the provider pick its own default model.
|
||||
var visionModelDefaults = map[string]string{
|
||||
"openrouter": "google/gemini-2.5-flash-image",
|
||||
"gemini": "gemini-2.5-flash",
|
||||
"anthropic": "",
|
||||
"claude-cli": "",
|
||||
"dashscope": "qwen3-vl",
|
||||
}
|
||||
|
||||
@@ -136,6 +140,18 @@ func (t *ReadImageTool) callProvider(ctx context.Context, cp credentialProvider,
|
||||
|
||||
slog.Info("read_image: calling vision provider", "provider", providerName, "model", model, "images", len(images))
|
||||
|
||||
opts := map[string]any{
|
||||
"max_tokens": 1024,
|
||||
"temperature": 0.3,
|
||||
}
|
||||
// claude-cli spawns the Claude CLI binary; loading its built-in MCP
|
||||
// toolset costs latency we don't need for a one-shot vision call. Keep
|
||||
// this flag scoped to claude-cli so other providers don't receive
|
||||
// options they ignore (or worse, choke on in the future).
|
||||
if providerName == "claude-cli" {
|
||||
opts["disable_tools"] = true
|
||||
}
|
||||
|
||||
resp, err := p.Chat(ctx, providers.ChatRequest{
|
||||
Messages: []providers.Message{
|
||||
{
|
||||
@@ -144,11 +160,8 @@ func (t *ReadImageTool) callProvider(ctx context.Context, cp credentialProvider,
|
||||
Images: images,
|
||||
},
|
||||
},
|
||||
Model: model,
|
||||
Options: map[string]any{
|
||||
"max_tokens": 1024,
|
||||
"temperature": 0.3,
|
||||
},
|
||||
Model: model,
|
||||
Options: opts,
|
||||
})
|
||||
if err != nil {
|
||||
return nil, nil, fmt.Errorf("vision provider error: %w", err)
|
||||
|
||||
Reference in new issue
Block a user