Bug: Inconsistent Return Format When Using web_search with MiniMax

[znlnzi]

Description:

When using the web_search method with the minimax model, the return data format is inconsistent compared to the standard method. This inconsistency causes an error in the MiniMaxChatModel class’s call method, as it does not handle the web_search specific return format properly.

Steps to Reproduce: 1.Use the minimax model with the web_search method. 2.Observe the returned data format. 3.Compare it with the data format returned by the standard method (without web_search).

Expected Behavior: •The data returned should have a consistent format regardless of whether web_search is used or not.

Actual Behavior: •When using the standard method, the response is contained in choice.message. •When using web_search, the response is contained in choice.messages. •The MiniMaxChatModel class’s call method does not handle the web_search return format correctly, leading to an error.

Example response format with standard method:

{ "choice": { "message": "..." } }

Example response format with web_search:

{ "choice": { "messages": ["..."] } }

Additional Context: To fix this issue, the MiniMaxChatModel class’s call method should be updated to handle the web_search specific return format properly. The method should check for the existence of choice.messages and handle it accordingly.

[markpollack]

@mxsl-gr would you be able to look into this?

[jun10920]

Hello, I've created a PR to address this issue.

I approached this problem with the following steps:

1. Modified the call method in MiniMaxChatModel to handle both choice.message() and choice.messages().
2. Enhanced null checking to prevent NullPointerExceptions.
3. Added explanatory comments to the MiniMaxApi class regarding the messages field in web search responses.
4. Included additional test cases to cover both standard and web search response formats.

You can find these changes in PR #1324.

Additionally, I've suggested updating the official MiniMax API documentation to include an explanation of this difference in response formats.

I would appreciate your review and feedback on this approach.

[mxsl-gr]

@mxsl-gr would you be able to look into this?你能调查一下吗？

hi, @markpollack @znlnzi sorry for the late reply. i have been busy with a new project lately, aiming to manage the prompt engineering in a more structured way. why messages weren't directly fetched and put into the Choice.message was because the minimax API separates the returns multiple messages at once of web_search calls, which is incompatible with the previous setup of handling single messages. i noticed there's already a PR for this. I'll check if I can resolve it later.

[mxsl-gr]

Hello, I've created a PR to address this issue.

I approached this problem with the following steps:

1. Modified the call method in MiniMaxChatModel to handle both choice.message() and choice.messages().
2. Enhanced null checking to prevent NullPointerExceptions.
3. Added explanatory comments to the MiniMaxApi class regarding the messages field in web search responses.
4. Included additional test cases to cover both standard and web search response formats.

You can find these changes in PR #1324.

Additionally, I've suggested updating the official MiniMax API documentation to include an explanation of this difference in response formats.

I would appreciate your review and feedback on this approach.

hi @jun10920 , thanks for your PR. but i have checked the code and run the tests, and it seems to only work with mock interfaces. in real calls, the message is actually null

after reviewing the MiniMax documentation, i noticed some inconsistencies. For example, the web search returns 4 messages: [{user's question}, {assistant tool call}, {search tool call}, {assistant message}].

However, this PR https://github.com/spring-projects/spring-ai/pull/1324 is set to default to the first message in the Choice messages array. so the Choice.message we're getting through is actually the user's question.

[jun10920]

Hello @mxsl-gr, Would it be alright if I proceed to modify the mocking tests and adjust the code to return the fourth message? Thank you for reviewing and providing feedback! I'll work on these changes and update the PR accordingly.

[mxsl-gr]

Hello @mxsl-gr, Would it be alright if I proceed to modify the mocking tests and adjust the code to return the fourth message? Thank you for reviewing and providing feedback! I'll work on these changes and update the PR accordingly.

it's not just this one problem, return index 3 of the Choice.messages might resolve the issue with message errors. but i think there will definitely be problems in the stream mode.

[jun10920]

it's not just this one problem, return index 3 of the Choice.messages might resolve the issue with message errors. but i think there will definitely be problems in the stream mode.

I've been considering some approaches to address the inconsistency between standard and web-search response formats, as well as potential issues in streaming mode. I'd like to get your thoughts on the following proposed solutions:

Unifying response format: Modify the ChatCompletion class to always return a messages list. For standard responses, this list would contain a single message.

Improving message processing logic: In the call method, return only the last message when using web-search mode. Implement a way to store and access all messages if needed.

Enhancing streaming mode: Update the stream method to properly handle web-search responses. Identify message types for each chunk and only deliver the final response to the client.

Do you think these approaches would effectively address the current issues? Are there any potential drawbacks or additional considerations we should keep in mind? I'm particularly interested in your thoughts on how these changes might affect the overall design and usage of the MiniMaxChatModel.

Thank you for your time. It's lunchtime here, and I hope you're having a great time wherever you are!

[mxsl-gr]

it's not just this one problem, return index 3 of the Choice.messages might resolve the issue with message errors. but i think there will definitely be problems in the stream mode.

I've been considering some approaches to address the inconsistency between standard and web-search response formats, as well as potential issues in streaming mode. I'd like to get your thoughts on the following proposed solutions:

Unifying response format: Modify the ChatCompletion class to always return a messages list. For standard responses, this list would contain a single message.

Improving message processing logic: In the call method, return only the last message when using web-search mode. Implement a way to store and access all messages if needed.

Enhancing streaming mode: Update the stream method to properly handle web-search responses. Identify message types for each chunk and only deliver the final response to the client.

Do you think these approaches would effectively address the current issues? Are there any potential drawbacks or additional considerations we should keep in mind? I'm particularly interested in your thoughts on how these changes might affect the overall design and usage of the MiniMaxChatModel.

Thank you for your time. It's lunchtime here, and I hope you're having a great time wherever you are!

hi @jun10920 sorry, I just saw the message, thank you for reply. when I was testing the stream issue, i dealt with it by the way and added some related tests for ChatModel. but there is still a certain possibility that the tests will fail due to the uncertainty of web search. you can try it if it solves your problem

[mxsl-gr]

it's not just this one problem, return index 3 of the Choice.messages might resolve the issue with message errors. but i think there will definitely be problems in the stream mode.

I've been considering some approaches to address the inconsistency between standard and web-search response formats, as well as potential issues in streaming mode. I'd like to get your thoughts on the following proposed solutions:

Unifying response format: Modify the ChatCompletion class to always return a messages list. For standard responses, this list would contain a single message.

Improving message processing logic: In the call method, return only the last message when using web-search mode. Implement a way to store and access all messages if needed.

Enhancing streaming mode: Update the stream method to properly handle web-search responses. Identify message types for each chunk and only deliver the final response to the client.

Do you think these approaches would effectively address the current issues? Are there any potential drawbacks or additional considerations we should keep in mind? I'm particularly interested in your thoughts on how these changes might affect the overall design and usage of the MiniMaxChatModel.

@jun10920 sorry, i did not read the entire message content thoroughly just now, but I think the points you mentioned could solve the issue: returning the correct message and handling the block correctly.  if possible, you could also take a look at my changes and see if we have the same thoughts. it would be great if you could review my code.

[jun10920]

it's not just this one problem, return index 3 of the Choice.messages might resolve the issue with message errors. but i think there will definitely be problems in the stream mode.

I've been considering some approaches to address the inconsistency between standard and web-search response formats, as well as potential issues in streaming mode. I'd like to get your thoughts on the following proposed solutions: Unifying response format: Modify the ChatCompletion class to always return a messages list. For standard responses, this list would contain a single message. Improving message processing logic: In the call method, return only the last message when using web-search mode. Implement a way to store and access all messages if needed. Enhancing streaming mode: Update the stream method to properly handle web-search responses. Identify message types for each chunk and only deliver the final response to the client. Do you think these approaches would effectively address the current issues? Are there any potential drawbacks or additional considerations we should keep in mind? I'm particularly interested in your thoughts on how these changes might affect the overall design and usage of the MiniMaxChatModel.

@jun10920 sorry, i did not read the entire message content thoroughly just now, but I think the points you mentioned could solve the issue: returning the correct message and handling the block correctly.  if possible, you could also take a look at my changes and see if we have the same thoughts. it would be great if you could review my code.

@mxsl-gr I'll take a look at the code and get back to you within this week. I'll also submit a PR with additional documentation and comments related to web_search. Have a great day!