fix(openai): Tool name or arguments might be empty/null#9828
fix(openai): Tool name or arguments might be empty/null#9828tcx4c70 wants to merge 1 commit intopgadmin-org:masterfrom
Conversation
Some providers might return empty/null values in streaming delta (I
guess it's to keep schema consistent?). For example, streaming data for
a request might be:
data: { ... "choices":[{"index":0,"delta":{"role":"assistant","tool_calls":[{"index":0, ... "function":{"name":"get_database_schema","arguments":""}}]}}] ... }
data: { ... "choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"name":null,"arguments":"{}"}}]}}] ... }
data: { ... "choices":[{"index":0,"finish_reason":"tool_calls","delta":{}}] ... }
data: [DONE]
Then pgAdmin will combine the delta to a tool call with name "null" and
arguments "{}", which leads to a error that can't find the tool.
Though I don't find the official doc that mentions we should skip null
value in delta, the official openai python sdk skips null values [1].
This commit is to skip null values in streaming delta to avoid
empty/null tool name or arguments.
[1] https://github.com/openai/openai-python/blob/58184ad545ee2abd98e171ee09766f259d7f38cd/src/openai/lib/streaming/_deltas.py#L6
Signed-off-by: Adam Tao <tcx4c70@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe change adds truthiness checks to tool call field assignments in OpenAI's SSE stream parsing to prevent empty-string overwrites and concatenations while processing tool call deltas. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Some providers might return empty/null values in streaming delta (I guess it's to keep schema consistent?). For example, streaming data for a request might be:
data: { ... "choices":[{"index":0,"delta":{"role":"assistant","tool_calls":[{"index":0, ... "function":{"name":"get_database_schema","arguments":""}}]}}] ... }
data: { ... "choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"name":null,"arguments":"{}"}}]}}] ... }
data: { ... "choices":[{"index":0,"finish_reason":"tool_calls","delta":{}}] ... }
data: [DONE]
Then pgAdmin will combine the delta to a tool call with name "null" and arguments "{}", which leads to a error that can't find the tool.
Though I don't find the official doc that mentions we should skip null value in delta, the official openai python sdk skips null values [1].
This commit is to skip null values in streaming delta to avoid empty/null tool name or arguments.
[1] https://github.com/openai/openai-python/blob/58184ad545ee2abd98e171ee09766f259d7f38cd/src/openai/lib/streaming/_deltas.py#L6
Summary by CodeRabbit