Skip to content

add edit tool formatters #1018

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
LasmarKhalifa wants to merge 1 commit into
base: 06-30/pi-tool-formatter-write
Choose a base branch
from
Open

add edit tool formatters #1018

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
21 changes: 21 additions & 0 deletions lib/roast/cogs/agent/providers/pi/messages/tool_call_message.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,27 @@ def format_write
"WRITE #{path} \"#{preview}\" (+#{count} #{"line".pluralize(count)})"
end

# Formats an edit tool call.
#
# Input fields:
# :path (String) – file to edit [required]
# :edits (Array) – edit blocks, each {oldText, newText} [required]
#
# Output: "EDIT <path> (<n> <edit|edits>)" – <n> is the number of edit
# blocks applied in the call. The count is always shown.
#
# Examples:
# EDIT lib/roast.rb (1 edit)
# EDIT config/app.yml (3 edits)
#
#: () -> String
def format_edit
path = arguments[:path]
edits = arguments[:edits] || []

@mathiusj mathiusj Jul 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

|| [] only guards nil — if :edits ever arrives as a non-Array (e.g. a String or a single Hash), .length either silently returns the wrong count (char count for a String) or raises. Given this is display-only, I wouldn't add validation here; the clean fix is a rescue→format_unknown at the dispatch site in #1012 (commented there). Just flagging so it's a conscious choice rather than an assumption. Also note EDIT #{path} interpolates the path untruncated, so a long path blows past the one-line goal the comments state.

count = edits.length
"EDIT #{path} (#{count} #{"edit".pluralize(count)})"
end

# Formats a tool call for which Roast has no dedicated formatter.
#
# Output: "<NAME> <key>: <value>, ..." – the upcased tool name, then each
Expand Down
16 changes: 16 additions & 0 deletions lib/roast/cogs/agent/providers/pi/messages/tool_result_message.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,22 @@ def format_write
ok_line(@input[:path])
end

# Formats an edit tool result.
#
# Input: :path – the path that was edited, from the originating call.
#
# Output: "EDIT OK <path>" – the file path, omitted when the call
# had none.
#
# Examples:
# EDIT OK lib/roast/version.rb
# EDIT OK
#
#: () -> String
def format_edit

@mathiusj mathiusj Jul 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Readability nit: the result formatter reads @input[:path] while the call formatter (same field) reads arguments[:path]. Both correct — @input is the resolved call's args in the result context, arguments is the call's own ivar — but the naming divergence across the two files costs a beat to reconcile. A one-line comment noting they're deliberately different objects would help future readers.

ok_line(@input[:path])
end

# Formats a result for which Roast has no dedicated formatter.
#
# Content: the tool's output text.
Expand Down
26 changes: 26 additions & 0 deletions test/roast/cogs/agent/providers/pi/messages/tool_call_message_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,32 @@ class ToolCallMessageTest < ActiveSupport::TestCase
assert_equal "WRITE lib/roast.rb \"#{"x" * (ToolCallMessage::TRUNCATE_LIMIT - 3)}...\" (+1 line)", msg.format
end

test "format renders EDIT with the path and edit count" do
msg = ToolCallMessage.new(
id: "1",
name: "edit",
arguments: {
path: "lib/roast.rb",
edits: [{ oldText: "a", newText: "b" }, { oldText: "c", newText: "d" }],
},
)
assert_equal "EDIT lib/roast.rb (2 edits)", msg.format
end

test "format renders a singular edit count for a single edit" do
msg = ToolCallMessage.new(
id: "1",
name: "edit",
arguments: { path: "config/app.yml", edits: [{ oldText: "a", newText: "b" }] },
)
assert_equal "EDIT config/app.yml (1 edit)", msg.format
end

test "format renders zero edits when the edits array is missing" do
msg = ToolCallMessage.new(id: "1", name: "edit", arguments: { path: "lib/roast.rb" })
assert_equal "EDIT lib/roast.rb (0 edits)", msg.format
end

test "format renders an unhandled tool as NAME key: value, ..." do
msg = ToolCallMessage.new(
id: "1",
Expand Down
25 changes: 25 additions & 0 deletions test/roast/cogs/agent/providers/pi/messages/tool_result_message_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,31 @@ def setup
assert_equal "WRITE OK", msg.format(@context)
end

test "format renders EDIT OK with the path from the originating call" do
@context.add_tool_call(
ToolCallMessage.new(id: "1", name: "edit", arguments: { path: "lib/roast/version.rb" }),
)
msg = ToolResultMessage.new(
tool_call_id: "1",
tool_name: "edit",
content: "The file has been updated successfully",
is_error: false,
)

assert_equal "EDIT OK lib/roast/version.rb", msg.format(@context)
end

test "format renders a bare EDIT OK when the path is unavailable" do
msg = ToolResultMessage.new(
tool_call_id: "1",
tool_name: "edit",
content: "The file has been updated successfully",
is_error: false,
)

assert_equal "EDIT OK", msg.format(@context)
end

test "format renders NAME ERROR with the message for an error result" do
msg = ToolResultMessage.new(
tool_call_id: "1",
Expand Down
Loading