restructure Pi tool formatting into per-method dispatch with generic fallback

lib/roast/cogs/agent/providers/pi/messages/tool_call_message.rb

@@ -25,31 +25,53 @@ class ToolCallMessage
               def initialize(id:, name:, arguments:)
                 @id = id
                 @name = name
-                @arguments = arguments
+                @arguments = arguments.is_a?(Hash) ? arguments : {}
               end
 
               #: () -> String?
               def format
                 return unless name
 
-                case name.to_s.downcase
-                when "bash"
-                  "BASH #{arguments[:command]}"
-                when "read"
-                  "READ #{arguments[:path]}"
-                when "edit"
-                  "EDIT #{arguments[:path]}"
-                when "write"
-                  "WRITE #{arguments[:path]}"
-                when "grep"
-                  "GREP #{arguments[:pattern]} #{arguments[:path]}"
-                when "find"
-                  "FIND #{arguments[:pattern]} #{arguments[:path]}"
-                when "ls"
-                  "LS #{arguments[:path]}"
-                else
-                  "TOOL [#{name}] #{arguments.inspect}"
-                end
+                format_method_name = "format_#{name.to_s.downcase}".to_sym
+                return send(format_method_name) if respond_to?(format_method_name, true)
+
+                format_unknown
+              rescue StandardError
+                format_unknown
+              end
+
+              # Truncates each value in a formatted tool-call string to keep terminal output concise.
+              TRUNCATE_LIMIT = 45
+
+              private
+
+              # Formats a tool call for which Roast has no dedicated formatter.
+              #
+              # Output: "<NAME> <key>: <value>, ..." – the upcased tool name, then each
+              # argument as "<key>: <inspected value>", ordered shortest pair first so the
+              # most compact arguments stay visible, joined with ", ". Every value is
+              # truncated to TRUNCATE_LIMIT chars so one large argument can't flood the
+              # line; keys are always shown. No arguments renders the bare "<NAME>".
+              #
+              # Examples:
+              #   WEB_SEARCH max_results: 5, query: "ruby pluralize"
+              #   DEPLOY
+              #
+              #: () -> String
+              def format_unknown
+                label = name.to_s.upcase
+                return label if arguments.empty?
+
+                details = arguments.map { |key, value| "#{key}: #{truncate(value.inspect)}" }.sort_by { |s| [s.length, s] }.join(", ")
+                "#{label} #{details}"
+              end
+
+              # Truncates to TRUNCATE_LIMIT chars, appending "..." when cut. nil -> "".
+              #
+              #: (String?) -> String
+              def truncate(str)
+                s = str.to_s
+                s.length > TRUNCATE_LIMIT ? "#{s[0...TRUNCATE_LIMIT - 3]}..." : s
               end
             end
           end

lib/roast/cogs/agent/providers/pi/messages/tool_result_message.rb

@@ -30,23 +30,77 @@ def initialize(tool_call_id:, tool_name:, content:, is_error:)
                 @tool_name = tool_name
                 @content = content
                 @is_error = is_error
+                @name = (tool_name || "unknown").to_s #: String
+                @input = {} #: Hash[Symbol, untyped]
               end
 
               #: (PiInvocation::Context) -> String?
               def format(context)
-                tool_call = context.tool_call(tool_call_id)
-                name = tool_name || tool_call&.name || "unknown"
-                status = is_error ? "ERROR" : "OK"
-
-                # Truncate long tool results for progress display
-                c = content
-                display_content = if c && c.length > 200
-                  "#{c[0..197]}..."
-                else
-                  c
-                end
-
-                "#{name.upcase} #{status}#{display_content ? " #{display_content}" : ""}"
+                call = context.tool_call(tool_call_id)
+                @name = (tool_name || call&.name || "unknown").to_s
+                @input = call&.arguments || {}
+
+                return error_line if is_error
+
+                format_method_name = "format_#{@name.downcase}".to_sym
+                return send(format_method_name) if respond_to?(format_method_name, true)
+
+                format_unknown
+              rescue StandardError
+                format_unknown
+              end
+
+              # Truncates each element in a formatted tool-result string to keep terminal output concise.
+              TRUNCATE_LIMIT = 45
+
+              private
+
+              # Formats a result for which Roast has no dedicated formatter.
+              #
+              # Content: the tool's output text.
+              #
+              # Output: "<NAME> OK <preview>" – the first line of content, stripped and
+              # truncated to TRUNCATE_LIMIT chars. The preview is omitted when there is
+              # no content.
+              #
+              # Examples:
+              #   WEB_SEARCH OK 3 results for "ruby pluralize"
+              #   DEPLOY OK
+              #
+              #: () -> String
+              def format_unknown
+                preview = truncate(content.to_s.lines.first.to_s.strip)
+                ok_line(preview)
+              end
+
+              # Renders "<TOOL> OK[ <part> · <part> · ...]"; the success-side twin of
+              # #error_line. Blank/nil parts are dropped and the rest joined with " · ",
+              # so callers pass each piece of the summary without minding separators.
+              #
+              #: (*String?) -> String
+              def ok_line(*parts)
+                summary = parts.select(&:present?).join(" · ")
+                prefix = "#{@name.upcase} OK"
+                summary.present? ? "#{prefix} #{summary}" : prefix
+              end
+
+              # Renders "<TOOL> ERROR <message>". The content is shown as-is and intentionally not truncated,
+              # preserving the full diagnostic.
+              #
+              # Examples:
+              #   READ ERROR ENOENT: no such file or directory
+              #
+              #: () -> String
+              def error_line
+                "#{@name.upcase} ERROR #{content.to_s.strip}".strip
+              end
+
+              # Truncates to TRUNCATE_LIMIT chars, appending "..." when cut. nil -> "".
+              #
+              #: (String?) -> String
+              def truncate(str)
+                s = str.to_s
+                s.length > TRUNCATE_LIMIT ? "#{s[0...TRUNCATE_LIMIT - 3]}..." : s
               end
             end
           end

test/roast/cogs/agent/providers/pi/messages/tool_call_message_test.rb

@@ -6,49 +6,54 @@ module Roast
   module Cogs
     module Agent::Providers::Pi::Messages
       class ToolCallMessageTest < ActiveSupport::TestCase
-        test "format returns BASH with command for bash tool" do
-          msg = ToolCallMessage.new(id: "1", name: "bash", arguments: { command: "ls -la" })
-          assert_equal "BASH ls -la", msg.format
+        test "format returns nil when name is nil" do
+          msg = ToolCallMessage.new(id: "1", name: nil, arguments: {})
+          assert_nil msg.format
         end
 
-        test "format returns READ with path for read tool" do
-          msg = ToolCallMessage.new(id: "1", name: "read", arguments: { path: "/tmp/test.rb" })
-          assert_equal "READ /tmp/test.rb", msg.format
+        test "format renders an unhandled tool as NAME key: value, ..." do
+          msg = ToolCallMessage.new(
+            id: "1",
+            name: "web_search",
+            arguments: { query: "ruby pluralize", max_results: 5 },
+          )
+          assert_equal 'WEB_SEARCH max_results: 5, query: "ruby pluralize"', msg.format
         end
 
-        test "format returns EDIT with path for edit tool" do
-          msg = ToolCallMessage.new(id: "1", name: "edit", arguments: { path: "/tmp/test.rb" })
-          assert_equal "EDIT /tmp/test.rb", msg.format
+        test "format orders arguments by rendered pair length, shortest first" do
+          msg = ToolCallMessage.new(
+            id: "1",
+            name: "search",
+            arguments: { longest_key: "a", b: "medium value", c: 1 },
+          )
+          assert_equal 'SEARCH c: 1, longest_key: "a", b: "medium value"', msg.format
         end
 
-        test "format returns WRITE with path for write tool" do
-          msg = ToolCallMessage.new(id: "1", name: "write", arguments: { path: "/tmp/test.rb" })
-          assert_equal "WRITE /tmp/test.rb", msg.format
+        test "format renders the bare name when an unhandled tool has no arguments" do
+          msg = ToolCallMessage.new(id: "1", name: "deploy", arguments: {})
+          assert_equal "DEPLOY", msg.format
         end
 
-        test "format returns GREP with pattern for grep tool" do
-          msg = ToolCallMessage.new(id: "1", name: "grep", arguments: { pattern: "foo", path: "." })
-          assert_equal "GREP foo .", msg.format
+        test "format truncates a long argument value" do
+          msg = ToolCallMessage.new(id: "1", name: "embed", arguments: { text: "x" * 100 })
+          assert_equal "EMBED text: #{("x" * 100).inspect[0, ToolCallMessage::TRUNCATE_LIMIT - 3]}...", msg.format
         end
 
-        test "format returns FIND with pattern for find tool" do
-          msg = ToolCallMessage.new(id: "1", name: "find", arguments: { pattern: "*.rb", path: "." })
-          assert_equal "FIND *.rb .", msg.format
+        test "format normalizes non-hash arguments instead of raising" do
+          msg = ToolCallMessage.new(id: "1", name: "deploy", arguments: "not a hash")
+          assert_equal "DEPLOY", msg.format
         end
 
-        test "format returns LS with path for ls tool" do
-          msg = ToolCallMessage.new(id: "1", name: "ls", arguments: { path: "/tmp" })
-          assert_equal "LS /tmp", msg.format
-        end
+        test "format falls back to the generic formatter when a dedicated formatter raises" do
+          klass = Class.new(ToolCallMessage) do
+            private
 
-        test "format returns TOOL with name and args for unknown tools" do
-          msg = ToolCallMessage.new(id: "1", name: "custom_tool", arguments: { key: "value" })
-          assert_equal "TOOL [custom_tool] #{{ key: "value" }.inspect}", msg.format
-        end
-
-        test "format returns nil when name is nil" do
-          msg = ToolCallMessage.new(id: "1", name: nil, arguments: {})
-          assert_nil msg.format
+            def format_boom
+              raise "boom"
+            end
+          end
+          msg = klass.new(id: "1", name: "boom", arguments: { a: 1 })
+          assert_equal "BOOM a: 1", msg.format
         end
       end
     end

test/roast/cogs/agent/providers/pi/messages/tool_result_message_test.rb

@@ -10,56 +10,75 @@ def setup
           @context = Roast::Cogs::Agent::Providers::Pi::PiInvocation::Context.new
         end
 
-        test "format returns tool name and OK status for successful result" do
+        test "format renders NAME ERROR with the message for an error result" do
           msg = ToolResultMessage.new(
             tool_call_id: "1",
             tool_name: "bash",
-            content: "file1.rb\nfile2.rb",
-            is_error: false,
+            content: "command not found",
+            is_error: true,
+          )
+
+          assert_equal "BASH ERROR command not found", msg.format(@context)
+        end
+
+        test "format does not truncate an error message" do
+          long_message = "boom " * 60
+          msg = ToolResultMessage.new(
+            tool_call_id: "1",
+            tool_name: "read",
+            content: long_message,
+            is_error: true,
           )
 
-          assert_equal "BASH OK file1.rb\nfile2.rb", msg.format(@context)
+          assert_equal "READ ERROR #{long_message.strip}", msg.format(@context)
         end
 
-        test "format returns tool name and ERROR status for error result" do
+        test "format renders a bare NAME ERROR when there is no content" do
           msg = ToolResultMessage.new(
             tool_call_id: "1",
             tool_name: "bash",
-            content: "command not found",
+            content: nil,
             is_error: true,
           )
 
-          assert_equal "BASH ERROR command not found", msg.format(@context)
+          assert_equal "BASH ERROR", msg.format(@context)
         end
 
-        test "format truncates long content" do
-          long_content = "x" * 300
+        test "format renders NAME OK with a one-line preview for an unhandled tool" do
           msg = ToolResultMessage.new(
             tool_call_id: "1",
-            tool_name: "read",
-            content: long_content,
+            tool_name: "web_search",
+            content: "3 results\nmore detail",
             is_error: false,
           )
 
-          result = msg.format(@context)
-          assert result.length < 220
-          assert result.end_with?("...")
+          assert_equal "WEB_SEARCH OK 3 results", msg.format(@context)
         end
 
-        test "format handles nil content" do
+        test "format renders a bare NAME OK when there is no content" do
           msg = ToolResultMessage.new(
             tool_call_id: "1",
-            tool_name: "bash",
+            tool_name: "deploy",
             content: nil,
             is_error: false,
           )
 
-          assert_equal "BASH OK", msg.format(@context)
+          assert_equal "DEPLOY OK", msg.format(@context)
+        end
+
+        test "format truncates a long preview" do
+          msg = ToolResultMessage.new(
+            tool_call_id: "1",
+            tool_name: "web_search",
+            content: "x" * 300,
+            is_error: false,
+          )
+
+          assert_equal "WEB_SEARCH OK #{"x" * (ToolResultMessage::TRUNCATE_LIMIT - 3)}...", msg.format(@context)
         end
 
-        test "format uses tool name from context when tool_name is nil" do
-          tool_call = ToolCallMessage.new(id: "1", name: "edit", arguments: {})
-          @context.add_tool_call(tool_call)
+        test "format resolves the tool name from the originating call when the result omits it" do
+          @context.add_tool_call(ToolCallMessage.new(id: "1", name: "deploy", arguments: {}))
 
           msg = ToolResultMessage.new(
             tool_call_id: "1",
@@ -68,20 +87,31 @@ def setup
             is_error: false,
           )
 
-          result = msg.format(@context)
-          assert result.start_with?("EDIT")
+          assert_equal "DEPLOY OK done", msg.format(@context)
         end
 
-        test "format falls back to unknown when no tool name available" do
+        test "format falls back to UNKNOWN when no tool name is available" do
           msg = ToolResultMessage.new(
             tool_call_id: "nonexistent",
             tool_name: nil,
             content: "result",
             is_error: false,
           )
 
-          result = msg.format(@context)
-          assert result.start_with?("UNKNOWN")
+          assert_equal "UNKNOWN OK result", msg.format(@context)
+        end
+
+        test "format falls back to the generic formatter when a dedicated formatter raises" do
+          klass = Class.new(ToolResultMessage) do
+            private
+
+            def format_boom
+              raise "boom"
+            end
+          end
+          msg = klass.new(tool_call_id: "1", tool_name: "boom", content: "some output", is_error: false)
+
+          assert_equal "BOOM OK some output", msg.format(@context)
         end
       end
     end