Skip to content

Keep compact tool schema stable accross compaction #565

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
zikajk wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Keep compact tool schema stable accross compaction #565

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
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## Unreleased

- Keep `compact_chat` in the tool schema across normal and compact requests, preserving prompt-cache prefixes while rejecting calls outside active compaction.

## 0.152.0

- Share chat history across git worktrees of the same repo, and merge workspace cache writes from concurrent servers instead of overwriting. #558
Expand Down
28 changes: 13 additions & 15 deletions src/eca/features/tools/chat.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,25 @@
(set! *warn-on-reflection* true)

(defn ^:private compact-chat [arguments {:keys [db* chat-id]}]
(let [summary (get arguments "summary")]
;; Mark chat as not compacting anymore
(swap! db* assoc-in [:chats chat-id :compacting?] false)

;; Save summary to replace chat history later
(swap! db* assoc-in [:chats chat-id :last-summary] summary)

;; Signal that compact is done so the LLM loop stops
(swap! db* assoc-in [:chats chat-id :compact-done?] true)

(tools.util/single-text-content "Compacted successfully!")))
(let [chat (get-in @db* [:chats chat-id])]
(if (or (:compacting? chat) (:auto-compacting? chat))
(do
(swap! db* update-in [:chats chat-id]
assoc
:compacting? false
:last-summary (get arguments "summary")
:compact-done? true)
(tools.util/single-text-content "Compacted successfully!"))
(tools.util/single-text-content
"Chat compaction is not active for this request. This tool is available only while chat compaction is in progress. To compact manually, the user must use the `/compact` command; compaction may also start automatically when context usage reaches the configured threshold."
:error))))

(def definitions
{"compact_chat"
{:description "Compact / summarize a chat, cleaning chat history, emptying usage and presenting the summary to user"
{:description "During chat compaction, submit a summary that will become the active conversation context"
:parameters {:type "object"
:properties {"summary" {:type "string"
:description "The summary/compacted text"}}
:required ["summary"]}
:handler #'compact-chat
:enabled-fn (fn [{:keys [db chat-id]}]
(or (get-in db [:chats chat-id :compacting?] false)
(get-in db [:chats chat-id :auto-compacting?] false)))
:summary-fn (constantly "Compacting...")}})
41 changes: 41 additions & 0 deletions test/eca/features/chat_test.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1238,6 +1238,47 @@
{:role :system :content {:state :finished :type :progress}}])}
(h/messages))))))

(deftest inactive-compact-tool-error-reaches-provider-continuation-test
(testing "inactive compact_chat result is included in the next provider request"
(h/reset-components!)
(let [continuation* (atom nil)
original-all-tools f.tools/all-tools
original-call-tool! f.tools/call-tool!
expected-error "Chat compaction is not active for this request. This tool is available only while chat compaction is in progress. To compact manually, the user must use the `/compact` command; compaction may also start automatically when context usage reaches the configured threshold."
{:keys [chat-id]}
(prompt!
{:message "Continue normally"}
{:all-tools-mock original-all-tools
:call-tool-mock original-call-tool!
:api-mock
(fn [{:keys [on-first-response-received on-message-received
on-prepare-tool-call on-tools-called]}]
(on-first-response-received)
(on-prepare-tool-call {:id "compact-call-1"
:full-name "eca__compact_chat"
:arguments-text "{\"summary\":\"Must not be stored\"}"})
(reset! continuation*
(on-tools-called [{:id "compact-call-1"
:full-name "eca__compact_chat"
:arguments {"summary" "Must not be stored"}}]))
(on-message-received {:type :text :text "Understood"})
(on-message-received {:type :finish}))})
tool-output (->> (:new-messages @continuation*)
(filter #(= "tool_call_output" (:role %)))
last
:content)]
(is (match? {:id "compact-call-1"
:full-name "eca__compact_chat"
:error true
:output {:error true
:contents [{:type :text :text expected-error}]}}
tool-output)
"the provider continuation must contain the exact inactive-compaction error for the LLM")
(is (nil? (get-in (h/db) [:chats chat-id :last-summary])))
(is (nil? (get-in (h/db) [:chats chat-id :compact-done?])))
(is (not (true? (get-in (h/db) [:chats chat-id :compacting?])))
"the accidental call must not activate or complete compaction"))))

(deftest concurrent-tool-calls-test
(testing "Running three calls simultaneously"
(h/reset-components!)
Expand Down
61 changes: 29 additions & 32 deletions test/eca/features/tools/chat_test.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,19 @@
(is (= true (:compact-done? chat-state))
"Should set compact-done? to true"))))))

(testing "Successfully compacts an auto-compacting chat"
(let [db* (h/db*)
chat-id "test-chat-auto-compacting"
test-summary "Auto-compacted summary"]
(swap! db* assoc-in [:chats chat-id :auto-compacting?] true)
(let [result ((get-in f.tools.chat/definitions ["compact_chat" :handler])
{"summary" test-summary}
{:db* db* :chat-id chat-id})
chat-state (get-in @db* [:chats chat-id])]
(is (false? (:error result)))
(is (= test-summary (:last-summary chat-state)))
(is (true? (:compact-done? chat-state))))))

(testing "Handles empty summary"
(let [db* (h/db*)
chat-id "test-chat-456"
Expand All @@ -50,37 +63,20 @@
(is (= empty-summary (:last-summary chat-state)))
(is (= true (:compact-done? chat-state))))))))

(deftest compact-chat-enabled-test
(testing "Tool is enabled when chat is compacting"
(let [db* (h/db*)
chat-id "test-chat-compacting"]
(swap! db* assoc-in [:chats chat-id :compacting?] true)

(is (true? ((get-in f.tools.chat/definitions ["compact_chat" :enabled-fn])
{:db @db* :chat-id chat-id})))))

(testing "Tool is disabled when chat is not compacting"
(let [db* (h/db*)
chat-id "test-chat-not-compacting"]
(swap! db* assoc-in [:chats chat-id :compacting?] false)

(is (false? ((get-in f.tools.chat/definitions ["compact_chat" :enabled-fn])
{:db @db* :chat-id chat-id})))))

(testing "Tool is disabled when compacting? is not set (defaults to false)"
(let [db* (h/db*)
chat-id "test-chat-no-compacting-key"]
;; Don't set compacting? at all

(is (false? ((get-in f.tools.chat/definitions ["compact_chat" :enabled-fn])
{:db @db* :chat-id chat-id})))))

(testing "Tool is disabled when chat doesn't exist"
(let [db* (h/db*)
chat-id "non-existent-chat"]

(is (false? ((get-in f.tools.chat/definitions ["compact_chat" :enabled-fn])
{:db @db* :chat-id chat-id}))))))
(deftest compact-chat-requires-active-compaction-test
(let [db* (h/db*)
chat-id "test-chat-not-compacting"
handler (get-in f.tools.chat/definitions ["compact_chat" :handler])]
(swap! db* assoc-in [:chats chat-id] {:id chat-id})
(let [before @db*
result (handler {"summary" "Must not be stored"}
{:db* db* :chat-id chat-id})]
(is (match? {:error true
:contents [{:type :text
:text "Chat compaction is not active for this request. This tool is available only while chat compaction is in progress. To compact manually, the user must use the `/compact` command; compaction may also start automatically when context usage reaches the configured threshold."}]}
result))
(is (= before @db*)
"Inactive compact tool calls must not mutate chat state"))))

(deftest compact-chat-summary-fn-test
(testing "Summary function returns constant string"
Expand All @@ -93,7 +89,8 @@
(is (string? (:description tool-def)) "Should have a description")
(is (map? (:parameters tool-def)) "Should have parameters")
(is (or (fn? (:handler tool-def)) (var? (:handler tool-def))) "Should have a handler function or var")
(is (fn? (:enabled-fn tool-def)) "Should have an enabled-fn")
(is (not (contains? tool-def :enabled-fn))
"Tool availability must not change the provider tool schema")
(is (fn? (:summary-fn tool-def)) "Should have a summary-fn")))

(testing "Tool parameters schema is correct"
Expand Down
11 changes: 11 additions & 0 deletions test/eca/features/tools_test.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,17 @@
:origin :native}])
(f.tools/all-tools "123" "code" {} {}))))

(testing "Compact tool schema is stable across compaction state"
(let [db {:chats {"123" {}}}
wire-tools (fn [db]
(mapv #(select-keys % [:full-name :description :parameters])
(f.tools/all-tools "123" "code" db {})))
normal-tools (wire-tools db)
manual-tools (wire-tools (assoc-in db [:chats "123" :compacting?] true))
auto-tools (wire-tools (assoc-in db [:chats "123" :auto-compacting?] true))]
(is (some #(= "eca__compact_chat" (:full-name %)) normal-tools))
(is (= normal-tools manual-tools auto-tools))))

(testing "Subagent excludes spawn_agent, task, git, and ask_user tools"
(let [db {:chats {"sub-1" {:subagent {:name "explorer"}}}}
tools (f.tools/all-tools "sub-1" "code" db {})
Expand Down
Loading