Skip to content

fix: strip empty text blocks in retry filter#4

Closed
alfadb wants to merge 1 commit intomainfrom
fix/empty-text-block-retry
Closed

fix: strip empty text blocks in retry filter#4
alfadb wants to merge 1 commit intomainfrom
fix/empty-text-block-retry

Conversation

@alfadb
Copy link
Owner

@alfadb alfadb commented Mar 18, 2026

Summary

  • Fix isThinkingBlockSignatureError pattern matching to cover new Anthropic error format: "text content blocks must be non-empty" (old patterns only matched "non-empty content" / "empty content")
  • Add empty text block ({"type":"text","text":""}) stripping to FilterThinkingBlocksForRetry, with fast-path byte patterns to avoid unnecessary JSON parsing
  • When all content blocks in a message are stripped, a placeholder text block is inserted (existing behavior, now also applies to empty text blocks)

Context

Request 02f05f361f7336a01a4ce002c5763959 from claude-cli/2.1.78 hit upstream 400 because of an empty text content block. The platform had no pre-validation for this, and the retry path could not catch it due to:

  1. isThinkingBlockSignatureError pattern mismatch on the new error format
  2. FilterThinkingBlocksForRetry only handled empty content: [] arrays, not individual empty text blocks

Test plan

  • TestFilterThinkingBlocksForRetry_StripsEmptyTextBlocks — verifies empty text blocks are stripped, placeholder inserted when message becomes empty
  • TestFilterThinkingBlocksForRetry_PreservesNonEmptyTextBlocks — verifies fast path returns body unchanged
  • All existing TestFilterThinking* tests pass (no regression)
  • go build ./cmd/server/ succeeds

🤖 Generated with Claude Code

…tching

Empty text blocks ({"type":"text","text":""}) cause Anthropic upstream to
return 400: "text content blocks must be non-empty". This was not caught
by the existing error detection pattern in isThinkingBlockSignatureError,
nor handled by FilterThinkingBlocksForRetry.

- Add empty text block stripping to FilterThinkingBlocksForRetry
- Fix isThinkingBlockSignatureError to match new Anthropic error format
- Add fast-path byte patterns to avoid unnecessary JSON parsing

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 18, 2026 06:20
@alfadb alfadb closed this Mar 18, 2026
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves the gateway’s retry sanitization for Anthropic-style requests by recognizing a newer upstream error message and stripping invalid empty "text" content blocks to prevent 400s on retry.

Changes:

  • Broaden isThinkingBlockSignatureError matching to detect the newer "text content blocks must be non-empty" error phrasing.
  • Extend FilterThinkingBlocksForRetry to strip empty text blocks (with a byte-pattern fast-path to avoid JSON parsing when unnecessary).
  • Add tests covering empty text block stripping and verifying the fast-path preserves non-empty text requests unchanged.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
backend/internal/service/gateway_service.go Expands signature/empty-content error detection to cover new upstream wording.
backend/internal/service/gateway_request.go Adds empty-text-block detection and stripping to the retry filter with a fast-path heuristic.
backend/internal/service/gateway_request_test.go Adds regression tests for empty text block stripping and fast-path preservation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

if strings.Contains(msg, "non-empty content") || strings.Contains(msg, "empty content") {
// "messages: text content blocks must be non-empty"
if strings.Contains(msg, "non-empty content") || strings.Contains(msg, "empty content") ||
strings.Contains(msg, "must be non-empty") {
Comment on lines +247 to +250
// Check for empty text blocks: {"type":"text","text":""}
// These cause upstream 400: "text content blocks must be non-empty"
hasEmptyTextBlock := bytes.Contains(body, patternEmptyText) ||
bytes.Contains(body, patternEmptyTextSpaced)
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b8ada63ac3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +249 to +250
hasEmptyTextBlock := bytes.Contains(body, patternEmptyText) ||
bytes.Contains(body, patternEmptyTextSpaced)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Handle empty text blocks with all valid JSON spacing

The new empty-text fast path only checks "text":"" and "text": "", so payloads that are still valid JSON like "text" : "" are missed. In requests without thinking blocks or content: [], hasEmptyTextBlock stays false and the function returns early, which means the retry path resends the same invalid empty text block and still gets a 400. This makes the fix ineffective for pretty-printed clients that include spaces around the colon.

Useful? React with 👍 / 👎.

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