Skip to content

Conversation

@connortbot
Copy link
Collaborator

No description provided.

connortbot and others added 4 commits November 19, 2025 14:33
* add mappings for google and vertex + update usageprocessors

* revert openAIUsageProcessor

* revert costCalc.ts changes

* revert responseBodyHandler.ts

* revert some unecessary changes + console logs

* file reversion

* send apiKey as requestParams

* Update packages/llm-mapper/transform/providers/google/response/toOpenai.ts

* explanation comment

* fix greptile mistake

* fix tsc error

* add missing semicolon

--------
@connortbot connortbot requested a review from chitalian November 25, 2025 19:24
@vercel
Copy link

vercel bot commented Nov 25, 2025

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

Project Deployment Preview Comments Updated (UTC)
helicone Ready Ready Preview Comment Dec 2, 2025 11:13pm
helicone-bifrost Ready Ready Preview Comment Dec 2, 2025 11:13pm
helicone-eu Ready Ready Preview Comment Dec 2, 2025 11:13pm

@claude
Copy link
Contributor

claude bot commented Nov 25, 2025

Claude finished @connortbot's task —— View job


PR Review: Native Gemini API Mappers

Todo List:

  • Read and analyze key implementation files
  • Review the flagged parseArguments function
  • Check security considerations
  • Evaluate performance implications
  • Assess code quality and best practices
  • Provide comprehensive feedback and score

Score: 7/10 - Good implementation with minor suggestions for improvement

Critical Findings

🔍 Primitive Tool Response Handling (Medium Priority)

  • Issue: In packages/llm-mapper/transform/providers/openai/request/toGoogle.ts:437-438, primitive tool responses are wrapped as { result: value }
  • Concern: Greptile flagged this - need to verify Google API expects this specific field name
  • Recommendation: Verify with Google Gemini API documentation that result is the correct field name for primitive values in functionResponse.response

🔒 Security Considerations (Low-Medium Priority)

  • Issue: API key in URL query parameter (packages/cost/models/providers/google.ts:20,23)
  • Impact: While this follows Google's API pattern, query params can appear in logs
  • Recommendation: Document this behavior and ensure log sanitization for URLs containing API keys

Code Quality & Performance

✅ Strengths:

  • Well-structured bidirectional mapping with clear separation of concerns
  • Comprehensive error handling in streaming implementation
  • Robust token reconciliation logic in VertexOpenAIUsageProcessor
  • Extensive test coverage including edge cases
  • Proper modality-specific token tracking (text, audio, image, video)

🔧 Minor Improvements:

  1. Magic String Usage (packages/llm-mapper/transform/providers/openai/request/toGoogle.ts:443)

    // Current
    return { raw: value };
    // Suggested: Use constant
    const UNPARSEABLE_CONTENT_KEY = 'raw';
    return { [UNPARSEABLE_CONTENT_KEY]: value };
  2. Error Handling Enhancement (packages/llm-mapper/transform/providers/google/streamedResponse/toOpenai.ts:44)

    // Current
    console.error("Failed to parse Gemini SSE data:", error);
    // Suggested: More context
    console.error("Failed to parse Gemini SSE data:", { 
      error: error.message, 
      jsonStr: jsonStr.substring(0, 100) 
    });
  3. Type Safety (packages/llm-mapper/transform/providers/openai/request/toGoogle.ts:225-226)

    // Current type guard could be more specific
    const isFunctionTool = (tool: ChatCompletionTool): tool is FunctionTool =>
      tool.type === "function" && Boolean((tool as any).function);
    // Suggested: Check function properties exist
    tool.type === "function" && 
    Boolean((tool as any).function?.name);

Performance & Architecture

👍 Well Optimized:

  • Efficient streaming implementation using ReadableStream API
  • Smart token reconciliation prevents cost attribution errors
  • Minimal object creation in hot paths
  • Proper buffer management in stream processing

📈 Minor Optimizations:

  • Consider memoizing URL construction in GoogleProvider.buildUrl() for repeated calls
  • The nested loops in VertexOpenAIUsageProcessor.extractUsageFromResponse() could use early returns to reduce nesting

Best Practices Assessment

✅ Follows Patterns:

  • Extends existing processor classes appropriately
  • Consistent error handling patterns
  • Proper TypeScript usage with defined interfaces
  • Clean separation of request/response transformation logic

🔄 Refactoring Opportunities:

  • parseArguments function handles multiple formats - consider splitting into separate functions
  • Large extractUsageFromResponse method could be broken into smaller utilities

Suggestions Summary:

  • Critical: Verify Google API expects { result: primitive } wrapper format
  • Security: Document API key in URL implications and ensure log sanitization
  • Quality: Add constants for magic strings, improve error context, enhance type guards
  • Performance: Consider memoization for URL building, reduce nesting in token processing
  • Refactoring: Split complex functions, extract token calculation utilities

The implementation is solid with comprehensive coverage. Main concern is verifying the primitive tool response format matches Google's expectations. All other suggestions are minor improvements.


@greptile-apps
Copy link
Contributor

greptile-apps bot commented Nov 25, 2025

Greptile Overview

Greptile Summary

Adds native Google Gemini API support to the AI Gateway, enabling OpenAI-to-Gemini request/response transformation for both streaming and non-streaming modes.

Key Changes:

  • Implemented bidirectional OpenAI ↔ Gemini format mappers in packages/llm-mapper
  • Added GoogleProvider with URL-based authentication and native request transformation
  • Created GoogleUsageProcessor extending Vertex processor for shared response format
  • Enhanced VertexOpenAIUsageProcessor with modality-specific token tracking (text, audio, image, video)
  • Integrated transformers in worker for both streaming (goog2oaiStream) and non-streaming (goog2oaiResponse) responses
  • Added comprehensive test coverage for Gemini format normalization and usage processing

Implementation Quality:

  • Well-structured mapper functions with clear separation of concerns
  • Proper handling of tool calls, function responses, and system messages
  • Robust streaming implementation using ReadableStream API
  • Comprehensive test coverage including edge cases

Confidence Score: 4/5

  • This PR is safe to merge with one verification needed for primitive tool response handling
  • Score reflects solid implementation with comprehensive test coverage. The only concern is the parseArguments function's primitive wrapping logic which needs verification that it matches Google's API requirements. All other code is well-structured with proper error handling.
  • Verify packages/llm-mapper/transform/providers/openai/request/toGoogle.ts parseArguments function behavior with Google API documentation

Important Files Changed

File Analysis

Filename Score Overview
packages/llm-mapper/transform/providers/openai/request/toGoogle.ts 4/5 Implements OpenAI to Google Gemini request transformation with comprehensive mapping of messages, tools, and generation config
packages/llm-mapper/transform/providers/google/response/toOpenai.ts 5/5 Converts Google response format to OpenAI format with proper usage metadata mapping including audio/video tokens
packages/llm-mapper/transform/providers/google/streamedResponse/toOpenai.ts 5/5 Handles streaming conversion from Google SSE format to OpenAI chat completion chunks
packages/cost/usage/vertexUsageProcessor.ts 4/5 Processes Google/Vertex usage metadata with modality-specific token tracking and token reconciliation logic
packages/cost/models/providers/google.ts 5/5 Google AI Studio provider implementation with URL-based auth and native request body transformation
worker/src/lib/clients/llmmapper/router/oai2google/stream.ts 5/5 Transforms Google streaming responses to OpenAI SSE format using ReadableStream API

Sequence Diagram

sequenceDiagram
    participant Client
    participant Worker
    participant AIGateway
    participant GoogleProvider
    participant GoogleAPI
    participant UsageProcessor

    Client->>Worker: OpenAI format request
    Worker->>AIGateway: Route request
    AIGateway->>GoogleProvider: buildRequestBody()
    GoogleProvider->>GoogleProvider: toGoogle() transform
    Note over GoogleProvider: Maps messages, tools,<br/>generation config
    GoogleProvider->>GoogleAPI: Native Gemini API request
    
    alt Streaming
        GoogleAPI-->>Worker: SSE stream (Google format)
        Worker->>Worker: goog2oaiStream()
        Note over Worker: GoogleToOpenAIStreamConverter<br/>processes chunks
        Worker-->>Client: OpenAI SSE stream
    else Non-streaming
        GoogleAPI-->>Worker: JSON response (Google format)
        Worker->>Worker: goog2oaiResponse()
        Note over Worker: toOpenAI() conversion
        Worker-->>Client: OpenAI format response
    end

    Worker->>UsageProcessor: Process usage metadata
    Note over UsageProcessor: GoogleUsageProcessor extends<br/>VertexOpenAIUsageProcessor
    UsageProcessor->>UsageProcessor: Extract modality tokens<br/>(text, audio, image, video)
    UsageProcessor-->>Worker: Normalized usage data
Loading

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.

38 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment on lines +410 to +414
// If the parsed value is a primitive (number, string, boolean, null), wrap it
// Google's API requires functionResponse.response to be an object (Struct)
if (typeof parsed !== 'object' || parsed === null) {
return { result: parsed };
}
Copy link
Contributor

Choose a reason for hiding this comment

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

style: verify that wrapping primitives in { result: value } matches Google's API expectations for functionResponse.response

the comment states Google requires an object (Struct), but confirm this is the correct field name (result) and that Google won't expect a different structure

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/llm-mapper/transform/providers/openai/request/toGoogle.ts
Line: 410:414

Comment:
**style:** verify that wrapping primitives in `{ result: value }` matches Google's API expectations for `functionResponse.response`

the comment states Google requires an object (Struct), but confirm this is the correct field name (`result`) and that Google won't expect a different structure

How can I resolve this? If you propose a fix, please make it concise.

@vercel vercel bot temporarily deployed to Preview – helicone-bifrost November 25, 2025 21:21 Inactive
* fix package tests to test for the new gemini api response format

* add bedrock vers to claude response
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.

4 participants