Skip to content

Centralize tool result error handling in format with error_line helper#948

Open
LasmarKhalifa wants to merge 1 commit into
mainfrom
06-15/tool-result-error-handling
Open

Centralize tool result error handling in format with error_line helper#948
LasmarKhalifa wants to merge 1 commit into
mainfrom
06-15/tool-result-error-handling

Conversation

@LasmarKhalifa

@LasmarKhalifa LasmarKhalifa commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Part of #919

Foundation PR for the tool result formatter implementations.

@juniper-shopify juniper-shopify left a comment

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.

Nice foundation PR! The error-handling centralization is clean — pulling the error path into an early-return at the top of format means every downstream formatter only needs to think about the success case. Makes the rest of your stack cleaner.

Two small things to address:

  1. The error_line doc comment could use a couple of examples (see inline comment).
  2. A test for error_line when content is nil would round out the edge-case coverage.

And lets switch to the xml parser we had talked about

Comment thread lib/roast/cogs/agent/providers/claude/tool_result.rb
Comment thread test/roast/cogs/agent/providers/claude/tool_result_test.rb
Comment thread lib/roast/cogs/agent/providers/claude/tool_result.rb
@LasmarKhalifa LasmarKhalifa force-pushed the 06-15/tool-result-error-handling branch 4 times, most recently from 00641d0 to cc4bb2a Compare June 22, 2026 23:50
@LasmarKhalifa LasmarKhalifa force-pushed the 06-15/tool-result-error-handling branch from cc4bb2a to 2408fb9 Compare June 23, 2026 18:33
@LasmarKhalifa LasmarKhalifa marked this pull request as ready for review June 23, 2026 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants