Skip to content

fix: stabilize long running agent tasks for Ollama APIkey #673

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
cpp977 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: stabilize long running agent tasks for Ollama APIkey #673

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
18 changes: 9 additions & 9 deletions integration-test/integration/chat/ollama_test.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)))))))

Expand Down Expand Up @@ -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"
Expand Down
74 changes: 44 additions & 30 deletions src/eca/llm_providers/ollama.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]}
Expand Down
52 changes: 43 additions & 9 deletions test/eca/llm_providers/ollama_test.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}}}
Expand All @@ -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"
Expand Down
Loading