From cd78852dab58c9188e8e097b4b3f61a645da3464 Mon Sep 17 00:00:00 2001 From: Matthias Peschke Date: Tue, 27 Oct 2026 08:18:38 +0100 Subject: [PATCH] fix: stabilize long running agent tasks for Ollama APIkey See #672. --- CHANGELOG.md | 3 + .../integration/chat/ollama_test.clj | 18 ++--- src/eca/llm_providers/ollama.clj | 74 +++++++++++-------- test/eca/llm_providers/ollama_test.clj | 52 ++++++++++--- 4 files changed, 99 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 116f00e32..ca6cf7799 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,9 +2,12 @@ ## Unreleased +- Fix Ollama models stopping after announcing a tool call in long chats: send previous tool calls as `tool_calls` and merge a response's reasoning, text and tool calls into a single assistant message. #673 - Official support for `eca-cli` as an official client. - Docs: Fix landing page hero clipping the editor tabs and demo on tablet and small laptop widths. - Docs: Add logo wall and improvements in home page. #676 +- Add `eca-cli` as an official client in docs and landing page. +- Fix landing page hero clipping the editor tabs and demo on tablet and small laptop widths. ## 0.168.0 diff --git a/integration-test/integration/chat/ollama_test.clj b/integration-test/integration/chat/ollama_test.clj index 66eec82e9..f08b0b37f 100644 --- a/integration-test/integration/chat/ollama_test.clj +++ b/integration-test/integration/chat/ollama_test.clj @@ -182,8 +182,7 @@ {:model "qwen3" :messages [{:role "system" :content (m/pred string?)} {:role "user" :content "hello!"} - {:role "assistant" :content "I should say hello"} - {:role "assistant" :content "hello there!"} + {:role "assistant" :content "I should say hello\n\nhello there!"} {:role "user" :content "how are you?"}]} (llm.mocks/get-req-body :reasoning-1))))))) @@ -259,13 +258,14 @@ (is (match? {:model "qwen3" :messages [{:role "user" :content "What files you see?"} - {:role "assistant" :content "I will list files"} - {:role "assistant" :tool-calls [{:type "function" - :function {:id (m/pred string?) - :name "eca__directory_tree" - :arguments {:path (h/project-path->canon-path "resources")} - :summary "Listing tree: resources" - :origin "native"}}]} + {:role "assistant" + :content "I will list files" + :tool_calls [{:type "function" + :function {:id (m/pred string?) + :name "eca__directory_tree" + :arguments {:path (h/project-path->canon-path "resources")} + :summary "Listing tree: resources" + :origin "native"}}]} {:role "tool" :content (str (h/project-path->canon-path "resources") "\n" " file1.md\n" " file2.md\n\n" diff --git a/src/eca/llm_providers/ollama.clj b/src/eca/llm_providers/ollama.clj index 58e3889cd..fc5cb0114 100644 --- a/src/eca/llm_providers/ollama.clj +++ b/src/eca/llm_providers/ollama.clj @@ -122,42 +122,56 @@ (assoc :name (:full-name tool)))}) tools)) +(defn ^:private merge-adjacent-assistants [messages] + (reduce (fn [acc msg] + (let [prev (peek acc)] + (if (and (= "assistant" (:role prev)) (= "assistant" (:role msg))) + (conj (pop acc) + (cond-> prev + (seq (:content msg)) + (update :content #(if (seq %) (str % "\n\n" (:content msg)) (:content msg))) + (:tool_calls msg) + (update :tool_calls (fnil into []) (:tool_calls msg)))) + (conj acc msg)))) + [] messages)) + (defn ^:private normalize-messages [messages] ;; Defense-in-depth against #209: skip entries whose :content :api was ;; tagged by another provider. Ollama's chat shape doesn't carry ;; provider-specific opaque ids, but foreign tool_call/server_tool_* ;; entries can still confuse the request. The central sanitizer in ;; eca.llm-api drops these first; this guard protects direct callers. - (into [] - (keep (fn [{:keys [role content] :as msg}] - (let [foreign-api? (let [origin (:api content)] - (and origin (not= :ollama origin)))] - (case role - "tool_call" (when-not foreign-api? - {:role "assistant" - :tool-calls [{:type "function" - :function (-> content - (assoc :name (:full-name content)) - (dissoc :full-name))}]}) - ;; NOTE: Image content from MCP tool results is flattened to - ;; placeholder text via `stringfy-tool-result`. Image round-trip - ;; is implemented for openai-responses and anthropic; see those - ;; providers' `tool_call_output` branches for the pattern. - "tool_call_output" (when-not foreign-api? - {:role "tool" - :content (llm-util/stringfy-tool-result content)}) - "reason" (when-not foreign-api? - {:role "assistant" :content (:text content)}) - "server_tool_use" nil - "server_tool_result" nil - {:role (:role msg) - :content (if (string? (:content msg)) - (:content msg) - (-> msg :content first :text)) - ;; TODO add image supprt - ;; :images [] - })))) - messages)) + (merge-adjacent-assistants + (into [] + (keep (fn [{:keys [role content] :as msg}] + (let [foreign-api? (let [origin (:api content)] + (and origin (not= :ollama origin)))] + (case role + "tool_call" (when-not foreign-api? + {:role "assistant" + :tool_calls [{:type "function" + :function (-> content + (assoc :name (:full-name content)) + (dissoc :full-name))}]}) + ;; NOTE: Image content from MCP tool results is flattened to + ;; placeholder text via `stringfy-tool-result`. Image round-trip + ;; is implemented for openai-responses and anthropic; see those + ;; providers' `tool_call_output` branches for the pattern. + "tool_call_output" (when-not foreign-api? + {:role "tool" + :content (llm-util/stringfy-tool-result content)}) + "reason" (when-not foreign-api? + {:role "assistant" :content (:text content)}) + "server_tool_use" nil + "server_tool_result" nil + {:role (:role msg) + :content (if (string? (:content msg)) + (:content msg) + (-> msg :content first :text)) + ;; TODO add image supprt + ;; :images [] + })))) + messages))) (defn chat! [{:keys [model user-messages reason? instructions api-url past-messages tools max-output-tokens extra-headers extra-payload cancelled? stream-idle-timeout-seconds]} diff --git a/test/eca/llm_providers/ollama_test.clj b/test/eca/llm_providers/ollama_test.clj index 8ea725829..204eaac0b 100644 --- a/test/eca/llm_providers/ollama_test.clj +++ b/test/eca/llm_providers/ollama_test.clj @@ -116,15 +116,16 @@ [{:role "user" :content "Count with me: 1"} {:role "assistant" :content "2"}])))) (testing "With tool_call history" - (is (match? - [{:role "user" :content "List the files you are allowed"} - {:role "assistant" :content "Ok!"} - {:role "assistant" :tool-calls [{:type "function" - :function {:name "eca__list_allowed_directories" - :arguments {}}}]} - {:role "tool" :content "Allowed directories: /foo/bar\n"} - {:role "assistant" :content "I see /foo/bar"}] - (#'llm-providers.ollama/normalize-messages + (is (match? + [{:role "user" :content "List the files you are allowed"} + {:role "assistant" + :content "Ok!" + :tool_calls [{:type "function" + :function {:name "eca__list_allowed_directories" + :arguments {}}}]} + {:role "tool" :content "Allowed directories: /foo/bar\n"} + {:role "assistant" :content "I see /foo/bar"}] + (#'llm-providers.ollama/normalize-messages [{:role "user" :content "List the files you are allowed"} {:role "assistant" :content "Ok!"} {:role "tool_call" :content {:id "call-1" :full-name "eca__list_allowed_directories" :arguments {}}} @@ -135,6 +136,39 @@ :error false :text "Allowed directories: /foo/bar"}]}}} {:role "assistant" :content "I see /foo/bar"}]))))) + (testing "Reasoning, text and tool call of one response merge into one message" + (is (match? + [{:role "user" :content "Add a todo"} + {:role "assistant" + :content "I should read the file first.\n\nReading the file." + :tool_calls [{:type "function" + :function {:name "eca__read_file" :arguments {"path" "/a"}}}]}] + (#'llm-providers.ollama/normalize-messages + [{:role "user" :content "Add a todo"} + {:role "reason" :content {:id "r1" :text "I should read the file first."}} + {:role "assistant" :content "Reading the file."} + {:role "tool_call" :content {:id "call-1" :full-name "eca__read_file" + :arguments {"path" "/a"}}}])))) + (testing "A tool result separates two responses" + (is (match? + [{:role "assistant" :content "First." :tool_calls [{:function {:name "eca__read_file"}}]} + {:role "tool" :content "file body\n"} + {:role "assistant" :content "Second.\n\nDone."}] + (#'llm-providers.ollama/normalize-messages + [{:role "reason" :content {:id "r1" :text "First."}} + {:role "tool_call" :content {:id "call-1" :full-name "eca__read_file" :arguments {}}} + {:role "tool_call_output" :content {:id "call-1" :full-name "eca__read_file" :arguments {} + :output {:contents [{:type :text :error false :text "file body"}]}}} + {:role "reason" :content {:id "r2" :text "Second."}} + {:role "assistant" :content "Done."}])))) + (testing "Consecutive tool calls merge into one message" + (is (match? + [{:role "assistant" + :tool_calls [{:function {:name "eca__read_file"}} + {:function {:name "eca__grep"}}]}] + (#'llm-providers.ollama/normalize-messages + [{:role "tool_call" :content {:id "c1" :full-name "eca__read_file" :arguments {}}} + {:role "tool_call" :content {:id "c2" :full-name "eca__grep" :arguments {}}}])))) (deftest chat-stream-cancelled-test (testing "watchdog aborts a hung stream when cancelled, surfacing a silent error"