Skip to content

Show args and details for generic (extension) tools #112

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

Merged
dnouri merged 1 commit into from
Feb 8, 2026
Merged

Show args and details for generic (extension) tools #112

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
57 changes: 47 additions & 10 deletions pi-coding-agent-render.el
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -802,10 +802,27 @@ it from extending to subsequent content. Sets pending overlay to nil."
(overlay-put ov 'face face)))
(setq pi-coding-agent--pending-tool-overlay nil)))

(defun pi-coding-agent--pretty-print-json (plist-data)
"Return PLIST-DATA as a 2-space indented JSON string, or nil.
Handles the plist/vector representation from `json-parse-string'
with `:object-type \\='plist'. Returns nil when PLIST-DATA is nil."
(when plist-data
(require 'json)
;; json-serialize is fast (C) but has no pretty-print option;
;; json-encode supports it, but needs alists — so we round-trip.
(let* ((compact (json-serialize plist-data))
(parsed (json-parse-string compact :object-type 'alist))
(json-encoding-pretty-print t)
(json-encoding-default-indentation " "))
(json-encode parsed))))

(defun pi-coding-agent--tool-header (tool-name args)
"Return propertized header for tool TOOL-NAME with ARGS.
The tool name prefix uses `pi-coding-agent-tool-name' face and
the arguments use `pi-coding-agent-tool-command' face.
Built-in tools show specialized formats (e.g., \"$ cmd\" for bash).
Generic tools show JSON args: compact when the full header fits
within `fill-column', pretty-printed otherwise.
Uses `font-lock-face' to survive gfm-mode refontification."
(let ((path (pi-coding-agent--tool-path args)))
(pcase tool-name
Expand All @@ -816,7 +833,21 @@ Uses `font-lock-face' to survive gfm-mode refontification."
((or "read" "write" "edit")
(concat (propertize tool-name 'font-lock-face 'pi-coding-agent-tool-name)
(propertize (concat " " (or path "...")) 'font-lock-face 'pi-coding-agent-tool-command)))
(_ (propertize tool-name 'font-lock-face 'pi-coding-agent-tool-name)))))
(_
(let* ((name (propertize tool-name 'font-lock-face 'pi-coding-agent-tool-name))
(json-pretty (pi-coding-agent--pretty-print-json args))
(json-compact (when json-pretty
(mapconcat #'string-trim
(split-string json-pretty "\n") " ")))
(json (cond
((null json-pretty) nil)
((<= (+ (length tool-name) 1 (length json-compact))
fill-column)
json-compact)
(t json-pretty))))
(if json
(concat name (propertize (concat " " json) 'font-lock-face 'pi-coding-agent-tool-command))
name))))))

(defun pi-coding-agent--display-tool-start (tool-name args)
"Display header for tool TOOL-NAME with ARGS and create overlay."
Expand Down Expand Up @@ -1134,7 +1165,8 @@ Returns nil if the buffer doesn't exist or has no complete lines."
(defun pi-coding-agent--display-tool-end (tool-name args content details is-error)
"Display result for TOOL-NAME and update overlay face.
ARGS contains tool arguments, CONTENT is a list of content blocks.
DETAILS contains tool-specific data (e.g., diff for edit tool).
DETAILS contains tool-specific data (e.g., diff for edit tool);
for generic tools, non-nil DETAILS are rendered below the content.
IS-ERROR indicates failure.
Shows preview lines with expandable toggle for long output."
(let* ((is-error (eq t is-error))
Expand All @@ -1152,14 +1184,19 @@ Shows preview lines with expandable toggle for long output."
(is-edit-diff (and (equal tool-name "edit")
(not is-error)
(plist-get details :diff)))
;; For edit with diff, use diff from details
;; For write, use content from args (result is just success message)
;; Strip ANSI escape codes - CLI tools often output colors
(display-content (ansi-color-filter-apply
(pcase tool-name
("edit" (or (plist-get details :diff) raw-output))
("write" (or (plist-get args :content) raw-output))
(_ raw-output))))
(display-content
(ansi-color-filter-apply
(pcase tool-name
("edit" (or (plist-get details :diff) raw-output))
("write" (or (plist-get args :content) raw-output))
((or "bash" "read") raw-output)
(_ (if-let ((details-json
(pi-coding-agent--pretty-print-json details)))
(concat raw-output "\n\n"
(propertize (concat "**Details**\n" details-json)
'font-lock-face
'pi-coding-agent-tool-output))
raw-output)))))
(preview-limit (pcase tool-name
("bash" pi-coding-agent-bash-preview-lines)
(_ pi-coding-agent-tool-preview-lines)))
Expand Down
34 changes: 0 additions & 34 deletions test/pi-coding-agent-input-test.el
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2452,40 +2452,6 @@ Errors still consume context, so their usage data is valid for display."
;; Should NOT have drawer syntax
(should-not (string-match-p ":BASH:" (buffer-string)))))

(ert-deftest pi-coding-agent-test-tool-header-faces ()
"Tool header applies tool-name face on prefix and tool-command on args."
;; bash: "$" is tool-name, command is tool-command
(let ((header (pi-coding-agent--tool-header "bash" '(:command "ls -la"))))
(should (eq (get-text-property 0 'font-lock-face header)
'pi-coding-agent-tool-name))
(should (eq (get-text-property 2 'font-lock-face header)
'pi-coding-agent-tool-command)))
;; read/write/edit: tool name is tool-name, path is tool-command
(dolist (tool '("read" "write" "edit"))
(let ((header (pi-coding-agent--tool-header tool '(:path "foo.txt"))))
(should (eq (get-text-property 0 'font-lock-face header)
'pi-coding-agent-tool-name))
(should (eq (get-text-property (1+ (length tool)) 'font-lock-face header)
'pi-coding-agent-tool-command))))
;; Unknown tool: entire string is tool-name
(let ((header (pi-coding-agent--tool-header "custom_tool" nil)))
(should (eq (get-text-property 0 'font-lock-face header)
'pi-coding-agent-tool-name))
(should (equal (substring-no-properties header) "custom_tool"))))

(ert-deftest pi-coding-agent-test-tool-header-survives-font-lock ()
"Tool header font-lock-face properties survive gfm-mode refontification."
(with-temp-buffer
(pi-coding-agent-chat-mode)
(pi-coding-agent--display-tool-start "edit" '(:path "foo.txt"))
(font-lock-ensure)
(goto-char (point-min))
(should (eq (get-text-property (point) 'font-lock-face)
'pi-coding-agent-tool-name))
(search-forward "foo.txt")
(should (eq (get-text-property (match-beginning 0) 'font-lock-face)
'pi-coding-agent-tool-command))))

(ert-deftest pi-coding-agent-test-tool-end-keeps-overlay-face ()
"tool_execution_end keeps base face on success."
(with-temp-buffer
Expand Down
253 changes: 253 additions & 0 deletions test/pi-coding-agent-render-test.el
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -939,6 +939,142 @@ since we don't display them locally. Let pi's message_start handle it."
(should response-sent)
(should (eq (plist-get response-sent :cancelled) t))))))

;;; Pretty-Print JSON Helper

(ert-deftest pi-coding-agent-test-pretty-print-json-simple-plist ()
"Pretty-print helper produces 2-space indented JSON from plist."
(let ((result (pi-coding-agent--pretty-print-json
'(:agent "worker" :task "Search for foo"))))
(should (stringp result))
(should (string-match-p "\"agent\": \"worker\"" result))
(should (string-match-p "\"task\": \"Search for foo\"" result))
;; Should be multi-line (pretty-printed)
(should (> (length (split-string result "\n")) 1))))

(ert-deftest pi-coding-agent-test-pretty-print-json-nested ()
"Pretty-print helper handles nested objects and arrays."
(let ((result (pi-coding-agent--pretty-print-json
'(:tasks [(:agent "worker" :task "foo")
(:agent "scout" :task "bar")]))))
(should (string-match-p "\"tasks\"" result))
(should (string-match-p "\"worker\"" result))
(should (string-match-p "\"scout\"" result))))

(ert-deftest pi-coding-agent-test-pretty-print-json-unicode ()
"Pretty-print helper preserves non-ASCII characters."
(let ((result (pi-coding-agent--pretty-print-json
'(:city "Malmö" :note "väder"))))
(should (string-match-p "Malmö" result))
(should (string-match-p "väder" result))
;; Should NOT have octal escapes
(should-not (string-match-p "\\\\303" result))))

(ert-deftest pi-coding-agent-test-pretty-print-json-nil ()
"Pretty-print helper returns nil for nil input."
(should-not (pi-coding-agent--pretty-print-json nil)))

;;; Tool Header

(ert-deftest pi-coding-agent-test-tool-header-faces ()
"Tool header applies tool-name face on prefix and tool-command on args."
;; bash: "$" is tool-name, command is tool-command
(let ((header (pi-coding-agent--tool-header "bash" '(:command "ls -la"))))
(should (eq (get-text-property 0 'font-lock-face header)
'pi-coding-agent-tool-name))
(should (eq (get-text-property 2 'font-lock-face header)
'pi-coding-agent-tool-command)))
;; read/write/edit: tool name is tool-name, path is tool-command
(dolist (tool '("read" "write" "edit"))
(let ((header (pi-coding-agent--tool-header tool '(:path "foo.txt"))))
(should (eq (get-text-property 0 'font-lock-face header)
'pi-coding-agent-tool-name))
(should (eq (get-text-property (1+ (length tool)) 'font-lock-face header)
'pi-coding-agent-tool-command))))
;; Unknown tool with nil args: entire string is tool-name
(let ((header (pi-coding-agent--tool-header "custom_tool" nil)))
(should (eq (get-text-property 0 'font-lock-face header)
'pi-coding-agent-tool-name))
(should (equal (substring-no-properties header) "custom_tool"))))

(ert-deftest pi-coding-agent-test-tool-header-survives-font-lock ()
"Tool header font-lock-face properties survive gfm-mode refontification."
(with-temp-buffer
(pi-coding-agent-chat-mode)
(pi-coding-agent--display-tool-start "edit" '(:path "foo.txt"))
(font-lock-ensure)
(goto-char (point-min))
(should (eq (get-text-property (point) 'font-lock-face)
'pi-coding-agent-tool-name))
(search-forward "foo.txt")
(should (eq (get-text-property (match-beginning 0) 'font-lock-face)
'pi-coding-agent-tool-command))))

(ert-deftest pi-coding-agent-test-generic-tool-header-with-args ()
"Generic tool header shows tool name and JSON args."
(let ((header (pi-coding-agent--tool-header
"subagent" '(:agent "worker" :task "Search"))))
;; Should start with "subagent "
(should (string-prefix-p "subagent " (substring-no-properties header)))
;; Should contain JSON keys
(should (string-match-p "\"agent\"" (substring-no-properties header)))
(should (string-match-p "\"worker\"" (substring-no-properties header)))))

(ert-deftest pi-coding-agent-test-generic-tool-header-compact-when-short ()
"Short args produce a single-line compact header."
(let* ((fill-column 70)
(header (pi-coding-agent--tool-header "subagent" '(:agent "worker")))
(text (substring-no-properties header)))
;; Single line
(should (= 1 (length (split-string text "\n"))))
;; Contains key and value with proper JSON spacing
(should (string-match-p "\"agent\": \"worker\"" text))))

(ert-deftest pi-coding-agent-test-generic-tool-header-pretty-when-long ()
"Long args that exceed fill-column produce a multi-line pretty header."
(let* ((fill-column 40)
(header (pi-coding-agent--tool-header
"subagent" '(:agent "worker" :task "Search for weather")))
(text (substring-no-properties header)))
;; Multi-line (pretty-printed)
(should (> (length (split-string text "\n")) 1))))

(ert-deftest pi-coding-agent-test-generic-tool-header-respects-fill-column ()
"Compact-vs-pretty threshold follows fill-column."
(let ((args '(:agent "worker" :task "Search")))
;; Wide fill-column → compact
(let* ((fill-column 200)
(text (substring-no-properties
(pi-coding-agent--tool-header "subagent" args))))
(should (= 1 (length (split-string text "\n")))))
;; Narrow fill-column → pretty
(let* ((fill-column 20)
(text (substring-no-properties
(pi-coding-agent--tool-header "subagent" args))))
(should (> (length (split-string text "\n")) 1)))))

(ert-deftest pi-coding-agent-test-generic-tool-header-faces ()
"Generic tool header applies tool-name face on name, tool-command on args."
(let ((header (pi-coding-agent--tool-header
"subagent" '(:agent "worker" :task "Search"))))
;; Tool name portion gets tool-name face
(should (eq (get-text-property 0 'font-lock-face header)
'pi-coding-agent-tool-name))
;; JSON body (after "subagent ") gets tool-command face
(let ((json-start (length "subagent ")))
(should (eq (get-text-property json-start 'font-lock-face header)
'pi-coding-agent-tool-command)))))

(ert-deftest pi-coding-agent-test-builtin-tools-unaffected-by-generic-header ()
"Built-in tools still use their specialized header formats."
;; bash: still "$ command"
(let ((header (pi-coding-agent--tool-header "bash" '(:command "ls -la"))))
(should (string-prefix-p "$ " (substring-no-properties header))))
;; read/write/edit: still "tool path"
(dolist (tool '("read" "write" "edit"))
(let ((header (pi-coding-agent--tool-header tool '(:path "foo.txt"))))
(should (string-prefix-p (concat tool " foo.txt")
(substring-no-properties header))))))

;;; Tool Output

(ert-deftest pi-coding-agent-test-tool-start-inserts-header ()
Expand Down Expand Up @@ -1039,6 +1175,123 @@ since we don't display them locally. Let pi's message_start handle it."
;; Should NOT have more-lines indicator
(should-not (string-match-p "more lines" (buffer-string))))))

;;; Generic Tool Details in Output

(defun pi-coding-agent-test--insert-generic-tool (content-text &optional details)
"Insert a subagent tool start+end in current buffer.
CONTENT-TEXT is the text block string. DETAILS is an optional plist.
Call inside `with-temp-buffer' after `pi-coding-agent-chat-mode'."
(pi-coding-agent--display-tool-start "subagent" '(:agent "worker"))
(pi-coding-agent--display-tool-end "subagent" '(:agent "worker")
(list (list :type "text" :text content-text))
details nil))

(ert-deftest pi-coding-agent-test-generic-tool-content-follows-header ()
"Generic tool content starts directly after the header line."
(with-temp-buffer
(pi-coding-agent-chat-mode)
(pi-coding-agent-test--insert-generic-tool "Task completed")
(should (string-match-p "}\nTask completed" (buffer-string)))))

(ert-deftest pi-coding-agent-test-bash-no-blank-line-after-header ()
"Bash tool does NOT get an extra blank line after header."
(with-temp-buffer
(pi-coding-agent-chat-mode)
(pi-coding-agent--display-tool-start "bash" '(:command "ls"))
(pi-coding-agent--display-tool-end "bash" '(:command "ls")
'((:type "text" :text "file.txt"))
nil nil)
(let ((text (buffer-string)))
;; Bash header is "$ ls", followed by fenced code block — no extra blank line
(should-not (string-match-p "ls\n\n```" text)))))

(ert-deftest pi-coding-agent-test-generic-tool-details-appended ()
"Generic tool with non-nil details shows details JSON after content."
(with-temp-buffer
(pi-coding-agent-chat-mode)
(pi-coding-agent-test--insert-generic-tool
"Task completed" '(:mode "single" :exitCode 0))
(let ((text (buffer-string)))
(should (string-match-p "Task completed" text))
(should (string-match-p "\\*\\*Details\\*\\*" text))
(should (string-match-p "\"mode\": \"single\"" text))
(should (string-match-p "\"exitCode\": 0" text)))))

(ert-deftest pi-coding-agent-test-generic-tool-details-face ()
"Details label and JSON both use pi-coding-agent-tool-output face."
(with-temp-buffer
(pi-coding-agent-chat-mode)
(pi-coding-agent-test--insert-generic-tool
"Done" '(:mode "single" :exitCode 0))
;; Label gets the face
(goto-char (point-min))
(should (search-forward "**Details**" nil t))
(should (eq (get-text-property (match-beginning 0) 'font-lock-face)
'pi-coding-agent-tool-output))
;; JSON body gets the face
(should (search-forward "\"mode\"" nil t))
(should (eq (get-text-property (match-beginning 0) 'font-lock-face)
'pi-coding-agent-tool-output))))

(ert-deftest pi-coding-agent-test-generic-tool-nil-details-unchanged ()
"Generic tool with nil details shows only content text."
(with-temp-buffer
(pi-coding-agent-chat-mode)
(pi-coding-agent-test--insert-generic-tool "Task completed")
(let ((text (buffer-string)))
(should (string-match-p "Task completed" text))
(should-not (string-match-p "\\*\\*Details\\*\\*" text)))))

(ert-deftest pi-coding-agent-test-generic-tool-details-nested ()
"Details with nested structure render as indented JSON."
(with-temp-buffer
(pi-coding-agent-chat-mode)
(pi-coding-agent-test--insert-generic-tool
"Done" '(:items [(:name "a") (:name "b")]))
;; Output may be collapsed if long; check via button's full content
;; or directly in buffer for short output
(let* ((text (buffer-string))
(button (progn (goto-char (point-min)) (next-button (point))))
(full (if button
(button-get button 'pi-coding-agent-full-content)
text)))
(should (string-match-p "\"items\"" full))
(should (string-match-p "\"name\": \"a\"" full))
(should (string-match-p "\"name\": \"b\"" full)))))

(ert-deftest pi-coding-agent-test-bash-details-not-appended ()
"Built-in tool (bash) does NOT append details to output."
(with-temp-buffer
(pi-coding-agent-chat-mode)
(pi-coding-agent--display-tool-start "bash" '(:command "ls"))
(pi-coding-agent--display-tool-end "bash" '(:command "ls")
'((:type "text" :text "file.txt"))
'(:truncation t :fullOutputPath "/tmp/out")
nil)
(let ((text (buffer-string)))
(should (string-match-p "file.txt" text))
(should-not (string-match-p "\\*\\*Details\\*\\*" text)))))

(ert-deftest pi-coding-agent-test-generic-tool-details-in-expanded-view ()
"Details are included in collapsed output and survive TAB expand."
(with-temp-buffer
(pi-coding-agent-chat-mode)
(let* ((long-output (mapconcat (lambda (n) (format "line%d" n))
(number-sequence 1 20)
"\n"))
(details '(:errors [(:task "foo" :error "timeout")])))
(pi-coding-agent-test--insert-generic-tool long-output details)
;; Should have a "more lines" toggle (output is long enough)
(should (string-match-p "more lines" (buffer-string)))
;; Details should be in the full content accessible via TAB
;; Find the toggle button and check its full-content property
(goto-char (point-min))
(let ((button (next-button (point))))
(should button)
(let ((full (button-get button 'pi-coding-agent-full-content)))
(should (string-match-p "\\*\\*Details\\*\\*" full))
(should (string-match-p "\"error\": \"timeout\"" full)))))))

;;; Diff Overlay Highlighting

(ert-deftest pi-coding-agent-test-apply-diff-overlays-added-line ()
Expand Down