Skip to content

Commit 4696263

Browse files
ericdalloericdallo
authored
Merge pull request #286 from editor-code-assistant/fix/openai-chat-internal-tool-call-id
Fix openai-chat tool_call_id handling
2 parents 9a38039 + 18136f5 commit 4696263

3 files changed

Lines changed: 123 additions & 21 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
## Unreleased
44

5+
- Fix openai-chat tool calls freezing when providers emit duplicate/invalid tool_calls[].id values.
6+
57
## 0.98.0
68

79
- Add support for adding `extraHeaders` to models configuration.

‎src/eca/llm_providers/openai_chat.clj‎

Lines changed: 40 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515

1616
(def ^:private chat-completions-path "/chat/completions")
1717

18+
(defn ^:private new-internal-tool-call-id []
19+
(str (random-uuid)))
20+
1821
(defn ^:private parse-usage [usage]
1922
(let [input-cache-read-tokens (-> usage :prompt_tokens_details :cached_tokens)]
2023
{:input-tokens (if input-cache-read-tokens
@@ -73,15 +76,29 @@
7376
tool-turn-id (when (seq (:tool_calls message)) (str (random-uuid)))
7477
tools-to-call (->> (:tool_calls message)
7578
(map-indexed (fn [idx tool-call]
76-
(cond-> {:index idx
77-
:id (:id tool-call)
78-
:full-name (:name (:function tool-call))
79-
:arguments (json/parse-string (:arguments (:function tool-call)))}
80-
tool-turn-id (assoc :tool-turn-id tool-turn-id)
81-
;; Preserve Google Gemini thought signatures
82-
(get-in tool-call [:extra_content :google :thought_signature])
83-
(assoc :external-id
84-
(get-in tool-call [:extra_content :google :thought_signature]))))))
79+
(let [full-name (:name (:function tool-call))
80+
args-text (:arguments (:function tool-call))]
81+
(try
82+
(cond-> {:index idx
83+
;; Provider-supplied tool_call_id is not safe to use as internal
84+
;; identity (can be duplicated / contain spaces, etc).
85+
:id (new-internal-tool-call-id)
86+
:llm-tool-call-id (:id tool-call)
87+
:full-name full-name
88+
:arguments (json/parse-string args-text)}
89+
tool-turn-id (assoc :tool-turn-id tool-turn-id)
90+
;; Preserve Google Gemini thought signatures
91+
(get-in tool-call [:extra_content :google :thought_signature])
92+
(assoc :external-id
93+
(get-in tool-call [:extra_content :google :thought_signature])))
94+
(catch Exception e
95+
(logger/warn logger-tag
96+
(format "Failed to parse JSON arguments for tool '%s': %s"
97+
full-name
98+
(ex-message e)))
99+
nil)))))
100+
(remove nil?)
101+
vec)
85102
;; DeepSeek returns reasoning_content, OpenAI o1 returns reasoning
86103
reasoning-content (:reasoning_content message)]
87104
{:usage (parse-usage usage)
@@ -145,17 +162,18 @@
145162
- Otherwise, wrap the text in thinking tags (text-based reasoning fallback)"
146163
[{:keys [role content] :as _msg} supports-image? think-tag-start think-tag-end]
147164
(case role
148-
"tool_call" {:role "assistant"
149-
:tool_calls [(cond-> {:id (:id content)
150-
:type "function"
151-
:function {:name (:full-name content)
152-
:arguments (json/generate-string (:arguments content))}}
153-
;; Preserve Google Gemini thought signatures if present
154-
(:external-id content)
155-
(assoc-in [:extra_content :google :thought_signature]
156-
(:external-id content)))]}
165+
"tool_call" (let [tool-call-id (or (:llm-tool-call-id content) (:id content))]
166+
{:role "assistant"
167+
:tool_calls [(cond-> {:id tool-call-id
168+
:type "function"
169+
:function {:name (:full-name content)
170+
:arguments (json/generate-string (:arguments content))}}
171+
;; Preserve Google Gemini thought signatures if present
172+
(:external-id content)
173+
(assoc-in [:extra_content :google :thought_signature]
174+
(:external-id content)))]})
157175
"tool_call_output" {:role "tool"
158-
:tool_call_id (:id content)
176+
:tool_call_id (or (:llm-tool-call-id content) (:id content))
159177
:content (llm-util/stringfy-tool-result content)}
160178
"user" {:role "user"
161179
:content (extract-content content supports-image?)}
@@ -474,7 +492,7 @@
474492
(on-reason {:status :started :id new-reason-id})))
475493
find-existing-tool-key (fn [tool-calls index id]
476494
;; Find existing tool call by id match, or by index when delta has no id
477-
(some (fn [[k v]] (when (or (some-> id (= (:id v)))
495+
(some (fn [[k v]] (when (or (some-> id (= (:llm-tool-call-id v)))
478496
(and (nil? id)
479497
(some-> index (= (:index v)))))
480498
k))
@@ -566,8 +584,9 @@
566584
;; Providers may omit tool_calls[].index; preserve
567585
;; stream arrival order for a stable UX fallback.
568586
created? (assoc :stream-order (count tool-calls))
587+
created? (assoc :id (new-internal-tool-call-id))
569588
(some? index) (assoc :index index)
570-
(and id (nil? (:id existing))) (assoc :id id)
589+
(and id (nil? (:llm-tool-call-id existing))) (assoc :llm-tool-call-id id)
571590
(and name (nil? (:full-name existing))) (assoc :full-name name)
572591
args (update :arguments-text (fnil str "") args)
573592
;; Store thought signature for Google Gemini

‎test/eca/llm_providers/openai_chat_test.clj‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,22 @@
146146
thinking-start-tag
147147
thinking-end-tag))))
148148

149+
(testing "Tool call transformation prefers :llm-tool-call-id when present"
150+
(is (match?
151+
{:role "assistant"
152+
:tool_calls [{:id "external-456"
153+
:type "function"
154+
:function {:name "foo__get_weather"}}]}
155+
(#'llm-providers.openai-chat/transform-message
156+
{:role "tool_call"
157+
:content {:id "internal-123"
158+
:llm-tool-call-id "external-456"
159+
:full-name "foo__get_weather"
160+
:arguments {:location "NYC"}}}
161+
true
162+
thinking-start-tag
163+
thinking-end-tag))))
164+
149165
(testing "Tool call output transformation"
150166
(is (match?
151167
{:role "tool"
@@ -159,6 +175,19 @@
159175
thinking-start-tag
160176
thinking-end-tag))))
161177

178+
(testing "Tool call output transformation prefers :llm-tool-call-id when present"
179+
(is (match?
180+
{:role "tool"
181+
:tool_call_id "external-456"}
182+
(#'llm-providers.openai-chat/transform-message
183+
{:role "tool_call_output"
184+
:content {:id "internal-123"
185+
:llm-tool-call-id "external-456"
186+
:output {:contents [{:type :text :text "ok"}]}}}
187+
true
188+
thinking-start-tag
189+
thinking-end-tag))))
190+
162191
(testing "Reason messages - use reasoning_content if :delta-reasoning?, otherwise tags"
163192
;; Without :delta-reasoning?, uses think tags (string, not array - for Gemini compatibility)
164193
(is (match?
@@ -190,6 +219,58 @@
190219
thinking-start-tag
191220
thinking-end-tag)))))
192221

222+
(deftest normalize-messages-llm-tool-call-id-test
223+
(testing "Outgoing normalization uses :llm-tool-call-id for tool_call + tool_call_output"
224+
(is (match?
225+
[{:role "user"}
226+
{:role "assistant"
227+
:tool_calls [{:id "external-1"
228+
:function {:name "eca__list_files"
229+
:arguments "{}"}}]}
230+
{:role "tool"
231+
:tool_call_id "external-1"}]
232+
(#'llm-providers.openai-chat/normalize-messages
233+
[{:role "user" :content "List"}
234+
{:role "tool_call"
235+
:content {:id "internal-1"
236+
:llm-tool-call-id "external-1"
237+
:full-name "eca__list_files"
238+
:arguments {}}}
239+
{:role "tool_call_output"
240+
:content {:id "internal-1"
241+
:llm-tool-call-id "external-1"
242+
:output {:contents [{:type :text :text "file1"}]}}}]
243+
true
244+
thinking-start-tag
245+
thinking-end-tag)))))
246+
247+
(deftest response-body->result-internal-tool-call-id-test
248+
(testing "Non-stream tool calls use internal :id and preserve provider tool_call_id in :llm-tool-call-id"
249+
(let [ids* (atom ["turn-uuid" "tool-uuid" "reason-uuid"])
250+
result (with-redefs [random-uuid (fn []
251+
(let [id (first @ids*)]
252+
(swap! ids* rest)
253+
id))]
254+
(#'llm-providers.openai-chat/response-body->result
255+
{:choices [{:message {:role "assistant"
256+
:content ""
257+
:tool_calls [{:id " functions.eca__shell_command:56"
258+
:type "function"
259+
:function {:name "eca__shell_command"
260+
:arguments "{\"command\":\"echo hi\"}"}}]}}]
261+
:usage {:prompt_tokens 1 :completion_tokens 1}}
262+
(fn [& _] nil)))
263+
tool-call (first (:tools-to-call result))]
264+
(is (match?
265+
{:id "tool-uuid"
266+
:llm-tool-call-id " functions.eca__shell_command:56"
267+
:full-name "eca__shell_command"
268+
:arguments {"command" "echo hi"}
269+
:tool-turn-id "turn-uuid"}
270+
tool-call))
271+
(is (= "reason-uuid" (:reason-id result)))
272+
(is (not= (:id tool-call) (:llm-tool-call-id tool-call))))))
273+
193274
(deftest merge-adjacent-assistants-test
194275
(testing "All adjacent assistant messages are merged (even without reasoning_content)"
195276
(is (match?

0 commit comments

Comments
 (0)