diff --git a/CHANGELOG.md b/CHANGELOG.md index d89bebad6..0385456eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Unreleased - Add `toolCall.askUser.trustModeTimeoutSeconds`: in trust mode, an unanswered `ask_user` question times out and the LLM continues with its best judgment. +- Speed up rollback, flags and forks on long chats via the `incrementalUpdates` capability: `chat/truncated`, positioned flags, and paged `/fork`, `/btw` and `chat/fork`. ## 0.165.0 diff --git a/docs/protocol.md b/docs/protocol.md index 222e33143..70ab3ffda 100644 --- a/docs/protocol.md +++ b/docs/protocol.md @@ -235,6 +235,24 @@ interface ClientCapabilities { * during a chat session. */ askQuestion?: boolean; + + /** + * Present when the client applies chat history changes in place + * instead of needing a full `chat/cleared` + replay. Then + * `chat/rollback` emits `chat/truncated`, `chat/addFlag` emits a + * single flag `chat/contentReceived` carrying `afterContentId`, and + * chats the server creates (`/fork`, `/btw`, `chat/fork`) stream + * only their newest messages, with `chat/opened` `history` + * describing the rest. Remote (SSE) clients keep receiving the + * full replays. + */ + incrementalUpdates?: { + /** + * Max messages to stream into a chat the server creates. + * Defaults to 50. + */ + historyPageSize?: number; + }; } } } @@ -1536,6 +1554,15 @@ interface ChatFlagContent { * Unique identifier for this flag. */ contentId: string; + + /** + * Id of the message the flag belongs after (message contentId or + * content id). Only sent by `chat/addFlag` to clients with the + * `incrementalUpdates` capability; when absent, render the flag at the + * current position. Clients may skip rendering when that message is not + * loaded (e.g. on an unloaded history page). + */ + afterContentId?: string; } ``` @@ -1834,7 +1861,8 @@ interface ChatPromptSteerRemoveParams { A client request to rollback chat messages to before a specific user sent message using `contentId`. Clients should show an option close to user sent messages in chat to rollback, calling this method. Server will then remove the messages from its memory after that contentId and produce `chat/cleared` followed -with `chat/contentReceived` with the kept messages. +with `chat/contentReceived` with the kept messages. Clients with the `incrementalUpdates` capability receive +a single `chat/truncated` instead. _Request:_ @@ -1907,6 +1935,8 @@ The flag is inserted after the message identified by the given `contentId`. The `contentId` matches against both message-level `contentId` (user messages) and content `id` fields (tool calls, reasons, etc), allowing placement between any message types. Flags are persisted as messages and propagate automatically with fork and resume. +Server then emits `chat/cleared` followed by the full history via `chat/contentReceived`, or, for clients with +the `incrementalUpdates` capability, a single flag `chat/contentReceived` carrying `afterContentId`. _Request:_ @@ -1974,7 +2004,9 @@ interface ChatRemoveFlagResponse {} A client request to fork the chat into a new chat with messages up to and including the message identified by the given `contentId`. The server creates the new chat, sends a `chat/opened` notification with the new chat, then streams the kept messages -via `chat/contentReceived`. A system message is also sent to the original chat. +(only the newest page of them for clients with the `incrementalUpdates` capability) via +`chat/contentReceived`. The fork always keeps the full history. A system message is +also sent to the original chat. _Request:_ @@ -2004,6 +2036,12 @@ interface ChatForkResponse { * Absent when the given contentId was not found. */ chatId?: string; + + /** + * Pagination info for the streamed messages, present when the client has + * the `incrementalUpdates` capability. Same as `chat/opened` `history`. + */ + meta?: ChatHistoryMeta; } ``` @@ -2031,12 +2069,38 @@ interface ChatClearedParams { } ``` +### Chat truncated (⬅️) + +A server notification to remove a message and everything after it from a chat UI, keeping the earlier +messages as they are. Only sent to clients with the `incrementalUpdates` capability, currently after +`chat/rollback`. When the message is not loaded in the client (e.g. it is on an unloaded history page), +clients should re-open the chat via `chat/open`. + +_Notification:_ + +* method: `chat/truncated` +* params: `ChatTruncatedParams` defined as follows: + +```typescript +interface ChatTruncatedParams { + /** + * The chat session identifier. + */ + chatId: string; + + /** + * The contentId of the first removed message. + */ + contentId: string; +} +``` + ### Chat opened (⬅️) A server notification indicating that a chat exists or was just materialized server-side. Emitted when: -- a chat is forked via `/fork` or the `chat/fork` request, +- a chat is forked via `/fork`, `/btw` or the `chat/fork` request, - a persisted chat is replayed via `chat/open`, - a client-supplied `chatId` is seen for the first time on `chat/prompt` (the legacy null-`chatId` path does not emit this). @@ -2064,6 +2128,14 @@ interface ChatOpenedParams { * not yet have a title. */ title?: string; + + /** + * Pagination info when only the newest page of the chat's messages + * follows, for chats the server creates (`/fork`, `/btw`, `chat/fork`) + * for clients with the `incrementalUpdates` capability. Older messages + * can be paged via `chat/history` using `beforeCursor`. + */ + history?: ChatHistoryMeta; } ``` @@ -2346,8 +2418,10 @@ the selected window; an opaque `after` cursor pages forward instead. The literal sentinel `"lastCompaction"` may be passed as `before`/`after`: `after: "lastCompaction"` returns the messages since the last compaction (the active context), `before: "lastCompaction"` the summarized-away history. Cursors are ephemeral — -a cursor whose message no longer exists (e.g. after a rollback or clear) returns -a `cursor_expired` error, prompting the client to refetch the latest page. +a cursor whose message no longer exists returns a `cursor_expired` error, +prompting the client to refetch the latest page. The exception is a cursor whose +message was cut off by a rollback: everything left is older than it, so a +`before` page returns the newest remaining messages. _Request:_ diff --git a/src/eca/db.clj b/src/eca/db.clj index 221fa92fe..11825de58 100644 --- a/src/eca/db.clj +++ b/src/eca/db.clj @@ -41,7 +41,8 @@ :client-capabilities {:code-assistant {:editor {:diagnostics :boolean :definition :boolean :references :boolean} - :chat-capabilities {:ask-question :boolean}}} + :chat-capabilities {:ask-question :boolean + :incremental-updates {:history-page-size :number}}}} :config-hash :string :providers-config-hash :string :last-config-notified ::any-map diff --git a/src/eca/features/chat.clj b/src/eca/features/chat.clj index 420c195e2..c8cb39d74 100644 --- a/src/eca/features/chat.clj +++ b/src/eca/features/chat.clj @@ -2632,7 +2632,8 @@ (defn rollback-chat "Remove messages from chat in db until content-id matches. - Then notify to clear chat and then the kept messages." + Then notify the client: `chat/truncated` when it supports incremental + updates, otherwise `chat/cleared` followed by the kept messages." [{:keys [chat-id content-id include]} db* messenger metrics] (db/hydrate-chat! db* chat-id metrics) (let [include (if (seq include) @@ -2661,15 +2662,20 @@ ;; state. Persist immediately so the cleaned-up history survives a ;; restart instead of relying on the next unrelated save. (db/save-chat! @db* chat-id metrics) - (messenger/chat-cleared - messenger - {:chat-id chat-id - :messages true}) - (send-chat-contents! - new-messages - {:chat-id chat-id - :db* db* - :messenger messenger})) + (if (history/incremental-updates? @db*) + (when (< (count new-messages) (count all-messages)) + (messenger/chat-truncated messenger {:chat-id chat-id + :content-id content-id})) + (do + (messenger/chat-cleared + messenger + {:chat-id chat-id + :messages true}) + (send-chat-contents! + new-messages + {:chat-id chat-id + :db* db* + :messenger messenger})))) {})) (defn ^:private find-last-message-idx @@ -2696,7 +2702,9 @@ "Add a named flag after the message identified by content-id. Searches both :content-id and [:content :id] to support placement after any message type (user, tool call, reason, etc). - Clears and replays the chat to render the flag at the correct position." + Clients supporting incremental updates receive only the flag, with + `afterContentId` telling where to render it; others get a clear and + full replay so the flag renders at the correct position." [{:keys [chat-id content-id text]} db* messenger metrics] (db/hydrate-chat! db* chat-id metrics) (let [messages (vec (get-in @db* [:chats chat-id :messages])) @@ -2709,8 +2717,17 @@ (cons flag-msg (subvec messages insert-after)))] (swap! db* assoc-in [:chats chat-id :messages] new-messages) (db/save-chat! @db* chat-id metrics) - (messenger/chat-cleared messenger {:chat-id chat-id :messages true}) - (send-chat-contents! new-messages {:chat-id chat-id :db* db* :messenger messenger}))) + (if (history/incremental-updates? @db*) + (messenger/chat-content-received messenger + {:chat-id chat-id + :role :system + :content {:type :flag + :text text + :contentId flag-id + :afterContentId content-id}}) + (do + (messenger/chat-cleared messenger {:chat-id chat-id :messages true}) + (send-chat-contents! new-messages {:chat-id chat-id :db* db* :messenger messenger}))))) {})) (defn remove-flag @@ -2728,7 +2745,10 @@ (defn fork-chat "Fork the chat creating a new chat with messages up to and including - the message identified by content-id." + the message identified by content-id. + Clients supporting incremental updates get only the newest page of kept + messages; the paging meta is returned as `:meta` and in `chat/opened` so + the client can page older via `chat/history`." [{:keys [chat-id content-id]} db* messenger metrics] (db/hydrate-chat! db* chat-id metrics) (let [chat (get-in @db* [:chats chat-id]) @@ -2748,19 +2768,23 @@ :model (:model chat) :last-api (:last-api chat) :messages kept-messages - :prompt-finished? true}] + :prompt-finished? true} + page (history/server-created-page @db* kept-messages) + meta (when page (history/page-meta page kept-messages))] (swap! db* assoc-in [:chats new-id] new-chat) (mark-editor-open! db* new-id) (db/save-chat! @db* new-id metrics) - (messenger/chat-opened messenger {:chat-id new-id :title new-title}) - (send-chat-contents! kept-messages {:chat-id new-id :db* db* :messenger messenger}) + (messenger/chat-opened messenger (assoc-some {:chat-id new-id :title new-title} + :history meta)) + (send-chat-contents! (if page (:messages page) kept-messages) + {:chat-id new-id :db* db* :messenger messenger}) (lifecycle/send-content! {:messenger messenger :chat-id new-id} :system (assoc-some {:type :metadata} :title new-title)) (lifecycle/send-content! {:messenger messenger :chat-id chat-id} :system {:type :text :text (str "Chat forked to: " new-title)}) - {:chat-id new-id})))) + (assoc-some {:chat-id new-id} :meta meta))))) (defn list-chats "Pure projection over `(:chats db)`: returns a summary list intended for the @@ -2852,11 +2876,7 @@ :title title :selection selection} :meta (when windowed? - {:total (:total page) - :returned (:returned page) - :before-cursor (:before-cursor page) - :after-cursor (:after-cursor page) - :compaction-cursor (history/compaction-cursor (:messages chat))})))))))) + (history/page-meta page (:messages chat)))))))))) (defn fetch-history "Window a chat's persisted messages and return the transformed content items @@ -2880,8 +2900,4 @@ :message "Cursor no longer points to an existing message; refetch the latest page"}} {:contents (messages->contents (:messages result) {:chat-id chat-id :parent-chat-id nil :db db}) - :meta {:total (:total result) - :returned (:returned result) - :before-cursor (:before-cursor result) - :after-cursor (:after-cursor result) - :compaction-cursor (history/compaction-cursor messages)}})))))) + :meta (history/page-meta result messages)})))))) diff --git a/src/eca/features/chat/history.clj b/src/eca/features/chat/history.clj index 29bc347bb..f255ccf76 100644 --- a/src/eca/features/chat/history.clj +++ b/src/eca/features/chat/history.clj @@ -6,7 +6,9 @@ Cursors are opaque tokens: base64url of \".\" where checksum fingerprints the message. Resolving trusts the encoded index when its checksum still matches, else rescans by checksum (tolerating shifts like flag - insert/remove); a cursor whose message is gone resolves to :expired." + insert/remove); a cursor whose message was cut off by truncating the history + resolves to its end, and any other cursor whose message is gone resolves to + :expired." (:require [clojure.string :as string]) (:import @@ -45,14 +47,19 @@ :else (recur (dec i))))) (defn ^:private resolve-cursor - "Resolves an opaque cursor to an index, or :expired if its message is gone." + "Resolves an opaque cursor to an index, or :expired if its message is gone. + A gone message whose index is past the end resolves to the end, as the + history was truncated at or before it (rollback) and every remaining + message is older." [messages ^String cursor] (if-let [{:keys [idx checksum]} (decode-cursor cursor)] - (if (and (<= 0 idx) (< idx (count messages)) - (= checksum (message-checksum (nth messages idx)))) - idx - (or (first (keep-indexed (fn [i m] (when (= checksum (message-checksum m)) i)) messages)) - :expired)) + (let [n (count messages)] + (if (and (<= 0 idx) (< idx n) + (= checksum (message-checksum (nth messages idx)))) + idx + (or (first (keep-indexed (fn [i m] (when (= checksum (message-checksum m)) i)) messages)) + (when (>= idx n) n) + :expired))) :expired)) (defn ^:private resolve-bound @@ -140,3 +147,41 @@ [messages] (when-let [idx (last-compaction-idx (vec messages))] (encode-cursor idx (nth (vec messages) idx)))) + +(defn page-meta + "Pagination meta for a `window-messages` `page` of `messages`." + [page messages] + {:total (:total page) + :returned (:returned page) + :before-cursor (:before-cursor page) + :after-cursor (:after-cursor page) + :compaction-cursor (compaction-cursor messages)}) + +(def default-history-page-size + "Messages sent when the server creates a chat for a client supporting + incremental updates that did not declare a `historyPageSize`." + 50) + +(defn ^:private incremental-updates-capability [db] + (get-in db [:client-capabilities :code-assistant :chat-capabilities :incremental-updates])) + +(defn incremental-updates? + "Whether the client applies chat history changes in place and pages + server-created chats instead of needing full replays." + [db] + (boolean (incremental-updates-capability db))) + +(defn history-page-size + "Number of messages to send for a chat the server creates, as declared by + the client's `incrementalUpdates` capability." + [db] + (let [capability (incremental-updates-capability db)] + (or (coerce-limit (when (map? capability) (:history-page-size capability))) + default-history-page-size))) + +(defn server-created-page + "The newest page of `messages` to send for a chat the server creates, or nil + when the client needs the full history." + [db messages] + (when (incremental-updates? db) + (window-messages messages {:limit (history-page-size db)}))) diff --git a/src/eca/features/commands.clj b/src/eca/features/commands.clj index fc5a777f5..3b577394a 100644 --- a/src/eca/features/commands.clj +++ b/src/eca/features/commands.clj @@ -9,6 +9,7 @@ [eca.db :as db] [eca.features.chat.debug :as f.chat.debug] [eca.features.chat.export :as f.chat.export] + [eca.features.chat.history :as history] [eca.features.chat.lifecycle :as lifecycle] [eca.features.context :as f.context] [eca.features.index :as f.index] @@ -72,7 +73,9 @@ (defn ^:private fork-chat! "Creates a new chat copying `chat`'s history (sanitized) and base settings plus `extra-copied-keys`, persists it and notifies the client. - Returns the new chat." + Returns `{:chat new-chat :replay-messages msgs}`, where `msgs` are the + messages to send to the client: only the newest page for clients + supporting incremental updates, announced in `chat/opened` `:history`." [chat new-title extra-copied-keys {:keys [db* messenger metrics]}] (let [new-id (str (random-uuid)) now (System/currentTimeMillis) @@ -85,11 +88,17 @@ :last-api (:last-api chat) :messages (drop-dangling-tool-calls (vec (:messages chat))) :prompt-finished? true} - (select-keys chat extra-copied-keys))] + (select-keys chat extra-copied-keys)) + messages (:messages new-chat) + page (history/server-created-page @db* messages)] (swap! db* assoc-in [:chats new-id] new-chat) (db/save-chat! @db* new-id metrics) - (messenger/chat-opened messenger {:chat-id new-id :title new-title}) - new-chat)) + (messenger/chat-opened messenger + (shared/assoc-some {:chat-id new-id :title new-title} + :history (when page + (history/page-meta page messages)))) + {:chat new-chat + :replay-messages (if page (:messages page) messages)})) (defn ^:private normalize-command-name [f] (string/lower-case (fs/strip-ext (fs/file-name f)))) @@ -827,7 +836,7 @@ (config/notify-selected-agent-changed! selected-agent db* messenger chat-id) (chat-message (str "Selected agent: `" selected-agent "`."))))) "fork" (let [chat (get-in db [:chats chat-id]) - {new-id :id new-title :title new-messages :messages} + {{new-id :id new-title :title} :chat new-messages :replay-messages} (fork-chat! chat (fork-title (:title chat)) [] chat-ctx)] {:type :chat-messages :chats {new-id {:messages new-messages @@ -842,7 +851,7 @@ {:type :side-prompt :notice-text "Usage: /btw . Asks a side question in a forked chat, keeping this chat history clean."} (let [chat (get-in @db* [:chats chat-id]) - {new-id :id new-title :title new-messages :messages} + {{new-id :id new-title :title} :chat new-messages :replay-messages} (fork-chat! chat (btw-title question) [:variant :trust :remembered-approvals :prompt-cache :startup-context] chat-ctx)] diff --git a/src/eca/messenger.clj b/src/eca/messenger.clj index 3abe1c873..767d8a324 100644 --- a/src/eca/messenger.clj +++ b/src/eca/messenger.clj @@ -7,6 +7,7 @@ (defprotocol IMessenger (chat-content-received [this data]) (chat-cleared [this params]) + (chat-truncated [this params]) (chat-status-changed [this params]) (chat-deleted [this params]) (chat-opened [this params]) diff --git a/src/eca/remote/messenger.clj b/src/eca/remote/messenger.clj index a4789a3bc..29b2ce987 100644 --- a/src/eca/remote/messenger.clj +++ b/src/eca/remote/messenger.clj @@ -1,7 +1,12 @@ (ns eca.remote.messenger "BroadcastMessenger wraps an inner IMessenger (typically ServerMessenger) - and broadcasts events to all connected SSE clients." + and broadcasts events to all connected SSE clients. + + The `incrementalUpdates` capability is declared by the editor only, so SSE + clients get the full-replay equivalent of incremental notifications + (truncation, positioned flags and paged server-created chats)." (:require + [eca.features.chat :as f.chat] [eca.messenger :as messenger] [eca.remote.sse :as sse] [eca.shared :as shared])) @@ -11,6 +16,26 @@ (defn ^:private ->camel [data] (shared/map->camel-cased-map data)) +(defn ^:private broadcast-messages! + "Broadcasts `messages` of `chat-id` as SSE content events." + [sse-connections* db chat-id messages] + ;; Skip building the payloads when nobody listens: replays cost O(chat). + (when (seq @sse-connections*) + (doseq [payload (f.chat/messages->contents messages {:chat-id chat-id :db db})] + (sse/broadcast! sse-connections* "chat:content-received" (->camel payload))))) + +(defn ^:private broadcast-replay! + "Broadcasts a clear of `chat-id` followed by its whole history, as clients + without incremental updates expect after a history change." + [sse-connections* db chat-id] + (sse/broadcast! sse-connections* "chat:cleared" (->camel {:chat-id chat-id :messages true})) + (broadcast-messages! sse-connections* db chat-id (get-in db [:chats chat-id :messages]))) + +(defn ^:private positioned-flag? [{:keys [content parent-chat-id]}] + (and (nil? parent-chat-id) + (= "flag" (name (or (:type content) ""))) + (some? (:afterContentId content)))) + (defn ^:private cancel-handle! [handle] (when (future? handle) ;; PendingRequest cancellation sends $/cancelRequest; a transport failure @@ -71,17 +96,25 @@ @result (cancel-handle! handle)))) -(defrecord BroadcastMessenger [inner sse-connections* pending-questions*] +(defrecord BroadcastMessenger [inner sse-connections* pending-questions* db*] messenger/IMessenger (chat-content-received [_this data] (messenger/chat-content-received inner data) - (sse/broadcast! sse-connections* "chat:content-received" (->camel data))) + (if (and db* (positioned-flag? data)) + (broadcast-replay! sse-connections* @db* (:chat-id data)) + (sse/broadcast! sse-connections* "chat:content-received" (->camel data)))) (chat-cleared [_this params] (messenger/chat-cleared inner params) (sse/broadcast! sse-connections* "chat:cleared" (->camel params))) + (chat-truncated [_this params] + (messenger/chat-truncated inner params) + (if db* + (broadcast-replay! sse-connections* @db* (:chat-id params)) + (sse/broadcast! sse-connections* "chat:truncated" (->camel params)))) + (chat-status-changed [_this params] (messenger/chat-status-changed inner params) (sse/broadcast! sse-connections* "chat:status-changed" (->camel params))) @@ -92,7 +125,18 @@ (chat-opened [_this params] (messenger/chat-opened inner params) - (sse/broadcast! sse-connections* "chat:opened" (->camel params))) + (if-let [{:keys [total returned]} (:history params)] + ;; Only the newest page follows; send the older messages first so SSE + ;; clients get the whole history in order. + (let [{:keys [chat-id]} params] + (sse/broadcast! sse-connections* "chat:opened" (->camel (dissoc params :history))) + (when db* + (let [db @db* + messages (vec (get-in db [:chats chat-id :messages]))] + (broadcast-messages! sse-connections* db chat-id + (subvec messages 0 (max 0 (min (count messages) + (- total returned)))))))) + (sse/broadcast! sse-connections* "chat:opened" (->camel params)))) (rewrite-content-received [_this data] (messenger/rewrite-content-received inner data)) @@ -159,9 +203,12 @@ (defn make-broadcast-messenger "Creates a BroadcastMessenger with a fresh pending-questions registry. Prefer this over `->BroadcastMessenger` so callers don't have to know - about the internal registry atom." - [inner sse-connections*] - (->BroadcastMessenger inner sse-connections* (atom {}))) + about the internal registry atom. `db*` is read to give SSE clients full + replays in place of incremental notifications." + ([inner sse-connections*] + (make-broadcast-messenger inner sse-connections* nil)) + ([inner sse-connections* db*] + (->BroadcastMessenger inner sse-connections* (atom {}) db*))) (defn answer-question! "Resolves a pending question by request-id: delivers diff --git a/src/eca/server.clj b/src/eca/server.clj index 1c861cc64..50af64f25 100644 --- a/src/eca/server.clj +++ b/src/eca/server.clj @@ -288,6 +288,9 @@ (chat-cleared [_this params] (jsonrpc.server/discarding-stdout (jsonrpc.server/send-notification server "chat/cleared" params))) + (chat-truncated [_this params] + (jsonrpc.server/discarding-stdout + (jsonrpc.server/send-notification server "chat/truncated" params))) (chat-status-changed [_this params] (jsonrpc.server/discarding-stdout (jsonrpc.server/send-notification server "chat/statusChanged" params))) @@ -348,7 +351,7 @@ ;; HTTP server can be started later (e.g. when local project config ;; enables it after initialize). Broadcasting to an empty set is a no-op. sse-connections* (atom #{}) - messenger (remote.messenger/make-broadcast-messenger stdio-messenger sse-connections*) + messenger (remote.messenger/make-broadcast-messenger stdio-messenger sse-connections* db*) start-remote-server! (fn [components] (when-let [rs (remote.server/start! components sse-connections*)] diff --git a/test/eca/features/chat/history_test.clj b/test/eca/features/chat/history_test.clj index 745e9f205..2de403a99 100644 --- a/test/eca/features/chat/history_test.clj +++ b/test/eca/features/chat/history_test.clj @@ -51,7 +51,20 @@ (testing "expired cursor returns error" (let [r (f.chat.history/window-messages messages {:before "not-a-real-cursor"})] - (is (= :cursor-expired (:error r)))))) + (is (= :cursor-expired (:error r))))) + + (testing "a before cursor whose message was truncated away pages the rest" + (let [before (:before-cursor (f.chat.history/window-messages messages {:limit 3})) + ;; The cursor points at m2; a rollback to m2 truncates m2 onwards. + r (f.chat.history/window-messages (subvec messages 0 2) {:limit 3 :before before})] + (is (= ["m0" "m1"] (texts r))) + (is (nil? (:before-cursor r))))) + + (testing "a cursor whose message was removed mid-history still expires" + (let [before (:before-cursor (f.chat.history/window-messages messages {:limit 3})) + without-m2 (into (subvec messages 0 2) (subvec messages 3))] + (is (= :cursor-expired + (:error (f.chat.history/window-messages without-m2 {:before before}))))))) (deftest window-messages-keeps-reason-runs-test (let [reason (fn [id created-at] {:role "reason" :content {:id id :text id} :created-at created-at}) diff --git a/test/eca/features/chat_test.clj b/test/eca/features/chat_test.clj index 0ceba52aa..ac8cf19da 100644 --- a/test/eca/features/chat_test.clj +++ b/test/eca/features/chat_test.clj @@ -6,6 +6,7 @@ [eca.config :as config] [eca.db :as db] [eca.features.chat :as f.chat] + [eca.features.chat.history :as history] [eca.features.chat.lifecycle :as lifecycle] [eca.features.context :as f.context] [eca.features.index :as f.index] @@ -2380,6 +2381,93 @@ :role "assistant"}]} (h/messages))))))) +(defn ^:private enable-incremental-updates! + ([] (enable-incremental-updates! {})) + ([capability] + (swap! (h/db*) assoc-in + [:client-capabilities :code-assistant :chat-capabilities :incremental-updates] + capability))) + +(defn ^:private two-turn-chat! [] + (let [api-mock (fn [reply] + (fn [{:keys [on-first-response-received on-message-received]}] + (on-first-response-received {:type :text :text reply}) + (on-message-received {:type :text :text reply}) + (on-message-received {:type :finish}))) + {:keys [chat-id]} (prompt! {:message "1"} + {:all-tools-mock (constantly []) + :api-mock (api-mock "2")})] + (prompt! {:message "3" :chat-id chat-id} + {:all-tools-mock (constantly []) + :api-mock (api-mock "4")}) + chat-id)) + +(deftest rollback-chat-incremental-updates-test + (testing "Clients with incrementalUpdates get chat/truncated instead of a replay" + (h/reset-components!) + (enable-incremental-updates!) + (let [chat-id (two-turn-chat!) + second-content-id (get-in (h/db) [:chats chat-id :messages 2 :content-id])] + (h/reset-messenger!) + (f.chat/rollback-chat {:chat-id chat-id + :include ["messages"] + :content-id second-content-id} + (h/db*) (h/messenger) (h/metrics)) + (is (= 2 (count (get-in (h/db) [:chats chat-id :messages])))) + (is (= {:chat-truncated [{:chat-id chat-id :content-id second-content-id}]} + (h/messages))))) + + (testing "Unknown content-id changes nothing and sends nothing" + (h/reset-components!) + (enable-incremental-updates!) + (let [chat-id (two-turn-chat!)] + (h/reset-messenger!) + (f.chat/rollback-chat {:chat-id chat-id + :include ["messages"] + :content-id "missing"} + (h/db*) (h/messenger) (h/metrics)) + (is (= 4 (count (get-in (h/db) [:chats chat-id :messages])))) + (is (= {} (h/messages)))))) + +(deftest add-flag-test + (testing "Legacy clients get chat/cleared and a full replay including the flag" + (h/reset-components!) + (let [chat-id (two-turn-chat!) + first-content-id (get-in (h/db) [:chats chat-id :messages 0 :content-id])] + (h/reset-messenger!) + (f.chat/add-flag {:chat-id chat-id :content-id first-content-id :text "here"} + (h/db*) (h/messenger) (h/metrics)) + (is (match? [{:role "user"} {:role "flag" :content {:text "here"}} + {:role "assistant"} {:role "user"} {:role "assistant"}] + (get-in (h/db) [:chats chat-id :messages]))) + (is (match? {:chat-clear [{:chat-id chat-id :messages true}] + :chat-content-received + [{:role "user"} + {:role :system :content {:type :flag :text "here"}} + {:role "assistant"} + {:role "user"} + {:role "assistant"}]} + (h/messages))))) + + (testing "Clients with incrementalUpdates get only the positioned flag" + (h/reset-components!) + (enable-incremental-updates!) + (let [chat-id (two-turn-chat!) + first-content-id (get-in (h/db) [:chats chat-id :messages 0 :content-id])] + (h/reset-messenger!) + (f.chat/add-flag {:chat-id chat-id :content-id first-content-id :text "here"} + (h/db*) (h/messenger) (h/metrics)) + (let [flag-id (get-in (h/db) [:chats chat-id :messages 1 :content-id])] + (is (some? flag-id)) + (is (= {:chat-content-received + [{:chat-id chat-id + :role :system + :content {:type :flag + :text "here" + :contentId flag-id + :afterContentId first-content-id}}]} + (h/messages))))))) + (deftest prompt-cache-agent-switch-test (testing "Static prompt cache is rebuilt when switching agents within the same chat" (h/reset-components!) @@ -3521,6 +3609,66 @@ (h/messages))) (is (nil? (get-in (h/db) [:chats chat-id :reason-group])))))) +(deftest fork-chat-paging-test + (h/reset-components!) + (swap! (h/db*) assoc-in [:chats "c1"] + {:id "c1" + :title "Orig" + :messages (vec (for [i (range 5)] + {:role "user" + :content [{:type :text :text (str "q" i)}] + :content-id (str "m" i)}))}) + (testing "incremental clients get only the declared page size, with paging meta" + (h/reset-messenger!) + (enable-incremental-updates! {:history-page-size 2}) + (let [{:keys [chat-id meta]} (f.chat/fork-chat {:chat-id "c1" :content-id "m3"} + (h/db*) (h/messenger) (h/metrics))] + (is (match? [{:chat-id chat-id :history {:total 4 :returned 2}}] + (:chat-opened (h/messages)))) + (is (= 4 (count (get-in (h/db) [:chats chat-id :messages]))) + "the fork keeps the full history") + (is (= ["m2" "m3"] + (->> (:chat-content-received (h/messages)) + (filter #(= chat-id (:chat-id %))) + (keep #(get-in % [:content :content-id]))))) + (is (match? {:total 4 :returned 2 :before-cursor string? :after-cursor nil} + meta)) + (testing "the before cursor pages the older kept messages" + (is (match? {:contents [{:content {:content-id "m0"}} + {:content {:content-id "m1"}}]} + (f.chat/fetch-history {:chat-id chat-id + :before (:before-cursor meta)} + (h/db*))))))) + (testing "rolling the fork back to its first loaded message keeps older pages loadable" + (h/reset-messenger!) + (enable-incremental-updates! {:history-page-size 2}) + (let [{:keys [chat-id meta]} (f.chat/fork-chat {:chat-id "c1" :content-id "m3"} + (h/db*) (h/messenger) (h/metrics))] + (f.chat/rollback-chat {:chat-id chat-id :content-id "m2" :include ["messages"]} + (h/db*) (h/messenger) (h/metrics)) + (is (match? {:contents [{:content {:content-id "m0"}} + {:content {:content-id "m1"}}] + :meta {:before-cursor nil}} + (f.chat/fetch-history {:chat-id chat-id :before (:before-cursor meta)} + (h/db*)))))) + (testing "incremental clients without a page size get the default" + (h/reset-messenger!) + (with-redefs [history/default-history-page-size 3] + (enable-incremental-updates!) + (let [{:keys [meta]} (f.chat/fork-chat {:chat-id "c1" :content-id "m3"} + (h/db*) (h/messenger) (h/metrics))] + (is (match? {:total 4 :returned 3} meta)))) + (swap! (h/db*) update :client-capabilities dissoc :code-assistant)) + (testing "other clients get the whole kept history and no meta" + (h/reset-messenger!) + (let [{:keys [chat-id] :as res} (f.chat/fork-chat {:chat-id "c1" :content-id "m3"} + (h/db*) (h/messenger) (h/metrics))] + (is (not (contains? res :meta))) + (is (= ["m0" "m1" "m2" "m3"] + (->> (:chat-content-received (h/messages)) + (filter #(= chat-id (:chat-id %))) + (keep #(get-in % [:content :content-id])))))))) + (deftest tool-call-approve-on-unknown-chat-is-noop-test (testing "Approving a tool-call against an unknown chat does not throw or mutate state" (h/reset-components!) diff --git a/test/eca/features/commands_test.clj b/test/eca/features/commands_test.clj index 603ce141d..b346c5eae 100644 --- a/test/eca/features/commands_test.clj +++ b/test/eca/features/commands_test.clj @@ -9,7 +9,8 @@ [eca.features.rules :as f.rules] [eca.features.skills :as f.skills] [eca.shared :as shared] - [eca.test-helper :as h])) + [eca.test-helper :as h] + [matcher-combinators.test :refer [match?]])) (h/reset-components-before-test) @@ -420,6 +421,44 @@ (is (= [{:chat-id new-id :title "btw: who is foo?"}] (:chat-opened (h/messages)))))))) +(deftest fork-commands-page-history-test + (let [messages (vec (for [i (range 60)] + {:role "user" :content [{:type :text :text (str "q" i)}]})) + setup! (fn [incremental?] + (h/reset-components!) + (when incremental? + (swap! (h/db*) assoc-in + [:client-capabilities :code-assistant :chat-capabilities :incremental-updates] + {:history-page-size 50})) + (swap! (h/db*) assoc-in [:chats "chat-1"] + {:id "chat-1" :title "My chat" :messages messages}))] + (with-redefs [db/save-chat! (fn [& _])] + (testing "/fork sends only the newest page to incremental clients" + (setup! true) + (let [result (f.commands/handle-command! "fork" [] (command-context "chat-1")) + [new-id {new-messages :messages}] (first (dissoc (:chats result) "chat-1"))] + (is (= 60 (count (get-in (h/db) [:chats new-id :messages])))) + (is (= (subvec messages 10) new-messages)) + (is (match? [{:chat-id new-id + :history {:total 60 :returned 50 :before-cursor string?}}] + (:chat-opened (h/messages)))))) + + (testing "/btw sends only the newest page to incremental clients" + (setup! true) + (let [result (f.commands/handle-command! "btw" ["why?"] + (assoc (command-context "chat-1") + :message "/btw why?"))] + (is (= (subvec messages 10) (:target-messages result))) + (is (match? [{:history {:total 60 :returned 50}}] + (:chat-opened (h/messages)))))) + + (testing "other clients get the whole history and no history meta" + (setup! false) + (let [result (f.commands/handle-command! "fork" [] (command-context "chat-1")) + [_ {new-messages :messages}] (first (dissoc (:chats result) "chat-1"))] + (is (= messages new-messages)) + (is (not (contains? (first (:chat-opened (h/messages))) :history)))))))) + (deftest all-commands-include-model-command-test (let [commands (f.commands/all-commands {:workspace-folders []} {})] (is (some #(= {:name "model" diff --git a/test/eca/remote/handlers_test.clj b/test/eca/remote/handlers_test.clj index 215547dae..9684cbfa9 100644 --- a/test/eca/remote/handlers_test.clj +++ b/test/eca/remote/handlers_test.clj @@ -303,7 +303,9 @@ (testing "expired cursor returns 409" (swap! (h/db*) assoc-in [:chats "c1"] {:id "c1" :title "T" :status :idle :messages messages}) - (let [bogus (#'f.chat.history/encode-cursor 99 {:role "x" :content "gone" :created-at 999999}) + ;; In range but not matching any message (past-the-end cursors + ;; resolve to the end, as after a rollback). + (let [bogus (#'f.chat.history/encode-cursor 1 {:role "x" :content "gone" :created-at 999999}) response (handlers/handle-get-chat (components) {:params {:before bogus}} "c1") body (json/parse-string (:body response) true)] (is (= 409 (:status response))) diff --git a/test/eca/remote/messenger_test.clj b/test/eca/remote/messenger_test.clj index ae81a35b2..a29d201e3 100644 --- a/test/eca/remote/messenger_test.clj +++ b/test/eca/remote/messenger_test.clj @@ -61,6 +61,55 @@ (sse/close-all! sse-connections*))) +(deftest incremental-notifications-replay-for-sse-test + (let [user-msg (fn [i] {:role "user" :content [{:type :text :text (str "q" i)}] + :content-id (str "m" i)}) + inner (h/messenger) + sent (fn [k] (get @(:messages* inner) k)) + sse-connections* (sse/create-connections) + broadcast-messenger (remote.messenger/make-broadcast-messenger + inner sse-connections* (h/db*)) + sse-output (fn [f] + (let [os (java.io.ByteArrayOutputStream.) + client (sse/add-client! sse-connections* os)] + (f) + (Thread/sleep 100) + (sse/remove-client! sse-connections* client) + (.toString os "UTF-8")))] + (swap! (h/db*) assoc-in [:chats "c1"] + {:id "c1" :messages [(user-msg 0) (user-msg 1) (user-msg 2)]}) + + (testing "chat-truncated reaches the editor, SSE clients get a clear and replay" + (reset! (:messages* inner) {}) + (let [output (sse-output #(messenger/chat-truncated + broadcast-messenger {:chat-id "c1" :content-id "m3"}))] + (is (= [{:chat-id "c1" :content-id "m3"}] (sent :chat-truncated))) + (is (not (.contains output "chat:truncated"))) + (is (.contains output "chat:cleared")) + (is (every? #(.contains output %) ["q0" "q1" "q2"])))) + + (testing "a positioned flag reaches the editor, SSE clients get a clear and replay" + (reset! (:messages* inner) {}) + (let [data {:chat-id "c1" :role :system + :content {:type :flag :text "here" :contentId "f1" :afterContentId "m0"}} + output (sse-output #(messenger/chat-content-received broadcast-messenger data))] + (is (= [data] (sent :chat-content-received))) + (is (not (.contains output "afterContentId"))) + (is (.contains output "chat:cleared")) + (is (every? #(.contains output %) ["q0" "q1" "q2"])))) + + (testing "a paged chat-opened sends SSE clients the messages older than the page" + (reset! (:messages* inner) {}) + (let [params {:chat-id "c1" :title "Fork" :history {:total 3 :returned 1}} + output (sse-output #(messenger/chat-opened broadcast-messenger params))] + (is (= [params] (sent :chat-opened))) + (is (.contains output "chat:opened")) + (is (not (.contains output "history"))) + (is (every? #(.contains output %) ["q0" "q1"])) + (is (not (.contains output "q2")) "the page itself streams afterwards"))) + + (sse/close-all! sse-connections*))) + (deftest ask-question-broadcasts-and-resolves-via-answer-test (testing "ask-question registers a promise, broadcasts SSE, and answer-question! resolves it" (let [inner (h/messenger) diff --git a/test/eca/test_helper.clj b/test/eca/test_helper.clj index 3e811a6cb..b0c85b279 100644 --- a/test/eca/test_helper.clj +++ b/test/eca/test_helper.clj @@ -42,6 +42,7 @@ messenger/IMessenger (chat-content-received [_ data] (swap! messages* update :chat-content-received (fnil conj []) data)) (chat-cleared [_ params] (swap! messages* update :chat-clear (fnil conj []) params)) + (chat-truncated [_ params] (swap! messages* update :chat-truncated (fnil conj []) params)) (chat-status-changed [_ params] (swap! messages* update :chat-status-changed (fnil conj []) params)) (chat-deleted [_ params] (swap! messages* update :chat-deleted (fnil conj []) params)) (chat-opened [_ params] (swap! messages* update :chat-opened (fnil conj []) params))