fix(openai): Tool name or arguments might be empty/null#9828
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 |
…#9828) Some OpenAI-compatible providers emit empty or null name/arguments/id fields in streaming continuation deltas to keep the response schema stable. pgAdmin's accumulator overwrote the real tool name (captured in the first delta) with the later null, producing a tool call named "null" that could not be dispatched. Skip falsy name/arguments/id when accumulating (matching the OpenAI Python SDK, which ignores nulls the same way) so the values captured in the first delta survive. Also guard against a null `function` object in a delta, which previously raised TypeError. Without the id guard a null id in a continuation delta clobbered the real id, which the final build then replaced with a random uuid rather than the provider's id. Adds a unit test covering the null-continuation, multi-chunk-arguments, and null-function cases, and a 9.16 release note.
|
Merged to Folded in two related fixes while merging, since they're the same root cause (providers emitting null fields in continuation deltas):
Also added a unit test ( |
2 participants
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