Skip to content

Conversation

arturfromtabnine
Copy link
Contributor

@arturfromtabnine arturfromtabnine commented Aug 12, 2025

Description

Allow tool content to be a typeof object for google models on vertex (which is compatible with open ai api https://github.com/openai/openai-node/blob/40b79cb8a372dce925b14a26e62110f9de6786ba/src/resources/chat/completions/completions.ts#L1407). Otherwise if content is a typeof object, role would not be handled properly and request will fail with following error:

{"error":{"message":"vertex-ai error: Please use a valid role: user, model.","type":"INVALID_ARGUMENT","param":null,"code":"400"},"provider":"vertex-ai"}

https://cloud.google.com/vertex-ai/generative-ai/docs/multimodal/function-calling

Motivation

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)

How Has This Been Tested?

  • Unit Tests
  • Integration Tests
  • Manual Testing

Screenshots (if applicable)

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Related Issues

Copy link
Contributor

@matter-code-review matter-code-review bot left a comment

Choose a reason for hiding this comment

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

Clean implementation that properly extends tool content support to include objects while maintaining backward compatibility.

Comment on lines 104 to 105
(typeof message.content === 'string' ||
typeof message.content === 'object')
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Code Refactor

Issue: The type check could be more robust for object validation
Fix: Consider validating that object content is not null and has expected structure
Impact: Prevents potential runtime errors with malformed object content

Suggested change
(typeof message.content === 'string' ||
typeof message.content === 'object')
(typeof message.content === 'string' ||
(typeof message.content === 'object' && message.content !== null))

Copy link
Collaborator

@narengogi narengogi left a comment

Choose a reason for hiding this comment

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

Hey @arturfromtabnine
Openai chat completion content is always a string

message.role === 'tool' &&
typeof message.content === 'string'
(typeof message.content === 'string' ||
typeof message.content === 'object')
Copy link
Collaborator

@narengogi narengogi Aug 12, 2025

Choose a reason for hiding this comment

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

OpenAI spec only supports sending text content or array of text content parts as part of tool message content https://platform.openai.com/docs/api-reference/chat/create#chat_create-messages

I believe what you are trying to do is to send an object like this

{
  "role": "user",
  "parts": [
    {
      "functionResponse": {
        "name": "get_current_weather",
        "response": {
          "temperature": 20,
          "unit": "C"
        }
      }
    }
  ]
}

instead of

{
  "role": "user",
  "parts": [
    {
      "functionResponse": {
        "name": "get_current_weather",
        "response": { "content": "{'temperature': 20, 'unit': 'C'}"}
      }
    }
  ]
}

the current implementation is definitely incomplete, what we should be doing is

 else if (
            message.role === 'tool' &&
            typeof message.content === 'string'
          ) {
            let toolResponse;
            try {
              toolResponse = JSON.parse(message.content);
            } catch (e) {
              toolResponse = {
                content: message.content,
              };
            }
            parts.push({
              functionResponse: {
                name: message.name ?? 'gateway-tool-filler-name',
                response,
              },
            });
          } 

Copy link
Collaborator

Choose a reason for hiding this comment

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

we need to handle array of text contents as well, which we can change in a separate PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, we need to do a strict check for array, but your example is not what we are trying to send. Our client is open ai compatible, so its trying to send an array in message.content

{
role: 'tool',
tool_call_id: 'call_ty2u3gJiEIsf9TLOwUB67opO',
content: [
  {
   type: 'text',
   text: '{"filePath"}'
  }
 ]
}

with suggested change there no issue, vertex api for gemini models allows to have response.content as an array or string

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if we need to parse content like was suggested

            typeof message.content === 'string'
          ) {
            let toolResponse;
            try {
              toolResponse = JSON.parse(message.content);
            } catch (e) {
              toolResponse = {
                content: message.content,
              };
            }

@Portkey-AI Portkey-AI deleted a comment from matter-code-review bot Aug 12, 2025
@Portkey-AI Portkey-AI deleted a comment from matter-code-review bot Aug 12, 2025
@arturfromtabnine
Copy link
Contributor Author

arturfromtabnine commented Aug 13, 2025

Hey @arturfromtabnine Openai chat completion content is always a string

Hey @narengogi it can be an array as well
Screenshot 2025-08-13 at 16 50 54

@narengogi
Copy link
Collaborator

narengogi commented Aug 14, 2025

Hey @arturfromtabnine Openai chat completion content is always a string

Hey @narengogi it can be an array as well

But it's an array of text content parts, you need an object for vertex right?

@arturfromtabnine
Copy link
Contributor Author

arturfromtabnine commented Aug 26, 2025

Hey @arturfromtabnine Openai chat completion content is always a string

Hey @narengogi it can be an array as well

But it's an array of text content parts, you need an object for vertex right?

Sorry, was off
we do send content for tools in form of array , not an object

        content: [
          {
            type: "text",
            text: "content",
          },
        ],

will update suggested condition to be more precise on array instead of being too generic

Copy link
Contributor

matter-code-review bot commented Aug 26, 2025

Code Quality bug fix

Description

Summary By MatterAI MatterAI logo

🔄 What Changed

The condition typeof message.content === 'string' was removed from the tool role message processing, allowing message.content to be any type (including objects). Additionally, the key for the tool's response content was updated from content to output within the functionResponse.response object.

🔍 Impact of the Change

This modification resolves an INVALID_ARGUMENT error (code 400) that occurred when message.role was tool and message.content was an object. By allowing object-type content and using the output key, the system now correctly handles tool responses, aligning with Google Vertex AI's multimodal function calling and OpenAI API specifications, enhancing compatibility and preventing request failures.

📁 Total Files Changed

  • src/providers/google/chatComplete.ts: Modified the condition for tool role messages and updated the key for tool response content (2 additions, 5 deletions).

🧪 Test Added

Manual testing was performed to verify the fix.

🔒Security Vulnerabilities

N/A - No new security vulnerabilities were introduced or detected.

Motivation

Allow tool content to be a typeof object for google models on vertex (which is compatible with open ai api https://github.com/openai/openai-node/blob/40b79cb8a372dce925b14a26e621110f9de6786ba/src/resources/chat/completions/completions.ts#L1407). Otherwise if content is a typeof object role would not be handled properly and request will fail with following error:

{"error":{"message":"vertex-ai error: Please use a valid role: user, model.","type":"INVALID_ARGUMENT","param":null,"code":"400"},"provider":"vertex-ai"}

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)

How Has This Been Tested?

  • Unit Tests
  • Integration Tests
  • Manual Testing

Screenshots (if applicable)

N/A

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Related Issues

https://cloud.google.com/vertex-ai/generative-ai/docs/multimodal/function-calling

Tip

Quality Recommendations

  1. Consider making the default name for functionResponse (currently 'gateway-tool-filler-name') configurable or ensuring it's always provided by the upstream system for better clarity and debugging.

  2. While message.content now accepts objects, ensure that the structure of these objects is validated against expected schemas for the specific tools to prevent runtime errors in downstream systems.

Tanka Poem ♫

Object now flows free,
No more errors, Vertex smiles,
Data finds its path. 🚀
Tool calls sing a new sweet song,
Logic's dance, a graceful step. ✨

Sequence Diagram

sequenceDiagram
    participant User
    participant Gateway
    participant GoogleChatComplete as Google Chat Complete Provider
    participant VertexAI as Vertex AI API

    User->>Gateway: Send Chat Message (with tool output)
    Gateway->>+GoogleChatComplete: processMessage(message: { role: "tool", content: any, name?: string })
    Note over GoogleChatComplete: Handles message with role 'tool'
    GoogleChatComplete->>GoogleChatComplete: Construct functionResponse
    Note over GoogleChatComplete: Sets functionResponse.response.output = message.content (object or string)
    GoogleChatComplete->>+VertexAI: POST /v1/projects/.../locations/.../publishers/google/models/...:generateContent
    VertexAI-->>-GoogleChatComplete: API Response (Success or Error)
    GoogleChatComplete-->>-Gateway: Formatted Response
    Gateway-->>User: Chat Response
Loading

Copy link
Contributor

Important

PR Review Skipped

PR review skipped as per the configuration setting. Run a manually review by commenting /matter review

💡Tips to use Matter AI

Command List

  • /matter summary: Generate AI Summary for the PR
  • /matter review: Generate AI Reviews for the latest commit in the PR
  • /matter review-full: Generate AI Reviews for the complete PR
  • /matter release-notes: Generate AI release-notes for the PR
  • /matter : Chat with your PR with Matter AI Agent
  • /matter remember : Generate AI memories for the PR
  • /matter explain: Get an explanation of the PR
  • /matter help: Show the list of available commands and documentation
  • Need help? Join our Discord server: https://discord.gg/fJU5DvanU3

Copy link
Contributor

Important

PR Review Skipped

PR review skipped as per the configuration setting. Run a manually review by commenting /matter review

💡Tips to use Matter AI

Command List

  • /matter summary: Generate AI Summary for the PR
  • /matter review: Generate AI Reviews for the latest commit in the PR
  • /matter review-full: Generate AI Reviews for the complete PR
  • /matter release-notes: Generate AI release-notes for the PR
  • /matter : Chat with your PR with Matter AI Agent
  • /matter remember : Generate AI memories for the PR
  • /matter explain: Get an explanation of the PR
  • /matter help: Show the list of available commands and documentation
  • Need help? Join our Discord server: https://discord.gg/fJU5DvanU3

Copy link
Contributor

Important

PR Review Skipped

PR review skipped as per the configuration setting. Run a manually review by commenting /matter review

💡Tips to use Matter AI

Command List

  • /matter summary: Generate AI Summary for the PR
  • /matter review: Generate AI Reviews for the latest commit in the PR
  • /matter review-full: Generate AI Reviews for the complete PR
  • /matter release-notes: Generate AI release-notes for the PR
  • /matter : Chat with your PR with Matter AI Agent
  • /matter remember : Generate AI memories for the PR
  • /matter explain: Get an explanation of the PR
  • /matter help: Show the list of available commands and documentation
  • Need help? Join our Discord server: https://discord.gg/fJU5DvanU3

Copy link
Collaborator

@narengogi narengogi left a comment

Choose a reason for hiding this comment

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

I've removed the check entirely and updated the key from content to output as mentioned in the vertex spex
Image

https://cloud.google.com/vertex-ai/docs/reference/rest/v1/Content#FunctionResponse

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