branch: externals/ellama
commit 9352e29a3b3e85bccefd2d61626970004a0495fe
Merge: d3ede72b26 4cc6d2e87b
Author: Sergey Kostyaev <[email protected]>
Commit: GitHub <[email protected]>
Merge pull request #387 from s-kostyaev/fix-system-message-spam
Fix duplicate system message handling in chat prompts
---
AGENTS.md | 2 +
NEWS.org | 14 +++
ellama-tools.el | 82 ++++++++++++++---
ellama.el | 255 +++++++++++++++++++++++++++++++--------------------
tests/test-ellama.el | 99 ++++++++++++++++++++
5 files changed, 337 insertions(+), 115 deletions(-)
diff --git a/AGENTS.md b/AGENTS.md
index 1d1b1b3828..647bb3160d 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -17,6 +17,8 @@
- **Docstrings**: One‑line summary, then optional details; keep under 80 chars
per line. Do not add empty lines. If there are multiple sentences on one
line,
sentences should be separated by two spaces.
+ Use base verb form in function docstrings (no `-s`), e.g. `contain` not
+ `contains`, `return` not `returns`.
- **Comments**: Prefix with `;;` for buffer comments; avoid inline `#` clutter.
## Operation Guidelines
diff --git a/NEWS.org b/NEWS.org
index 6b80c149a0..5af8a452a3 100644
--- a/NEWS.org
+++ b/NEWS.org
@@ -1,3 +1,17 @@
+* Version 1.12.11
+- Fix duplicate system message handling in chat prompts. This change removes
the
+ redundant ~llm-chat-prompt-append-response~ call that was appending the
system
+ message on each interaction, ensuring the system message is included only
once
+ in the initial prompt context setup.
+- Fix bad src block replacement nil match crash that occurred when handling
+ certain source code blocks.
+- Warn LLM when reading binary content to prevent potential parsing errors and
+ improve robustness of tool integration.
+- Generalize docstring verb-form rule to apply consistently across
documentation
+ and code.
+- Implement automatic retry for tool call errors. This feature adds retry logic
+ to handle transient failures during tool usage by appending error messages to
+ the prompt and re-invoking requests, improving overall request reliability.
* Version 1.12.10
- Fix stack overflow in markdown code fence conversion in
~ellama--translate-markdown-to-org-filter~ by replacing expensive regular
diff --git a/ellama-tools.el b/ellama-tools.el
index a349f27492..d88d6fa0df 100644
--- a/ellama-tools.el
+++ b/ellama-tools.el
@@ -306,13 +306,38 @@ TOOL-PLIST is a property list in the format expected by
`llm-make-tool'."
(interactive)
(setq ellama-tools-enabled nil))
+(defun ellama-tools--string-has-raw-bytes-p (string)
+ "Return non-nil when STRING contain binary-like chars.
+Treat Emacs raw-byte chars and NUL bytes as binary-like."
+ (let ((idx 0)
+ (len (length string))
+ found)
+ (while (and (not found) (< idx len))
+ (when (or (> (aref string idx) #x10FFFF)
+ (= (aref string idx) 0))
+ (setq found t))
+ (setq idx (1+ idx)))
+ found))
+
+(defun ellama-tools--sanitize-tool-text-output (text label)
+ "Return TEXT or a warning when TEXT is binary-like.
+LABEL is used to identify the source in the warning."
+ (if (ellama-tools--string-has-raw-bytes-p text)
+ (concat label
+ " appears to contain binary data. Reading binary data as "
+ "text is a bad idea for this tool.")
+ text))
+
(defun ellama-tools-read-file-tool (file-name)
"Read the file FILE-NAME."
(json-encode (if (not (file-exists-p file-name))
(format "File %s doesn't exists." file-name)
- (with-temp-buffer
- (insert-file-contents-literally file-name)
- (buffer-string)))))
+ (let ((content (with-temp-buffer
+ (insert-file-contents file-name)
+ (buffer-string))))
+ (ellama-tools--sanitize-tool-text-output
+ content
+ (format "File %s" file-name))))))
(ellama-tools-define-tool
'(:function
@@ -522,17 +547,42 @@ Replace OLDCONTENT with NEWCONTENT."
(defun ellama-tools-shell-command-tool (callback cmd)
"Execute shell command CMD.
CALLBACK – function called once with the result string."
- (let ((buf (get-buffer-create (concat (make-temp-name " *ellama shell
command") "*"))))
- (set-process-sentinel
- (start-process "*ellama-shell-command*" buf shell-file-name
shell-command-switch cmd)
- (lambda (process _)
- (when (not (process-live-p process))
- (funcall callback
- ;; we need to trim trailing newline
- (string-trim-right
- (with-current-buffer buf (buffer-string))
- "\n"))
- (kill-buffer buf)))))
+ (condition-case err
+ (let ((buf (get-buffer-create
+ (concat (make-temp-name " *ellama shell command") "*"))))
+ (set-process-sentinel
+ (start-process
+ "*ellama-shell-command*" buf shell-file-name shell-command-switch cmd)
+ (lambda (process _)
+ (when (not (process-live-p process))
+ (let* ((raw-output
+ ;; trim trailing newline to reduce noisy tool output
+ (string-trim-right
+ (with-current-buffer buf (buffer-string))
+ "\n"))
+ (output
+ (ellama-tools--sanitize-tool-text-output
+ raw-output
+ "Command output"))
+ (exit-code (process-exit-status process))
+ (result
+ (cond
+ ((and (string= output "") (zerop exit-code))
+ "Command completed successfully with no output.")
+ ((string= output "")
+ (format "Command failed with exit code %d and no output."
+ exit-code))
+ ((zerop exit-code)
+ output)
+ (t
+ (format "Command failed with exit code %d.\n%s"
+ exit-code output)))))
+ (funcall callback result)
+ (kill-buffer buf))))))
+ (error
+ (funcall callback
+ (format "Failed to start shell command: %s"
+ (error-message-string err)))))
;; async tool should always return nil
;; to work properly with the llm library
nil)
@@ -691,7 +741,9 @@ ANSWER-VARIANT-LIST is a list of possible answer
variants."))
(forward-line (1- to))
(end-of-line)
(point))))
- (buffer-substring-no-properties start end))))))
+ (ellama-tools--sanitize-tool-text-output
+ (buffer-substring-no-properties start end)
+ (format "File %s" file-name)))))))
(ellama-tools-define-tool
'(:function
diff --git a/ellama.el b/ellama.el
index 78d2f2bd34..c3fd36d428 100644
--- a/ellama.el
+++ b/ellama.el
@@ -6,7 +6,7 @@
;; URL: http://github.com/s-kostyaev/ellama
;; Keywords: help local tools
;; Package-Requires: ((emacs "28.1") (llm "0.24.0") (plz "0.8") (transient
"0.7") (compat "29.1") (yaml "1.2.3"))
-;; Version: 1.12.10
+;; Version: 1.12.11
;; SPDX-License-Identifier: GPL-3.0-or-later
;; Created: 8th Oct 2023
@@ -568,17 +568,18 @@ It should be a function with single argument generated
text string."
"Replace code src blocks in TEXT."
(with-temp-buffer
(insert (propertize text 'hard t))
- (goto-char (point-min))
- ;; skip good code blocks
- (while (re-search-forward "#\\+BEGIN_SRC\\(.\\|\n\\)*?#\\+END_SRC" nil t))
- (while (re-search-forward "#\\+END_SRC\\(\\(.\\|\n\\)*?\\)#\\+END_SRC" nil
t)
- (unless (string-match-p "#\\+BEGIN_SRC" (match-string 1))
- (replace-match "#+BEGIN_SRC\\1#+END_SRC")))
- (goto-char (match-beginning 0))
- (while (re-search-backward "#\\+END_SRC\\(\\(.\\|\n\\)*?\\)#\\+END_SRC"
nil t)
- (unless (string-match-p "#\\+BEGIN_SRC" (match-string 1))
- (replace-match "#+BEGIN_SRC\\1#+END_SRC"))
- (goto-char (match-beginning 0)))
+ (let ((open-blocks 0)
+ (pattern
+
"^\\([[:blank:]]*\\)#\\+\\(BEGIN_SRC\\|END_SRC\\)\\(?:[[:blank:]].*\\)?$"))
+ (goto-char (point-min))
+ (while (re-search-forward pattern nil t)
+ (if (string= (match-string 2) "BEGIN_SRC")
+ (setq open-blocks (1+ open-blocks))
+ (if (> open-blocks 0)
+ (setq open-blocks (1- open-blocks))
+ (let ((indent (match-string 1)))
+ (replace-match (concat indent "#+BEGIN_SRC") t t))
+ (setq open-blocks (1+ open-blocks))))))
(buffer-substring-no-properties (point-min) (point-max))))
(defun ellama--replace (from to beg end)
@@ -1441,18 +1442,43 @@ REASONING-BUFFER is a buffer for reasoning."
(when text
(string-trim text)))))))
-(defun ellama--error-handler (buffer errcb)
+(defun ellama--tool-call-error-p (err-type)
+ "Return non-nil when ERR-TYPE indicates a tool call error."
+ (and err-type
+ (memq 'llm-tool-call-error
+ (get err-type 'error-conditions))))
+
+(defun ellama--append-tool-error-to-prompt (prompt msg)
+ "Append tool call error MSG to PROMPT."
+ (when prompt
+ (llm-chat-prompt-append-response
+ prompt
+ (if (stringp msg)
+ msg
+ (format "%s" (or msg "Unknown tool call error")))
+ 'system)))
+
+(defun ellama--error-handler (buffer errcb &optional prompt
+ retry-fn)
"Error handler function.
BUFFER is the current ellama buffer.
-ERRCB is an error callback."
- (lambda (_ msg)
+ERRCB is an error callback.
+PROMPT is the active prompt.
+RETRY-FN is called to retry the request."
+ (lambda (err-type msg)
(with-current-buffer buffer
- (cancel-change-group ellama--change-group)
- (when ellama-spinner-enabled
- (spinner-stop))
- (funcall errcb msg)
- (setq ellama--current-request nil)
- (ellama-request-mode -1))))
+ (if (and retry-fn
+ prompt
+ (ellama--tool-call-error-p err-type))
+ (progn
+ (ellama--append-tool-error-to-prompt prompt msg)
+ (funcall retry-fn))
+ (cancel-change-group ellama--change-group)
+ (when ellama-spinner-enabled
+ (spinner-stop))
+ (funcall errcb msg)
+ (setq ellama--current-request nil)
+ (ellama-request-mode -1)))))
(defun ellama--response-handler (result-handler reasoning-buffer buffer donecb
errcb provider llm-prompt async filter)
"Response handler function.
@@ -1475,41 +1501,56 @@ inserted into the BUFFER."
(not reasoning))
(when (not tool-result) (kill-buffer reasoning-buffer)))
(if tool-result
- (let* ((insert-text
- (ellama--insert buffer (with-current-buffer buffer (if
ellama--current-session
-
(point-max)
- (point)))
- filter))
- (insert-reasoning
- (ellama--insert reasoning-buffer nil
#'ellama--translate-markdown-to-org-filter))
- (handler (ellama--handle-partial insert-text insert-reasoning
reasoning-buffer))
- (cnt 0)
- (skip-handler
- (lambda (request)
- (if (= cnt ellama-response-process-method)
- (progn
- (funcall handler request)
- (setq cnt 0))
- (cl-incf cnt)))))
- (with-current-buffer buffer
- (if async
- (llm-chat-async
- provider
- llm-prompt
- (ellama--response-handler
- handler reasoning-buffer buffer donecb errcb provider
llm-prompt async filter)
- (ellama--error-handler buffer errcb)
- t)
- (llm-chat-streaming
- provider
- llm-prompt
- (if (integerp ellama-response-process-method)
- skip-handler handler)
- (ellama--response-handler
- handler
- reasoning-buffer buffer donecb errcb provider llm-prompt
async filter)
- (ellama--error-handler buffer errcb)
- t))))
+ (cl-labels
+ ((start-request ()
+ (let* ((insert-text
+ (ellama--insert
+ buffer
+ (with-current-buffer buffer
+ (if ellama--current-session
+ (point-max)
+ (point)))
+ filter))
+ (insert-reasoning
+ (ellama--insert reasoning-buffer nil
+
#'ellama--translate-markdown-to-org-filter))
+ (handler
+ (ellama--handle-partial
+ insert-text insert-reasoning reasoning-buffer))
+ (cnt 0)
+ (skip-handler
+ (lambda (request)
+ (if (= cnt ellama-response-process-method)
+ (progn
+ (funcall handler request)
+ (setq cnt 0))
+ (cl-incf cnt))))
+ (error-handler
+ (ellama--error-handler
+ buffer errcb llm-prompt
+ (lambda ()
+ (start-request)))))
+ (with-current-buffer buffer
+ (if async
+ (llm-chat-async
+ provider
+ llm-prompt
+ (ellama--response-handler
+ handler reasoning-buffer buffer donecb errcb provider
+ llm-prompt async filter)
+ error-handler
+ t)
+ (llm-chat-streaming
+ provider
+ llm-prompt
+ (if (integerp ellama-response-process-method)
+ skip-handler handler)
+ (ellama--response-handler
+ handler reasoning-buffer buffer donecb errcb provider
+ llm-prompt async filter)
+ error-handler
+ t))))))
+ (start-request))
(with-current-buffer buffer
(accept-change-group ellama--change-group)
(when ellama-spinner-enabled
@@ -1596,10 +1637,8 @@ failure (with BUFFER current).
prompt-with-ctx)
(setf (llm-chat-prompt-tools
(ellama-session-prompt session))
tools)
- (when system
- (llm-chat-prompt-append-response
- (ellama-session-prompt session)
- system 'system))
+ ;; System message is part of prompt context and
should not be
+ ;; appended on each interaction.
(ellama-session-prompt session))
(setf (ellama-session-prompt session)
(llm-make-chat-prompt prompt-with-ctx :context
system
@@ -1610,48 +1649,64 @@ failure (with BUFFER current).
(org-mode))
(with-current-buffer buffer
(ellama-request-mode +1)
- (let* ((insert-text
- (ellama--insert buffer point filter))
- (insert-reasoning
- (ellama--insert reasoning-buffer nil
#'ellama--translate-markdown-to-org-filter)))
- (setq ellama--change-group (prepare-change-group))
- (activate-change-group ellama--change-group)
- (when ellama-spinner-enabled
- (require 'spinner)
- (spinner-start ellama-spinner-type))
- (let* ((handler (ellama--handle-partial insert-text insert-reasoning
reasoning-buffer))
- (request (pcase ellama-response-process-method
- ('async (llm-chat-async
+ (cl-labels
+ ((start-request ()
+ (let* ((insert-text
+ (ellama--insert buffer point filter))
+ (insert-reasoning
+ (ellama--insert reasoning-buffer nil
+
#'ellama--translate-markdown-to-org-filter))
+ (handler
+ (ellama--handle-partial
+ insert-text insert-reasoning reasoning-buffer))
+ (error-handler
+ (ellama--error-handler
+ buffer errcb llm-prompt
+ #'start-request))
+ (request (pcase ellama-response-process-method
+ ('async (llm-chat-async
+ provider
+ llm-prompt
+ (ellama--response-handler
+ handler reasoning-buffer buffer donecb
errcb provider
+ llm-prompt t filter)
+ error-handler
+ t))
+ ('streaming (llm-chat-streaming
+ provider
+ llm-prompt
+ handler
+ (ellama--response-handler
+ handler reasoning-buffer buffer
donecb errcb
+ provider llm-prompt nil filter)
+ error-handler
+ t))
+ ((pred integerp)
+ (let* ((cnt 0)
+ (skip-handler
+ (lambda (request)
+ (if (= cnt
ellama-response-process-method)
+ (progn
+ (funcall handler request)
+ (setq cnt 0))
+ (cl-incf cnt)))))
+ (llm-chat-streaming
provider
llm-prompt
- (ellama--response-handler handler
reasoning-buffer buffer donecb errcb provider llm-prompt t filter)
- (ellama--error-handler buffer errcb)
- t))
- ('streaming (llm-chat-streaming
- provider
- llm-prompt
- handler
- (ellama--response-handler handler
reasoning-buffer buffer donecb errcb provider llm-prompt nil filter)
- (ellama--error-handler buffer errcb)
- t))
- ((pred integerp)
- (let* ((cnt 0)
- (skip-handler
- (lambda (request)
- (if (= cnt ellama-response-process-method)
- (progn
- (funcall handler request)
- (setq cnt 0))
- (cl-incf cnt)))))
- (llm-chat-streaming
- provider
- llm-prompt
- skip-handler
- (ellama--response-handler handler
reasoning-buffer buffer donecb errcb provider llm-prompt t filter)
- (ellama--error-handler buffer errcb)
- t))))))
- (with-current-buffer buffer
- (setq ellama--current-request request)))))))
+ skip-handler
+ (ellama--response-handler
+ handler reasoning-buffer buffer donecb
errcb provider
+ llm-prompt t filter)
+ error-handler
+ t))))))
+ (setq ellama--change-group (prepare-change-group))
+ (activate-change-group ellama--change-group)
+ (when ellama-spinner-enabled
+ (require 'spinner)
+ (spinner-start ellama-spinner-type))
+ (with-current-buffer buffer
+ (setq ellama--current-request request)))))
+ (start-request)))))
(defun ellama-chain (initial-prompt forms &optional acc)
"Call chain of FORMS on INITIAL-PROMPT.
diff --git a/tests/test-ellama.el b/tests/test-ellama.el
index b81c5f5ba4..1552d0a505 100644
--- a/tests/test-ellama.el
+++ b/tests/test-ellama.el
@@ -31,6 +31,12 @@
(require 'ert)
(require 'llm-fake)
+(defconst ellama-test-root
+ (expand-file-name
+ ".."
+ (file-name-directory (or load-file-name buffer-file-name)))
+ "Project root directory for test assets.")
+
(ert-deftest test-ellama--code-filter ()
(should (equal "" (ellama--code-filter "")))
(should (equal "(hello)" (ellama--code-filter "(hello)")))
@@ -842,6 +848,10 @@ That's it."))))
(should (string-match-p "#\\+END_SRC" result))
(should (string-match-p "#\\+END_QUOTE" result))))
+(ert-deftest test-ellama-replace-bad-code-blocks-no-src-blocks ()
+ (let ((text "\n#+BEGIN_QUOTE\n((shell_command . ))\n#+END_QUOTE\n"))
+ (should (string-equal (ellama--replace-bad-code-blocks text) text))))
+
(ert-deftest test-ellama-md-to-org-code-inline-latex ()
(let ((result (ellama--translate-markdown-to-org-filter "_some italic_
$$P_\\theta(Y_T, ..., Y_2|Y_1, x_1, ..., x_T)$$
@@ -1019,6 +1029,95 @@ region, season, or type)! 🍎🍊"))))
(should (equal (ellama--string-without-last-two-lines "Line1\nLine2")
"")))
+(ert-deftest test-ellama--append-tool-error-to-prompt-uses-llm-message ()
+ (let (captured)
+ (cl-letf (((symbol-function 'llm-chat-prompt-append-response)
+ (lambda (_prompt msg role)
+ (setq captured (list msg role)))))
+ (ellama--append-tool-error-to-prompt
+ 'prompt
+ "Unknown tool 'search' called"))
+ (should (equal captured
+ '("Unknown tool 'search' called" system)))))
+
+(defun ellama-test--ensure-local-ellama-tools ()
+ "Ensure tests use local `ellama-tools.el' from project root."
+ (unless (fboundp 'ellama-tools--sanitize-tool-text-output)
+ (load-file (expand-file-name "ellama-tools.el" ellama-test-root))))
+
+(defun ellama-test--wait-shell-command-result (cmd)
+ "Run shell tool CMD and wait for a result string."
+ (ellama-test--ensure-local-ellama-tools)
+ (let ((result :pending)
+ (deadline (+ (float-time) 3.0)))
+ (ellama-tools-shell-command-tool
+ (lambda (res)
+ (setq result res))
+ cmd)
+ (while (and (eq result :pending)
+ (< (float-time) deadline))
+ (accept-process-output nil 0.01))
+ (when (eq result :pending)
+ (ert-fail (format "Timeout while waiting result for: %s" cmd)))
+ result))
+
+(ert-deftest test-ellama-shell-command-tool-empty-success-output ()
+ (should
+ (string=
+ (ellama-test--wait-shell-command-result "sh -c 'true'")
+ "Command completed successfully with no output.")))
+
+(ert-deftest test-ellama-shell-command-tool-empty-failure-output ()
+ (should
+ (string-match-p
+ "Command failed with exit code 7 and no output\\."
+ (ellama-test--wait-shell-command-result "sh -c 'exit 7'"))))
+
+(ert-deftest test-ellama-shell-command-tool-returns-stdout ()
+ (should
+ (string=
+ (ellama-test--wait-shell-command-result "printf 'ok\\n'")
+ "ok")))
+
+(ert-deftest test-ellama-shell-command-tool-rejects-binary-output ()
+ (should
+ (string-match-p
+ "binary data"
+ (ellama-test--wait-shell-command-result
+ "awk 'BEGIN { printf \"%c\", 0 }'"))))
+
+(ert-deftest test-ellama-read-file-tool-rejects-binary-content ()
+ (ellama-test--ensure-local-ellama-tools)
+ (let ((file (make-temp-file "ellama-read-file-bin-")))
+ (unwind-protect
+ (progn
+ (let ((coding-system-for-write 'no-conversion))
+ (with-temp-buffer
+ (set-buffer-multibyte nil)
+ (insert "%PDF-1.5\n%")
+ (insert (char-to-string 143))
+ (insert "\n")
+ (write-region (point-min) (point-max) file nil 'silent)))
+ (let ((result (ellama-tools-read-file-tool file)))
+ (should (string-match-p "binary data" result))
+ (should (string-match-p "bad idea" result))))
+ (when (file-exists-p file)
+ (delete-file file)))))
+
+(ert-deftest test-ellama-read-file-tool-accepts-utf8-markdown-text ()
+ (ellama-test--ensure-local-ellama-tools)
+ (let ((file (make-temp-file "ellama-read-file-utf8-" nil ".md")))
+ (unwind-protect
+ (progn
+ (with-temp-file file
+ (insert "# Research Plan\n\n")
+ (insert "Sub‑topics: temporal reasoning overview.\n"))
+ (let ((result (ellama-tools-read-file-tool file)))
+ (should-not (string-match-p "binary data" result))
+ (should (string-match-p "Research Plan" result))))
+ (when (file-exists-p file)
+ (delete-file file)))))
+
(provide 'test-ellama)
;;; test-ellama.el ends here
