Skip to content

Anthropic Helicone String -> Object#5102

Merged
colegottdank merged 1 commit intomainfrom
string-to-object-anthropic
Oct 22, 2025
Merged

Anthropic Helicone String -> Object#5102
colegottdank merged 1 commit intomainfrom
string-to-object-anthropic

Conversation

@colegottdank
Copy link
Collaborator

No description provided.

@vercel
Copy link

vercel bot commented Oct 22, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
helicone Building Building Preview Comment Oct 22, 2025 9:51pm
helicone-eu Building Building Preview Comment Oct 22, 2025 9:51pm
1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
helicone-bifrost Skipped Skipped Oct 22, 2025 9:51pm

@claude
Copy link
Contributor

claude bot commented Oct 22, 2025

Claude finished @colegottdank's task —— View job


Code Review Complete

Reviewing PR: Anthropic Helicone String -> Object

Todo List:

  • Read repository guidelines (CLAUDE.md)
  • Examine the changed file: packages/cost/models/providers/helicone.ts
  • Analyze code changes for issues and improvements
  • Provide detailed review feedback with score
  • Post final review

Code Review Summary

Score: 8/10 - Good implementation with correct API format handling

Analysis

The change adds proper handling for Anthropic's system message format in the HeliconeProvider when bodyMapping === "NO_MAPPING". The code correctly converts string-based system messages to the object format required by Anthropic's API.

What Changed

// Before
return JSON.stringify({
  ...context.parsedBody,
  model: endpoint.providerModelId,
});

// After  
const body = { ...context.parsedBody };

// Ensure system message is in object format if it's a string
if (typeof body.system === "string") {
  body.system = [{ type: "text", text: body.system }];
}

return JSON.stringify({
  ...body,
  model: endpoint.providerModelId,
});

Positive Aspects

  1. Correct API Format: The transformation follows Anthropic's API specification where system can be either string | AnthropicContentBlock[]
  2. Defensive Programming: Only transforms when needed (string → object array)
  3. Consistent with Existing Code: Matches the format used in toAnthropic.ts:46-48 and extractSystemMessage() function
  4. Type Safety: Uses proper object structure with { type: "text", text: string }

Minor Improvements Suggested

  1. Type Safety Enhancement: Consider adding a type guard for better type safety:
if (typeof body.system === "string") {
  body.system = [{ type: "text", text: body.system }] as const;
}
  1. Documentation: Could benefit from a brief comment explaining why this transformation is needed:
// Anthropic API requires system messages as object array format
// when using structured content blocks
if (typeof body.system === "string") {
  body.system = [{ type: "text", text: body.system }];
}

Technical Correctness

API Compliance: Correctly implements Anthropic's Messages API format
No Side Effects: Uses spread operator to avoid mutation
Conditional Logic: Only applies transformation when necessary
Code Style: Follows existing patterns in the codebase

No Critical Issues Found

  • No security vulnerabilities detected
  • No performance concerns
  • No breaking changes introduced
  • Proper error handling context maintained

@colegottdank colegottdank merged commit 68b5d61 into main Oct 22, 2025
10 of 12 checks passed
@colegottdank colegottdank deleted the string-to-object-anthropic branch October 22, 2025 21:53
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Overview

Summary

Converts string-formatted Anthropic system messages to object format when using NO_MAPPING body mapping in the HeliconeProvider.

  • The change ensures system messages are always in array format [{type: "text", text: string}] instead of plain strings
  • Only applies when bodyMapping === "NO_MAPPING" for Anthropic models
  • The Anthropic API accepts both string and object array formats for the system field according to the types in packages/llm-mapper/transform/types/anthropic.ts:21

Confidence Score: 4/5

  • This PR is safe to merge with low risk
  • The change standardizes the system message format to always use the object array format. While both formats are valid according to Anthropic's types, this conversion is safe and maintains API compatibility. The implementation is straightforward with no side effects.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
packages/cost/models/providers/helicone.ts 4/5 Converts Anthropic system message from string to object format when using NO_MAPPING body mapping

Sequence Diagram

sequenceDiagram
    participant Client
    participant HeliconeProvider
    participant AnthropicAPI

    Client->>HeliconeProvider: buildRequestBody(endpoint, context)
    alt bodyMapping === "NO_MAPPING"
        HeliconeProvider->>HeliconeProvider: Copy parsedBody
        alt system is string
            HeliconeProvider->>HeliconeProvider: Convert system to [{type:"text", text:system}]
        end
        HeliconeProvider->>HeliconeProvider: Set model to providerModelId
        HeliconeProvider->>AnthropicAPI: Send request with object-formatted system
    else bodyMapping !== "NO_MAPPING"
        HeliconeProvider->>HeliconeProvider: Call toAnthropic(parsedBody)
        HeliconeProvider->>AnthropicAPI: Send transformed request
    end
    AnthropicAPI-->>Client: Response
Loading

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

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