Open Moskize91 opened 1 week ago
Can I work on this issue? Thank you
@Moskize91 see I have patched a fix, default the role to assistant, if not provided in the vendor message.
I'm not convince that we want to do this unless there's a bit more evidence that this is an issue with many LLM providers. This could potentially lead to other subtle bugs down the road.
Would consider patching if there's enough support from the community + some examples of LLM providers that fail.
cc @efriis
@keenborder786 https://github.com/langchain-ai/langchain/pull/27398 great, if this PR can be merged, it will solve my problem.
@eyurtsev Actually, when I spent half an hour locating the crash of LangChain at these lines of code and determined that the vendor provided the wrong response, my first reaction was that it was the vendor's fault. So I fully understand what you said, "I don't love changing correct code into incorrect code just b/c some provider got the wrong format specification.".
But later I found that SillyTavern was actually compatible with this vendor that provided the error, so I went to check SillyTavern's code to see how its implementation was different from LangChain.
In fact, SillyTavern did nothing, it just didn't read the role field. If it doesn't read, it won't crash, it's that simple. I also agree that LangChain should not deliberately be compatible with vendor errors, but this does not mean that it will crash immediately when it finds that the vendor has made an error. LangChain can just pretend that it didn't see the error and let the code run.
In fact, as a user (such as me), when I encounter this problem, there is no way to get around it. In fact, I don't care about the role field at all. Because who else can the role returned by LLM be except "assistant"? But LangChain will crash on a problem that I don't care about.
For other LLM-related projects, you will find that they don't care about the role field at all. Since they don't read it, they won't crash like LangChain. They naturally don't make any compatibility for vendor errors, they just do nothing, so everything is normal.
Checked other resources
Example Code
Error Message and Stack Trace (if applicable)
Description
This error only occurs with a specific vendor. The vendor provides services in the Open AI API format, but some details are not completely consistent with Open AI.
Here is the code:
https://github.com/langchain-ai/langchain/blob/2197958366b527df23adbf12bf6578e4cd5e002c/libs/partners/openai/langchain_openai/chat_models/base.py#L109-L158
As you can see, LangChain will get the
role
field for the_dict
content returned by the vendor server and pass it into the if-else block for processing. After my test, in the reproduction code I provided, if the request is sent to the real OpenAI, the value of the role in the_dict
will beassistant
. But if the request is sent to certain specific vendors, the value ofrole
may be None.Therefore, the final code will enter the last else branch, namely:
ChatMessage(content=_dict.get("content", "")
.Unfortunately,
ChatMessage
will detect the format ofrole
in__init__()
and require it to be astr
. But since the value ofrole
isNone
, this request will fail 100%.I noticed that the
role
of some LLM suppliers does not contain any value. Of course, the API of these suppliers is not well designed. But if LangChain can be properly compatible with them, it will be perfect. Just add a judgment to this code. Ifrole
has no value, let it beassistant
by default, so that the API of this supplier can be handled smoothly.I noticed that the project https://github.com/SillyTavern/SillyTavern can handle this supplier's API correctly, but LangChain cannot handle it so far. SillyTavern uses the method I mentioned, so the request will not crash.
System Info
System Information
Package Information
Optional packages not installed
Other Dependencies