Skip to content

Commit b31cded

Browse files
ericdalloericdallo
committed
Improve protocol for tool call output formatting for tools that output json.
1 parent 2252098 commit b31cded

File tree

7 files changed

+71
-91
lines changed

7 files changed

+71
-91
lines changed

‎CHANGELOG.md‎

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

33
## Unreleased
44

5+
- Improve protocol for tool call output formatting for tools that output json.
6+
- Fix inconsistencies in `eca_read_file` not passing correct content to LLM when json.
7+
58
## 0.75.0
69

710
- Improved file contexts: now use :lines-range

‎docs/protocol.md‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -984,7 +984,7 @@ interface ChatToolCallRejectedContent {
984984

985985
type ToolCallOrigin = 'mcp' | 'native';
986986

987-
type ToolCallDetails = FileChangeDetails;
987+
type ToolCallDetails = FileChangeDetails | JsonOutputsDetails;
988988

989989
interface FileChangeDetails {
990990
type: 'fileChange';
@@ -1010,6 +1010,15 @@ interface FileChangeDetails {
10101010
linesRemoved: number;
10111011
}
10121012

1013+
interface JsonOutputsDetails {
1014+
type: 'jsonOutputs';
1015+
1016+
/**
1017+
* The list of json outputs of this tool call properly formatted.
1018+
*/
1019+
jsons: string[];
1020+
}
1021+
10131022
/**
10141023
* Extra information about a chat
10151024
*/

‎src/eca/features/tools.clj‎

Lines changed: 11 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
"This ns centralizes all available tools for LLMs including
33
eca native tools and MCP servers."
44
(:require
5-
[cheshire.core :as json]
65
[clojure.string :as string]
76
[eca.features.tools.chat :as f.tools.chat]
87
[eca.features.tools.custom :as f.tools.custom]
@@ -75,27 +74,6 @@
7574
(mapv #(assoc % :origin :native) (native-tools db config))
7675
(mapv #(assoc % :origin :mcp) (f.mcp/all-tools db))))))
7776

78-
(defn ^:private pretty-format-any-json-output
79-
[result]
80-
(update result
81-
:contents
82-
(fn [contents]
83-
(mapv (fn [content]
84-
(if (= :text (:type content))
85-
(let [text (string/trim (:text content))]
86-
(if (or (and (string/starts-with? text "{")
87-
(string/ends-with? text "}"))
88-
(and (string/starts-with? text "[")
89-
(string/ends-with? text "]")))
90-
(try
91-
(update content :text #(json/generate-string (json/parse-string %) {:pretty true}))
92-
(catch Exception e
93-
(logger/warn logger-tag "Could not pretty format json text output for %s: %s" content (.getMessage e))
94-
content))
95-
content))
96-
content))
97-
contents))))
98-
9977
(defn call-tool! [^String name ^Map arguments chat-id tool-call-id behavior db* config messenger metrics
10078
call-state-fn ; thunk
10179
state-transition-fn ; params: event & event-data
@@ -104,18 +82,17 @@
10482
(let [arguments (update-keys arguments clojure.core/name)
10583
db @db*]
10684
(try
107-
(let [result (-> (if-let [native-tool-handler (get-in (native-definitions db config) [name :handler])]
108-
(native-tool-handler arguments {:db db
109-
:db* db*
110-
:config config
111-
:messenger messenger
112-
:behavior behavior
113-
:chat-id chat-id
114-
:tool-call-id tool-call-id
115-
:call-state-fn call-state-fn
116-
:state-transition-fn state-transition-fn})
117-
(f.mcp/call-tool! name arguments {:db db}))
118-
(pretty-format-any-json-output))]
85+
(let [result (if-let [native-tool-handler (get-in (native-definitions db config) [name :handler])]
86+
(native-tool-handler arguments {:db db
87+
:db* db*
88+
:config config
89+
:messenger messenger
90+
:behavior behavior
91+
:chat-id chat-id
92+
:tool-call-id tool-call-id
93+
:call-state-fn call-state-fn
94+
:state-transition-fn state-transition-fn})
95+
(f.mcp/call-tool! name arguments {:db db}))]
11996
(logger/debug logger-tag "Tool call result: " result)
12097
(metrics/count-up! "tool-called" {:name name :error (:error result)} metrics)
12198
result)

‎src/eca/features/tools/util.clj‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
(ns eca.features.tools.util
22
(:require
3+
[cheshire.core :as json]
34
[clojure.java.io :as io]
45
[clojure.java.shell :as shell]
56
[clojure.string :as string]
7+
[eca.logger :as logger]
68
[eca.shared :as shared]))
79

10+
(def ^:private logger-tag "[TOOLS-UTIL]")
11+
812
(defmulti tool-call-details-before-invocation
913
"Return the tool call details before invoking the tool."
1014
(fn [name _arguments _server _ctx] (keyword name)))
@@ -14,10 +18,29 @@
1418

1519
(defmulti tool-call-details-after-invocation
1620
"Return the tool call details after invoking the tool."
17-
(fn [name _arguments _details _result] (keyword name)))
21+
(fn [name _arguments _before-details _result] (keyword name)))
22+
23+
(defn ^:private json-outputs-if-any [result]
24+
(when-let [jsons (->> (:contents result)
25+
(keep (fn [content]
26+
(when (= :text (:type content))
27+
(let [text (string/trim (:text content))]
28+
(when (or (and (string/starts-with? text "{")
29+
(string/ends-with? text "}"))
30+
(and (string/starts-with? text "[")
31+
(string/ends-with? text "]")))
32+
(try
33+
(json/generate-string (json/parse-string text) {:pretty true})
34+
(catch Exception e
35+
(logger/warn logger-tag "Could not pretty format json text output for %s: %s" content (.getMessage e))
36+
nil)))))))
37+
seq)]
38+
{:type :jsonOutputs
39+
:jsons jsons}))
1840

19-
(defmethod tool-call-details-after-invocation :default [_name _arguments details _result]
20-
details)
41+
(defmethod tool-call-details-after-invocation :default [_name _arguments before-details result]
42+
(or before-details
43+
(json-outputs-if-any result)))
2144

2245
(defmulti tool-call-destroy-resource!
2346
"Destroy the tool call resource."

‎src/eca/main.clj‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,9 @@
66
[borkdude.dynaload]
77
[clojure.string :as string]
88
[eca.config :as config]
9-
[eca.secrets :as secrets]
109
[eca.logger :as logger]
11-
[eca.server :as server]
12-
[eca.proxy :as proxy]))
10+
[eca.proxy :as proxy]
11+
[eca.server :as server]))
1312

1413
(set! *warn-on-reflection* true)
1514

‎test/eca/features/chat_test.clj‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -242,15 +242,15 @@
242242
(on-message-received {:type :finish}))
243243
:call-tool-mock
244244
(constantly {:error false
245-
:contents [{:type :text :content "Allowed directories: /foo/bar"}]})})]
245+
:contents [{:type :text :text "Allowed directories: /foo/bar"}]})})]
246246
(is (match?
247247
{chat-id {:id chat-id
248248
:messages [{:role "user" :content [{:type :text :text "List the files you are allowed to see"}]}
249249
{:role "assistant" :content [{:type :text :text "Ok, working on it"}]}
250250
{:role "tool_call" :content {:id "call-1" :name "list_allowed_directories" :arguments {}}}
251251
{:role "tool_call_output" :content {:id "call-1" :name "list_allowed_directories" :arguments {}
252252
:output {:error false
253-
:contents [{:content "Allowed directories: /foo/bar"
253+
:contents [{:text "Allowed directories: /foo/bar"
254254
:type :text}]}}}
255255
{:role "assistant" :content [{:type :text :text "I can see: \n/foo/bar"}]}]}}
256256
(:chats (h/db))))
@@ -265,7 +265,7 @@
265265
{:role :assistant :content {:type :toolCallRun :id "call-1" :name "list_allowed_directories" :arguments {} :manual-approval false}}
266266
{:role :assistant :content {:type :toolCallRunning :id "call-1" :name "list_allowed_directories" :arguments {}}}
267267
{:role :system :content {:type :progress :state :running :text "Calling tool"}}
268-
{:role :assistant :content {:type :toolCalled :id "call-1" :name "list_allowed_directories" :arguments {} :total-time-ms number? :outputs [{:content "Allowed directories: /foo/bar" :type :text}]}}
268+
{:role :assistant :content {:type :toolCalled :id "call-1" :name "list_allowed_directories" :arguments {} :total-time-ms number? :outputs [{:text "Allowed directories: /foo/bar" :type :text}]}}
269269
{:role :system :content {:type :progress :state :running :text "Generating"}}
270270
{:role :assistant :content {:type :text :text "I can see: \n"}}
271271
{:role :assistant :content {:type :text :text "/foo/bar"}}
@@ -304,17 +304,17 @@
304304
"ro_tool_1"
305305
(do (deep-sleep 900)
306306
{:error false
307-
:contents [{:type :text :content "RO tool call 1 result"}]})
307+
:contents [{:type :text :text "RO tool call 1 result"}]})
308308

309309
"ro_tool_2"
310310
(do (deep-sleep 600)
311311
{:error false
312-
:contents [{:type :text :content "RO tool call 2 result"}]})
312+
:contents [{:type :text :text "RO tool call 2 result"}]})
313313

314314
"ro_tool_3"
315315
(do (deep-sleep 100)
316316
{:error false
317-
:contents [{:type :text :content "RO tool call 3 result"}]})))})]
317+
:contents [{:type :text :text "RO tool call 3 result"}]})))})]
318318

319319
(is (match?
320320
{chat-id {:id chat-id
@@ -323,15 +323,15 @@
323323
{:role "tool_call" :content {:id "call-3" :name "ro_tool_3" :arguments {}}}
324324
{:role "tool_call_output" :content {:id "call-3" :name "ro_tool_3" :arguments {}
325325
:output {:error false
326-
:contents [{:type :text, :content "RO tool call 3 result"}]}}}
326+
:contents [{:type :text, :text "RO tool call 3 result"}]}}}
327327
{:role "tool_call" :content {:id "call-2" :name "ro_tool_2" :arguments {}}}
328328
{:role "tool_call_output" :content {:id "call-2" :name "ro_tool_2" :arguments {}
329329
:output {:error false
330-
:contents [{:type :text, :content "RO tool call 2 result"}]}}}
330+
:contents [{:type :text, :text "RO tool call 2 result"}]}}}
331331
{:role "tool_call" :content {:id "call-1" :name "ro_tool_1" :arguments {}}}
332332
{:role "tool_call_output" :content {:id "call-1" :name "ro_tool_1" :arguments {}
333333
:output {:error false
334-
:contents [{:type :text, :content "RO tool call 1 result"}]}}}
334+
:contents [{:type :text, :text "RO tool call 1 result"}]}}}
335335
{:role "assistant" :content [{:type :text, :text "The tool calls returned: \nsomething"}]}]}}
336336
(:chats (h/db))))
337337
(is (match?
@@ -354,15 +354,15 @@
354354
{:role :assistant :content {:type :toolCallRunning :id "call-3" :name "ro_tool_3" :arguments {}}}
355355
{:role :system :content {:type :progress :state :running, :text "Calling tool"}}
356356
{:role :assistant :content {:type :toolCalled :id "call-3" :name "ro_tool_3" :arguments {}
357-
:outputs [{:type :text :content "RO tool call 3 result"}]
357+
:outputs [{:type :text :text "RO tool call 3 result"}]
358358
:error false}}
359359
{:role :system :content {:type :progress :state :running, :text "Generating"}}
360360
{:role :assistant :content {:type :toolCalled :id "call-2" :name "ro_tool_2" :arguments {}
361-
:outputs [{:type :text :content "RO tool call 2 result"}]
361+
:outputs [{:type :text :text "RO tool call 2 result"}]
362362
:error false}}
363363
{:role :system :content {:type :progress :state :running, :text "Generating"}}
364364
{:role :assistant :content {:type :toolCalled :id "call-1" :name "ro_tool_1" :arguments {}
365-
:outputs [{:type :text :content "RO tool call 1 result"}]
365+
:outputs [{:type :text :text "RO tool call 1 result"}]
366366
:error false}}
367367
{:role :system :content {:type :progress :state :running, :text "Generating"}}
368368
{:role :assistant :content {:type :text :text "The tool calls returned: \n"}}
@@ -407,36 +407,36 @@
407407
(when (= :timeout (deref wait-for-tool2 10000 :timeout))
408408
(println "tool-calls-with-prompt-stop-test: deref in tool 1 timed out"))
409409
{:error false
410-
:contents [{:type :text :content "RO tool call 1 result"}]})
410+
:contents [{:type :text :text "RO tool call 1 result"}]})
411411

412412
"ro_tool_2"
413413
(do (deep-sleep 800)
414414
(when (= :timeout (deref wait-for-stop 10000 :timeout))
415415
(println "tool-calls-with-prompt-stop-test: deref in tool 2 timed out"))
416416
(deliver wait-for-tool2 true)
417417
{:error false
418-
:contents [{:type :text :content "RO tool call 2 result"}]})
418+
:contents [{:type :text :text "RO tool call 2 result"}]})
419419

420420
"ro_tool_3"
421421
(do (deep-sleep 200)
422422
(deliver wait-for-tool3 true)
423423
{:error false
424-
:contents [{:type :text :content "RO tool call 3 result"}]})))})]
424+
:contents [{:type :text :text "RO tool call 3 result"}]})))})]
425425
(is (match? {chat-id
426426
{:id chat-id
427427
:messages [{:role "user" :content [{:type :text :text "Run 3 read-only tool calls simultaneously."}]}
428428
{:role "tool_call" :content {:id "call-3" :name "ro_tool_3" :arguments {}}}
429429
{:role "tool_call_output" :content {:id "call-3" :name "ro_tool_3" :arguments {}
430430
:output {:error false
431-
:contents [{:type :text, :content "RO tool call 3 result"}]}}}
431+
:contents [{:type :text, :text "RO tool call 3 result"}]}}}
432432
{:role "tool_call" :content {:id "call-2" :name "ro_tool_2" :arguments {}}}
433433
{:role "tool_call_output" :content {:id "call-2" :name "ro_tool_2" :arguments {}
434434
:output {:error false
435-
:contents [{:type :text, :content "RO tool call 2 result"}]}}}
435+
:contents [{:type :text, :text "RO tool call 2 result"}]}}}
436436
{:role "tool_call" :content {:id "call-1" :name "ro_tool_1" :arguments {}}}
437437
{:role "tool_call_output" :content {:id "call-1" :name "ro_tool_1" :arguments {}
438438
:output {:error false
439-
:contents [{:type :text, :content "RO tool call 1 result"}]}}}]}}
439+
:contents [{:type :text, :text "RO tool call 1 result"}]}}}]}}
440440
(:chats (h/db))))
441441
(is (match? {:chat-content-received
442442
[{:role :user :content {:type :text :text "Run 3 read-only tool calls simultaneously.\n"}}
@@ -455,7 +455,7 @@
455455
{:role :assistant :content {:type :toolCallRunning :id "call-3" :name "ro_tool_3" :arguments {}}}
456456
{:role :system :content {:type :progress :state :running :text "Calling tool"}}
457457
{:role :assistant :content {:type :toolCalled :id "call-3" :name "ro_tool_3" :arguments {}
458-
:outputs [{:type :text :content "RO tool call 3 result"}]}}
458+
:outputs [{:type :text :text "RO tool call 3 result"}]}}
459459
{:role :system :content {:type :progress :state :running :text "Generating"}}
460460
{:role :system :content {:type :text :text "\nPrompt stopped"}}
461461
{:role :system :content {:type :progress :state :finished}}

‎test/eca/features/tools_test.clj‎

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@
44
[eca.config :as config]
55
[eca.features.tools :as f.tools]
66
[eca.features.tools.filesystem :as f.tools.filesystem]
7-
[eca.features.tools.mcp :as f.mcp]
8-
[eca.shared :refer [multi-str]]
97
[eca.test-helper :as h]
108
[matcher-combinators.matchers :as m]
119
[matcher-combinators.test :refer [match?]]))
@@ -192,32 +190,3 @@
192190
;; Test safe commands that should NOT be denied
193191
(is (= :ask (f.tools/approval all-tools "eca_shell_command" {"command" "ls -la"} {} config "plan")))
194192
(is (= :ask (f.tools/approval all-tools "eca_shell_command" {"command" "git status"} {} config "plan")))))))
195-
196-
(deftest call-tool!-test
197-
(testing "We adapt output if json"
198-
(is (match?
199-
{:contents [{:type :text
200-
:text (if h/windows?
201-
(multi-str "{\r"
202-
" \"foo\" : \"123\",\r"
203-
" \"bar\" : 234\r"
204-
"}")
205-
(multi-str "{"
206-
" \"foo\" : \"123\","
207-
" \"bar\" : 234"
208-
"}"))}]
209-
:error false}
210-
(with-redefs [f.mcp/call-tool! (constantly {:contents [{:type :text
211-
:text "{\"foo\": \"123\", \"bar\": 234}"}]
212-
:error false})]
213-
(f.tools/call-tool! "my-json-tool"
214-
{}
215-
"chat-123"
216-
"tool-234"
217-
"agent"
218-
(h/db*)
219-
(h/config)
220-
(h/messenger)
221-
(h/metrics)
222-
identity
223-
identity))))))

0 commit comments

Comments
 (0)