Skip to content

unmarshal anthropic msg content. abort if err unmarshal#27

Merged
sergei-bronnikov merged 2 commits into
mainfrom
17842_antropic_fixes
Jun 30, 2026
Merged

unmarshal anthropic msg content. abort if err unmarshal#27
sergei-bronnikov merged 2 commits into
mainfrom
17842_antropic_fixes

Conversation

@sergei-bronnikov

@sergei-bronnikov sergei-bronnikov commented Jun 30, 2026

Copy link
Copy Markdown

https://bugtracker.codiodev.com/issue/codio-17842/Added-anthropic-key-doesnt-show-the-actual-amount-of-money

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of AI message content so both plain text and structured content are processed correctly.
    • Token counting now uses the normalized message content, keeping usage estimates accurate.
    • Request parsing errors now return a proper JSON error response instead of failing silently or only logging.
    • Body read failures now stop the request immediately with a server error response.

@sergei-bronnikov

Copy link
Copy Markdown
Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

Adds a FlexContent type to the Anthropic provider that deserializes message content as either a JSON string or array. Updates policy filtering and cost token counting to call .String() on this type. Expands middleware error handling to return 500 JSON responses and abort on body-read or JSON unmarshal failures across all provider endpoints.

Changes

Anthropic FlexContent and middleware error handling

Layer / File(s) Summary
FlexContent type definition
internal/provider/anthropic/anthropic.go
Adds FlexContent struct with custom UnmarshalJSON accepting JSON string or array, a String() method, and changes Message.Content from string to FlexContent. Adds encoding/json and fmt imports.
Policy filter and cost estimator updated for FlexContent
internal/policy/policy.go, internal/provider/anthropic/cost.go
Policy Filter uses Content.String() when extracting text for scanning and wraps rebuilt text in FlexContent{Text: c}. CostEstimator.CountMessagesTokens passes Content.String() to the token counter.
Middleware 500 responses on deserialization failures
internal/server/web/proxy/middleware.go
getMiddleware now calls c.JSON(500, ...) and c.Abort() on io.ReadAll failure and on json.Unmarshal failures for all provider-specific endpoints (Anthropic, Bedrock, VLLM, DeepInfra, Azure OpenAI, OpenAI).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: Anthropic message content unmarshalling and aborting on unmarshal errors.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 17842_antropic_fixes

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
internal/policy/policy.go (1)

439-465: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Preserve Anthropic block structure when rewriting filtered messages.

This branch flattens content to one string and then always writes it back as FlexContent{Text: c}. Requests that originally contained block arrays lose non-text content and text-block boundaries, so policy filtering can corrupt otherwise valid Anthropic messages.

🤖 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 `@internal/policy/policy.go` around lines 439 - 465, The Anthropic rewrite path
in policy.go is flattening message content into plain text and then rebuilding
each message as FlexContent{Text: c}, which destroys the original block
structure. Update the filtering flow around p.scan and the result.Updated
rebuild loop so it preserves the original anthropic.Message content shape,
especially block arrays and text-block boundaries, instead of always converting
to a single text block. Use the existing converted.Messages entries as the
source of truth when reconstructing each message.
🧹 Nitpick comments (1)
internal/server/web/proxy/middleware.go (1)

396-397: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract an aborting JSON error helper.

This same JSON(...); c.Abort(); return block is now copied across many endpoints. A small helper like abortJSON(c, code, msg) would keep status/message handling consistent and make follow-up fixes much safer.

Also applies to: 420-421, 447-448, 474-475, 499-500, 637-638, 660-661, 683-684, 705-706, 727-728, 746-747, 769-770, 792-793, 818-819, 843-844, 909-910, 929-930, 950-951, 980-981, 1009-1010

🤖 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 `@internal/server/web/proxy/middleware.go` around lines 396 - 397, The repeated
JSON error response plus abort pattern should be centralized to avoid copy-paste
across these handlers. Add a helper such as abortJSON(c, code, msg) and update
the affected middleware paths in middleware.go to call it instead of duplicating
JSON(c, ...); c.Abort(); return. Keep the helper responsible for both writing
the response and aborting the context so status/message handling stays
consistent in all endpoints.
🤖 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 `@internal/provider/anthropic/anthropic.go`:
- Around line 44-53: The FlexContent.String method is using fmt.Sprintf on Raw
block arrays, which produces debug-style output instead of the real prompt text.
Update String to render Anthropic content arrays by extracting and concatenating
the underlying text from the block elements rather than formatting the slice/map
representation. Keep the existing nil and Text handling, and adjust the Raw
branch in FlexContent.String so policy scans and token counts see the actual
content.

In `@internal/server/web/proxy/middleware.go`:
- Around line 420-421: The request parsing branches in middleware.go are
returning http.StatusInternalServerError for invalid client JSON, which should
instead be a 400-class response. Update the JSON(c, ...) calls in the completion
request unmarshalling paths to use http.StatusBadRequest for these parse
failures, including the anthropic completion handler and the other listed
request decode branches in the same middleware logic. Keep the existing error
messages and Abort behavior, but ensure all JSON parse failures consistently map
to client errors.

---

Outside diff comments:
In `@internal/policy/policy.go`:
- Around line 439-465: The Anthropic rewrite path in policy.go is flattening
message content into plain text and then rebuilding each message as
FlexContent{Text: c}, which destroys the original block structure. Update the
filtering flow around p.scan and the result.Updated rebuild loop so it preserves
the original anthropic.Message content shape, especially block arrays and
text-block boundaries, instead of always converting to a single text block. Use
the existing converted.Messages entries as the source of truth when
reconstructing each message.

---

Nitpick comments:
In `@internal/server/web/proxy/middleware.go`:
- Around line 396-397: The repeated JSON error response plus abort pattern
should be centralized to avoid copy-paste across these handlers. Add a helper
such as abortJSON(c, code, msg) and update the affected middleware paths in
middleware.go to call it instead of duplicating JSON(c, ...); c.Abort(); return.
Keep the helper responsible for both writing the response and aborting the
context so status/message handling stays consistent in all endpoints.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 30e6e09a-3503-4834-8a42-b9093a44b1e6

📥 Commits

Reviewing files that changed from the base of the PR and between 0ddb013 and a99e0fb.

📒 Files selected for processing (4)
  • internal/policy/policy.go
  • internal/provider/anthropic/anthropic.go
  • internal/provider/anthropic/cost.go
  • internal/server/web/proxy/middleware.go

Comment thread internal/provider/anthropic/anthropic.go
Comment thread internal/server/web/proxy/middleware.go Outdated
@sergei-bronnikov sergei-bronnikov merged commit f5b6ab1 into main Jun 30, 2026
2 checks passed
@sergei-bronnikov sergei-bronnikov deleted the 17842_antropic_fixes branch June 30, 2026 15:07
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