Question: how to define parameters for edit-file tool?

[beacoder]

Please update gptel first -- errors are often fixed by the time they're reported.

• I have updated gptel to the latest commit and tested that the issue still exists

Bug Description

Hi @karthink,
I got another question to ask you about:
I wanna create an edit file tool like the following:
the first param is a file-path, while the second is a list of cons whose car is old-string, cdr is new-string.
how should I define the params with array ?

I tried to define one like the following, but seems not work.
could u plz help to suggest the right way to define the tool?
many thanks.

(defun my-gptel--edit-file (file-path file-edits)
    "In FILE-PATH, apply FILE-EDITS with pattern matching and replacing."
    (with-temp-buffer
      (insert-file-contents (expand-file-name file-path))
      (let ((case-fold-search nil)
            (edit-success nil))
        (dolist (file-edit file-edits)
          (when-let ((old-string (car file-edit))
                     (new-string (cdr file-edit))
                     (is-valid-old-string (not (string= old-string ""))))
            (goto-char (point-min))
            (when (search-forward old-string nil t)
              (replace-match new-string t t)
              (setq edit-success t))))
        (if (not edit-success)
            (format "Failed to edited %s" file-path)
          (write-file file-path)
          (format "Successfully edited %s" file-path)))))

  (gptel-make-tool
   :function #'my-gptel--edit-file
   :name "edit_file"
   :description "Update file content with a list of edits, each edit contains a old-string and a new-string, new-string will replace the old-string."
   :args (list '(:name "filename"
                       :type "string"
                       :description "The name of the file to edit")
               '(:name "edits"
                       :type array
                       :items (:type object
                                     :properties
                                     (:old_string
                                      (:type string :description "The old-string to be replaced.")
                                      :new_string
                                      (:type string :description "The new-string to replace old-string.")))
                       :description "The list of edits to apply on the file"))
   :category "filesystem")

Backend

None

Steps to Reproduce

Hi @karthink,
I got another question to ask you about:
I wanna create an edit file tool like the following:
the first param is a file-path, while the second is a list of cons whose car is old-string, cdr is new-string.
how should I define the params with array ?

I tried to define one like the following, but seems not work.
could u plz help to suggest the right way to define the tool?
many thanks.

(defun my-gptel--edit-file (file-path file-edits)
    "In FILE-PATH, apply FILE-EDITS with pattern matching and replacing."
    (with-temp-buffer
      (insert-file-contents (expand-file-name file-path))
      (let ((case-fold-search nil)
            (edit-success nil))
        (dolist (file-edit file-edits)
          (when-let ((old-string (car file-edit))
                     (new-string (cdr file-edit))
                     (is-valid-old-string (not (string= old-string ""))))
            (goto-char (point-min))
            (when (search-forward old-string nil t)
              (replace-match new-string t t)
              (setq edit-success t))))
        (if (not edit-success)
            (format "Failed to edited %s" file-path)
          (write-file file-path)
          (format "Successfully edited %s" file-path)))))

  (gptel-make-tool
   :function #'my-gptel--edit-file
   :name "edit_file"
   :description "Update file content with a list of edits, each edit contains a old-string and a new-string, new-string will replace the old-string."
   :args (list '(:name "filename"
                       :type "string"
                       :description "The name of the file to edit")
               '(:name "edits"
                       :type array
                       :items (:type object
                                     :properties
                                     (:old_string
                                      (:type string :description "The old-string to be replaced.")
                                      :new_string
                                      (:type string :description "The new-string to replace old-string.")))
                       :description "The list of edits to apply on the file"))
   :category "filesystem")

Additional Context

Emacs 30.1

Backtrace

Log Information

[karthink]

First, you have extra quotes (') that will be read literally as quote. So I would fix the nested quotes.

LLMs won't generate function call arguments that are elisp data structures. They only speak JSON. In this case you want the second argument to be a list of replacements (old -> new), so you can structure this as a JSON array of objects, where each object has two keys, old_string and new_string.

Working backwards from there, the edits argument is:

'(:name "edits"
  :type array
  :items (:type object
          :properties
          (:old_string
           (:type string :description "The old-string to be replaced.")
           :new_string
           (:type string :description "The new-string to replace old.")))
  :description "The list of edits to apply on the file")

How well this works depends on the LLM you're using. The Anthropic models should be able to handle this fine, at least.

(When you receive the function call arguments, you may need to parse the JSON yourself. You can do this using gptel--json-read-string if required.)

[beacoder]

Thanks a lot, I will try it myself, and it the tool works, I will publish it to the gptel wiki.
I will leave this issue open for now, if all works well, I will close it then.

[beacoder]

Hi @karthink ,
I updated the tool definition as you suggested, the tool got called but still failed, seems due to some parsing issue.
Using gptel--json-read-string failed to solve the issue, seems need to parse the json array into lisp-list.
could u plz help to have a look, the following is the tool_call result:
thanks.

  {
    "role": "assistant",
    "content": null,
    "tool_calls": [
      {
        "id": "chatcmpl-tool-dda604a8ae694c51afda14a24c8c91cb",
        "type": "function",
        "index": 0,
        "function": {
          "name": "edit_file",
          "arguments": "{\"filename\": \"~/workspace/ai/my_projects/test.py\", \"edits\": [{\"old_string\": \"def main():\", \"new_string\": \"    def main():\"}]}"
        }
      }
    ]
  },
  {
    "role": "tool",
    "tool_call_id": "call_chatcmpl-tool-dda604a8ae694c51afda14a24c8c91cb",
    "content": "wrong-type-argument listp [(:old_string \"def main():\" :new_string \"    def main():\")]"
  }

[karthink]

The tool call looks correct to me, and it looks like gptel--json-read-string did its job correctly as well (or was not required at all)

The problem is that you run (dolist (file-edit file-edits) ...), but file-edits is a vector here, not a list. Try (dolist (file-edit (seq-into file-edits 'list)) ...).

[beacoder]

thanks, I will try it again.

[beacoder]

It works now, thanks.

(defun my-gptel--edit-file (file-path file-edits)
    "In FILE-PATH, apply FILE-EDITS with pattern matching and replacing."
    (with-temp-buffer
      (let ((case-fold-search nil)
            (edit-success nil)
            (file-name (expand-file-name file-path)))
        (insert-file-contents file-name)
        (dolist (file-edit (seq-into file-edits 'list))
          (when-let ((line-number (plist-get file-edit :line_number))
                     (old-string (plist-get file-edit :old_string))
                     (new-string (plist-get file-edit :new_string))
                     (is-valid-old-string (not (string= old-string ""))))
            (goto-char (point-min))
            (forward-line (1- line-number))
            (when (search-forward old-string nil t)
              (replace-match new-string t t)
              (setq edit-success t))))
        (if (not edit-success)
            (format "Failed to edited %s" file-name)
          (write-file file-name)
          (format "Successfully edited %s" file-name)))))
(gptel-make-tool
   :function #'my-gptel--edit-file
   :name "edit_file"
   :description "Update file content with a list of edits, each edit contains a line-number,
a old-string and a new-string, new-string will replace the old-string at the specified line."
   :args (list '(:name "file-path"
                       :type "string"
                       :description "The full path of the file to edit")
               '(:name "file-edits"
                       :type array
                       :items (:type object
                                     :properties
                                     (:line_number
                                      (:type integer :description "The line number of the file where edit starts.")
                                      :old_string
                                      (:type string :description "The old-string to be replaced.")
                                      :new_string
                                      (:type string :description "The new-string to replace old-string.")))
                       :description "The list of edits to apply on the file"))
   :category "filesystem")