From 934ab7e485a0628a68c848005145dbf70d039621 Mon Sep 17 00:00:00 2001 From: viettranx Date: Sat, 11 Apr 2026 21:39:47 +0700 Subject: [PATCH] fix(media): route claude-cli PDFs as document blocks, scope disable_tools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- internal/providers/claude_cli_session.go | 15 ++- internal/providers/claude_cli_session_test.go | 125 ++++++++++++++++++ internal/tools/read_document.go | 9 +- internal/tools/read_document_resolve.go | 20 ++- internal/tools/read_image.go | 25 +++- 5 files changed, 178 insertions(+), 16 deletions(-) create mode 100644 internal/providers/claude_cli_session_test.go diff --git a/internal/providers/claude_cli_session.go b/internal/providers/claude_cli_session.go index c042d663..fdaaee0d 100644 --- a/internal/providers/claude_cli_session.go +++ b/internal/providers/claude_cli_session.go @@ -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, diff --git a/internal/providers/claude_cli_session_test.go b/internal/providers/claude_cli_session_test.go new file mode 100644 index 00000000..e4af199b --- /dev/null +++ b/internal/providers/claude_cli_session_test.go @@ -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)) + } +} diff --git a/internal/tools/read_document.go b/internal/tools/read_document.go index 9cac2a4d..9a7891f5 100644 --- a/internal/tools/read_document.go +++ b/internal/tools/read_document.go @@ -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", } diff --git a/internal/tools/read_document_resolve.go b/internal/tools/read_document_resolve.go index 8c899718..92150431 100644 --- a/internal/tools/read_document_resolve.go +++ b/internal/tools/read_document_resolve.go @@ -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) diff --git a/internal/tools/read_image.go b/internal/tools/read_image.go index b9caca1b..9d754d5c 100644 --- a/internal/tools/read_image.go +++ b/internal/tools/read_image.go @@ -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)