required text for document calls fix - #5675
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughAnthropic chat request conversion inserts non-whitespace placeholder text for document-only or whitespace-only content, preserves usable text, and places injected text after leading thinking blocks. Regression tests cover user messages and assistant prefills. ChangesAnthropic document validation
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/providers/anthropic/chat.go`:
- Around line 866-870: Update the assistant content normalization around the
hasDocument/hasText handling and the trimming logic near lines 891-903 so
document-only messages retain a single-space text placeholder after trimming.
Re-run the document/text validation after trimming or restore the placeholder
when documents are present, and add a regression test covering an assistant
prefill with a document-only final message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 68ec2337-5a8a-4208-959d-7212042546d2
📒 Files selected for processing (2)
core/providers/anthropic/chat.gocore/providers/anthropic/chat_test.go
38d82ad to
a4a4955
Compare
|
|
32db51f to
e27f1d6
Compare
a4a4955 to
8f40c80
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/providers/anthropic/chat.go`:
- Around line 874-892: The document-content validation around hasDocument and
hasText must treat whitespace-only text blocks as missing usable text. Use
strings.TrimSpace on text block content before setting hasText, while preserving
non-whitespace text as valid; add a regression test covering a non-final user
document message containing only whitespace text.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 7948cfc9-a7ce-4a2e-9524-146727f1731f
📒 Files selected for processing (2)
core/providers/anthropic/chat.gocore/providers/anthropic/chat_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- core/providers/anthropic/chat_test.go
e27f1d6 to
ab0f06b
Compare
8f40c80 to
51dd24a
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
core/providers/anthropic/chat_test.go (1)
176-180: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert non-whitespace, not merely non-empty.
*blocks[0].Text != ""would accept" ", even though the production contract requires non-whitespace text. Usestrings.TrimSpaceso this regression test catches a whitespace-only placeholder.Suggested test assertion
- if blocks[0].Text == nil || *blocks[0].Text == "" { + if blocks[0].Text == nil || strings.TrimSpace(*blocks[0].Text) == "" {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/providers/anthropic/chat_test.go` around lines 176 - 180, Update the placeholder text assertion in the test around blocks[0].Text to trim surrounding whitespace before checking for emptiness, while preserving the existing nil check and failure message. Ensure whitespace-only placeholder text is rejected.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@core/providers/anthropic/chat_test.go`:
- Around line 176-180: Update the placeholder text assertion in the test around
blocks[0].Text to trim surrounding whitespace before checking for emptiness,
while preserving the existing nil check and failure message. Ensure
whitespace-only placeholder text is rejected.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 12f201a7-8848-4d26-8029-88c01c8f4042
📒 Files selected for processing (2)
core/providers/anthropic/chat.gocore/providers/anthropic/chat_test.go
ab0f06b to
89e564a
Compare
51dd24a to
329e8db
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
core/providers/anthropic/chat.go (1)
874-892: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCount usable text, not merely text blocks.
Lines 882-883 treat
" "and"\t"as valid text. A user or non-final assistant message containing a document plus whitespace-only text therefore skips placeholder insertion and still sends invalid content. Remove/replace whitespace-only text blocks, then insert the placeholder only when no non-whitespace text remains. This repeats the previously reported issue, which is still present.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/providers/anthropic/chat.go` around lines 874 - 892, Update the content scan around AnthropicContentBlockTypeDocument and AnthropicContentBlockTypeText to consider text usable only when its contents are non-whitespace. Remove whitespace-only text blocks before validation, then prepend documentPlaceholderText when a document has no remaining non-whitespace text; preserve existing text blocks that contain meaningful content.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/providers/anthropic/chat_test.go`:
- Around line 179-180: Update the placeholder validation in the test assertion
around blocks[0].Text to trim whitespace before checking emptiness, using
strings.TrimSpace. Preserve the nil check and ensure whitespace-only placeholder
text fails the assertion.
In `@core/providers/anthropic/chat.go`:
- Around line 919-927: The placeholder restoration in the trimming logic around
hasAnthropicDocumentBlock must only occur when no other text block contains
non-whitespace text. Preserve existing usable text unchanged; for
whitespace-only blocks alongside documents when usable text exists, remove or
otherwise omit the invalid block instead of replacing it with
documentPlaceholderText.
---
Duplicate comments:
In `@core/providers/anthropic/chat.go`:
- Around line 874-892: Update the content scan around
AnthropicContentBlockTypeDocument and AnthropicContentBlockTypeText to consider
text usable only when its contents are non-whitespace. Remove whitespace-only
text blocks before validation, then prepend documentPlaceholderText when a
document has no remaining non-whitespace text; preserve existing text blocks
that contain meaningful content.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 560dc6ed-aafe-411c-baae-872de1f63d5b
📒 Files selected for processing (2)
core/providers/anthropic/chat.gocore/providers/anthropic/chat_test.go
89e564a to
4a10a92
Compare
ff97091 to
a05ac19
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/providers/anthropic/chat.go`:
- Around line 874-898: Update the placeholder insertion logic in the Anthropic
content normalization block to preserve leading thinking and redacted_thinking
blocks, inserting the documentPlaceholderText block immediately after them
rather than always prepending at index 0. Add a regression test covering an
assistant prefill containing ReasoningDetails and a document block, verifying
reasoning blocks remain first and the placeholder follows them.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8a7d9109-9540-4736-9213-11f95f5351fa
📒 Files selected for processing (2)
core/providers/anthropic/chat.gocore/providers/anthropic/chat_test.go
4a10a92 to
69305f7
Compare
e64abde to
0290b4f
Compare
0290b4f to
194b987
Compare
69305f7 to
78d8e96
Compare
Merge activity
|
The base branch was changed.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
core/providers/anthropic/chat.go (1)
14-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate the placeholder comment to match its insertion behavior.
The comment says the placeholder is “prepended,” but the implementation inserts it after leading reasoning blocks. Describe it as “inserted” and mention the ordering constraint to avoid future regressions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/providers/anthropic/chat.go` around lines 14 - 21, Update the comment for documentPlaceholderText to describe the placeholder as being inserted rather than prepended, and state that it is inserted after leading reasoning blocks to preserve their ordering.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@core/providers/anthropic/chat.go`:
- Around line 14-21: Update the comment for documentPlaceholderText to describe
the placeholder as being inserted rather than prepended, and state that it is
inserted after leading reasoning blocks to preserve their ordering.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9afab188-c460-4b61-a097-ce8985002e82
📒 Files selected for processing (2)
core/providers/anthropic/chat.gocore/providers/anthropic/chat_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- core/providers/anthropic/chat_test.go

Summary
Anthropic rejects messages that contain a document block without an accompanying text block, returning a validation error: "A text block must be included when using documents." This PR fixes that by automatically prepending a single-space placeholder text block whenever a message contains one or more document blocks but no text block.
Changes
" ") is prepended so the request passes Anthropic's validation.Type of change
Affected areas
How to test
go test ./core/providers/anthropic/...Expected: all tests pass, including the two new tests:
TestToAnthropicChatRequest_DocumentOnlyMessageGetsPlaceholderTextBlockTestToAnthropicChatRequest_DocumentWithTextDoesNotGetPlaceholderScreenshots/Recordings
N/A
Breaking changes
Related issues
N/A
Security considerations
None. The placeholder text block is a single space and does not expose any sensitive data.
Checklist
docs/contributing/README.mdand followed the guidelines