Adapt tool_calls and support xinghuo large model

[liugddx]

Thank you for taking time to contribute this pull request!
You might have already read the [contributor guide][1], but as a reminder, please make sure to:

• Sign the contributor license agreement
• Rebase your changes on the latest main branch and squash your commits
• Add/Update unit tests as needed
• Run a build and make sure all tests pass prior to submission

Currently the Xinghuo LLM model doesn't determine whether it's end or not by the finish_reason in the metadata but by the presence or absence of usage field.

• streaming

``` {"code":0,"message":"Success","sid":"cha000bc93f@dx1932638ff83b8f2532","id":"cha000bc93f@dx1932638ff83b8f2532","created":1731513092,"choices":[{"delta":{"role":"assistant","content":"","tool_calls":{"type":"function","function":{"arguments":"{\"format\":\"celsius\",\"location\":\"河北省承德市双桥区\"}","name":"get_current_weather"}}},"index":0}],"usage":{"prompt_tokens":2,"completion_tokens":0,"total_tokens":2}} ``` - non-streaming

{"code":0,"message":"Success","sid":"cha000bc793@dx19326377f0cb8f2532","choices":[{"message":{"role":"assistant","content":"","tool_calls":{"type":"function","function":{"arguments":"{\"format\":\"celsius\",\"location\":\"河北省承德市双桥区\"}","name":"get_current_weather"}}},"index":0}],"usage":{"prompt_tokens":2,"completion_tokens":0,"total_tokens":2}}

[markpollack]

in the metadata but by the presence or absence of usage field.

This won't work, as ChatGenerationMetadata does not contain usage and it is quite likely always not null. The Usage field in in ChatResponseMetadata.

I can't tell from the docs on this model, but if they are trying to support an OpenAI API, then they should really implement finish_reason, it is common across all AI models we have, 12 of which extend this abstract base class.

[liugddx]

in the metadata but by the presence or absence of usage field.

This won't work, as ChatGenerationMetadata does not contain usage and it is quite likely always not null. The Usage field in in ChatResponseMetadata.

I can't tell from the docs on this model, but if they are trying to support an OpenAI API, then they should really implement finish_reason, it is common across all AI models we have, 12 of which extend this abstract base class.

Thanks for your reply. Yes I found the problem and I will fix it.

[tzolov]

The .contains(finishReason.toLowerCase()) || metadata != null); or event version with Generationusage is likely not going to be good condition as it can break many of the other implementations.

[liugddx]

The .contains(finishReason.toLowerCase()) || metadata != null); or event version with Generationusage is likely not going to be good condition as it can break many of the other implementations.

Is there any good method? I can only think of this way, or should I add a new model for xinghuo separately?

[liugddx]

PTAL @tzolov @markpollack

[markpollack]

Is this model trying to support an OpenAI API?

Not having finish reason seems a design mistake in their API. I want to limit the number of new models as much as possible and it seems like this one is close to openai api compatability. If there is a method that can be made protected in the current OpenAiChatModel or some other seam introduced that would a good way to approach the problem by adding only a small amount of code to support this model. I don't want to have a new model introduced that cut-n-pastes 99% of the current OpenAIChatModel as it will be maintance issue - and we actually already have this problem and don't want to make the problem worse.

[liugddx]

Is this model trying to support an OpenAI API?

Not having finish reason seems a design mistake in their API. I want to limit the number of new models as much as possible and it seems like this one is close to openai api compatability. If there is a method that can be made protected in the current OpenAiChatModel or some other seam introduced that would a good way to approach the problem by adding only a small amount of code to support this model. I don't want to have a new model introduced that cut-n-pastes 99% of the current OpenAIChatModel as it will be maintance issue - and we actually already have this problem and don't want to make the problem worse.

I agree with your idea. Maybe adding a modelType parameter in the openai module can solve this problem, and i can rewrite some methods through modelType=xinghuo?

[liugddx]

Is this model trying to support an OpenAI API?
Not having finish reason seems a design mistake in their API. I want to limit the number of new models as much as possible and it seems like this one is close to openai api compatability. If there is a method that can be made protected in the current OpenAiChatModel or some other seam introduced that would a good way to approach the problem by adding only a small amount of code to support this model. I don't want to have a new model introduced that cut-n-pastes 99% of the current OpenAIChatModel as it will be maintance issue - and we actually already have this problem and don't want to make the problem worse.

I agree with your idea. Maybe adding a modelType parameter in the openai module can solve this problem, and i can rewrite some methods through modelType=xinghuo?

In this way, the openai module can be made more extensible, and other models compatible with the openai specification can be removed and adapted using the openai module.

[liugddx]

I've tested it and it works for me.