Add helper trait for CSV and Markdown tool responses

[kargnas]

Summary

• add a ToolResponse value object and FormatsTabularToolResponses trait for re-usable CSV/Markdown conversions
• update ToolsCallHandler to accept structured ToolResponse payloads and respect pre-formatted content
• document the new workflow and cover it with trait/unit tests plus a fixture tool exercising CSV and Markdown responses

Testing

• vendor/bin/pest

---

https://chatgpt.com/codex/tasks/task_e_68eaa6eaf4cc8321bf22d94ab6e4791d

[claude]

Pull Request Review: Add helper trait for CSV and Markdown tool responses

Overview

This PR introduces a well-designed system for returning structured tabular data from MCP tools. The implementation is solid with good attention to detail. Here is my comprehensive review:

---

✅ Strengths

1. Excellent Architecture

• ToolResponse Value Object (src/Services/ToolService/ToolResponse.php:10): Clean immutable design with proper validation
• Trait-based Approach: Opt-in pattern is perfect - tools only include the trait when needed
• Separation of Concerns: Clear distinction between formatting logic (trait) and response structure (value object)

2. Robust Implementation

• Input Validation: Strong validation in ToolResponse constructor prevents invalid states
• Type Safety: Comprehensive PHPDoc annotations with PHPStan-compatible types
• Error Handling: Appropriate InvalidArgumentException usage with descriptive messages

3. Good Test Coverage

• Integration tests in StreamableHttpTest.php verify end-to-end functionality
• Unit tests for the trait cover key scenarios
• Fixture tool demonstrates real-world usage

4. Documentation

• README.md update provides clear usage examples
• Inline code comments explain complex logic
• British spelling (normalise) is consistent throughout

---

🔍 Issues & Recommendations

Critical: Missing Unit Tests for ToolResponse Class

The ToolResponse class lacks dedicated unit tests. Recommend adding tests/Services/ToolService/ToolResponseTest.php with tests for:

• ToolResponse::text creates valid response
• ToolResponse throws when metadata contains content key
• ToolResponse validates content structure with invalid input

Medium: Expand Trait Test Coverage

Current trait tests (tests/Services/ToolService/Concerns/FormatsTabularToolResponsesTest.php:1) are minimal. Consider adding:

1. Edge Cases:

   • Empty arrays: toCsv([])
   • Scalar lists: toCsv([1, 2, 3])
   • Mixed column handling with explicit columns parameter
   • Custom CSV delimiters/enclosures
   • Stringable objects

2. Markdown Escaping:

   • Test cells with pipe characters: ['col' => 'foo|bar']
   • Test cells with newlines
   • Test cells with special markdown characters

3. Error Conditions:

   • Empty columns array (already validated line 176)
   • Large datasets (performance)

Low: Potential Enhancement - Resource Management

In FormatsTabularToolResponses::toCsv() (src/Services/ToolService/Concerns/FormatsTabularToolResponses.php:76-94), consider wrapping file operations in try-finally to guarantee resource cleanup if an exception occurs during processing.

Low: Documentation Enhancement

The README example (README.md:969-990) could benefit from mentioning:

• Performance characteristics (suitable for datasets of what size?)
• Memory considerations when using CSV vs Markdown
• Whether the MIME types are MCP protocol standard or package-specific

---

🔒 Security Assessment

✅ No security concerns identified:

• Proper CSV escaping via fputcsv (handles injection attacks)
• Markdown escaping prevents XSS in rendered contexts
• No user-controlled file paths or command execution
• Input validation prevents malformed data structures

---

🎯 Performance Considerations

Good:

• Uses php://temp for CSV generation (memory-efficient)
• Single-pass markdown generation
• Minimal string concatenation overhead

Consider:

• For very large datasets (10k+ rows), the trait normalizes ALL rows in memory
• Recommend documenting recommended dataset size limits
• Future enhancement: Consider streaming interfaces for large datasets

---

📊 Code Quality

PHPStan Compliance: ✅ Excellent type annotations throughout
Laravel Pint: Should pass (consistent with codebase style)
Naming Conventions: ✅ Consistent with Laravel/PHP standards
SOLID Principles: ✅ Well-applied (Single Responsibility, Open/Closed)

---

🎬 Backward Compatibility

✅ Fully backward compatible:

• Existing tools continue to work without changes
• ToolsCallHandler gracefully handles both old and new response formats (src/Server/Request/ToolsCallHandler.php:67-92)
• Optional trait means no breaking changes

---

✅ Final Verdict

Recommendation: APPROVE with minor improvements

This is high-quality code that adds valuable functionality. The architecture is sound and the implementation is robust. The main actionable items are:

1. Add unit tests for ToolResponse class (critical for comprehensive coverage)
2. Expand trait test coverage for edge cases
3. Consider resource cleanup enhancement in CSV generation
4. Minor documentation additions about performance characteristics

The PR is ready to merge after addressing the test coverage gap. Great work! 🎉