fix(worker): prevent orphaned tool_results and fix parallel merging by zmanian · Pull Request #1069 · nearai/ironclaw

zmanian · 2026-03-12T19:46:55Z

Summary

Clean reimplementation of the core fixes from #635, scoped to only the two bugs identified in the PR title. The Telegram, scheduler, state machine, and empty-response changes from #635 are excluded per maintainer feedback about PR scope.

Reasoning text preserved: JobDelegate::call_llm() now extracts the reasoning text from select_tools() results and passes it as content in RespondResult::ToolCalls. This ensures the assistant_with_tool_calls message includes the LLM's reasoning, preventing context loss on subsequent turns.
Parallel tool results merged: rig_adapter::convert_messages() now merges consecutive Role::Tool messages into a single RigMessage::User with multiple ToolResult content items. Without this, parallel tool execution produced consecutive User messages that Anthropic's API rejects.

Addresses review feedback from #635:

Regression tests included for both bug fixes
Minimal, focused scope (2 files changed)
No .unwrap() or .expect() in production code
Zero clippy warnings

Supersedes #635 (core bug fixes only).

Test plan

cargo check passes
cargo test --lib worker -- all 47 tests pass including new regression tests
cargo test --lib rig_adapter -- all 29 tests pass including new regression tests
cargo clippy --all --tests -- zero warnings
Manual verification with Anthropic API that parallel tool calls no longer produce empty responses

Generated with Claude Code

gemini-code-assist · 2026-03-12T19:47:10Z

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refines the handling of LLM tool interactions by addressing two critical bugs. It ensures that the LLM's reasoning is consistently preserved across turns and correctly merges parallel tool execution results to maintain compatibility with external APIs. The changes are a focused re-implementation of core fixes from a previous, broader pull request, emphasizing stability and correctness.

Highlights

Orphaned Tool Results Prevention: The JobDelegate::call_llm() function now extracts reasoning text from select_tools() results and includes it as content in RespondResult::ToolCalls. This ensures the assistant_with_tool_calls message retains the LLM's reasoning, preventing context loss in subsequent turns and addressing issues with orphaned tool results.
Parallel Tool Results Merging: The rig_adapter::convert_messages() function has been updated to merge consecutive Role::Tool messages into a single RigMessage::User containing multiple ToolResult content items. This resolves an issue where Anthropic's API would reject requests due to consecutive User messages resulting from parallel tool execution.
Regression Tests: New regression tests have been added for both bug fixes to ensure the stability and correctness of the implemented changes.
Code Quality: The changes maintain a minimal and focused scope, avoid the use of .unwrap() or .expect() in production code, and introduce zero clippy warnings, adhering to high code quality standards.

Changelog

src/llm/rig_adapter.rs
- Modified convert_messages to merge consecutive Role::Tool messages into a single RigMessage::User when converting chat messages.
- Added test_consecutive_tool_results_merged_into_single_user_message to verify the merging logic for parallel tool results.
- Added test_tool_result_after_user_text_not_merged to ensure tool results are not merged after non-tool user messages.
src/worker/job.rs
- Updated JobDelegate::call_llm to extract and preserve the LLM's reasoning text from ToolSelection results, passing it as content in RespondResult::ToolCalls.
- Added test_selections_to_tool_calls_preserves_ids to confirm that tool call IDs are correctly preserved during conversion, preventing orphaned tool results.
- Added test_reasoning_text_extraction_from_selections to validate the extraction of reasoning text from tool selections and handle empty reasoning appropriately.

Activity

This pull request is a clean reimplementation of core fixes from a previous PR (fix: orphaned tool_results and parallel tool_result merging in Worker #635), specifically scoping to the two identified bugs.
The scope was narrowed based on maintainer feedback from the previous PR, excluding Telegram, scheduler, state machine, and empty-response changes.
Review feedback from fix: orphaned tool_results and parallel tool_result merging in Worker #635 regarding regression tests, minimal scope, and avoidance of .unwrap()/.expect() has been addressed.
The PR supersedes fix: orphaned tool_results and parallel tool_result merging in Worker #635 for its core bug fixes.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces two important fixes for tool handling: merging consecutive tool results to support APIs like Anthropic, and preserving the LLM's reasoning text when tool calls are made. The changes are well-implemented and include corresponding regression tests. I've identified a potential issue where reasoning context could be lost if the first of several parallel tool selections has empty reasoning, and I've provided a suggestion to make this more robust. Additionally, tests should be updated to reflect this improved logic and ensure comprehensive coverage, aligning with best practices for test maintenance.

gemini-code-assist · 2026-03-12T19:49:50Z

+                let reasoning_text = s
+                    .first()
+                    .map(|sel| sel.reasoning.clone())
+                    .filter(|r| !r.is_empty());


The current implementation only considers the reasoning from the first ToolSelection. If the first selection has empty reasoning but a subsequent one has non-empty reasoning, this context will be lost. To make this more robust, you should iterate through all selections and use the first non-empty reasoning string found.

Suggested change

let reasoning_text = s

.first()

.map(|sel| sel.reasoning.clone())

.filter(|r| !r.is_empty());

let reasoning_text = s

.iter()

.find_map(|sel| (!sel.reasoning.is_empty()).then_some(sel.reasoning.clone()));

gemini-code-assist · 2026-03-12T19:49:51Z

+    fn test_reasoning_text_extraction_from_selections() {
+        // Simulate what call_llm does: extract reasoning from first selection
+        let selections = [
+            ToolSelection {
+                tool_name: "search".into(),
+                parameters: serde_json::json!({}),
+                reasoning: "I need to search for relevant information".into(),
+                alternatives: vec![],
+                tool_call_id: "call_1".into(),
+            },
+            ToolSelection {
+                tool_name: "fetch".into(),
+                parameters: serde_json::json!({}),
+                reasoning: "I need to search for relevant information".into(),
+                alternatives: vec![],
+                tool_call_id: "call_2".into(),
+            },
+        ];
+
+        let reasoning_text = selections
+            .first()
+            .map(|sel| sel.reasoning.clone())
+            .filter(|r| !r.is_empty());
+
+        assert_eq!(
+            reasoning_text.as_deref(),
+            Some("I need to search for relevant information"),
+            "Reasoning text should be extracted from first selection"
+        );
+
+        // Empty reasoning should result in None
+        let empty_selections = [ToolSelection {
+            tool_name: "echo".into(),
+            parameters: serde_json::json!({}),
+            reasoning: String::new(),
+            alternatives: vec![],
+            tool_call_id: "call_3".into(),
+        }];
+
+        let empty_reasoning = empty_selections
+            .first()
+            .map(|sel| sel.reasoning.clone())
+            .filter(|r| !r.is_empty());
+
+        assert!(
+            empty_reasoning.is_none(),
+            "Empty reasoning should not be included as content"
+        );
+    }


This test currently simulates the logic of only taking reasoning from the first selection. To align with the suggested improvement of finding the first non-empty reasoning from any selection, this test should be updated.

Specifically:

Update the logic inside the test to use find_map as suggested for the implementation.

Add a new test case to verify that reasoning is correctly extracted when the first selection has empty reasoning but a subsequent one has non-empty reasoning.

Example of the new test case:

let selections_with_first_empty = [ ToolSelection { tool_name: "search".into(), parameters: serde_json::json!({}), reasoning: "".into(), alternatives: vec![], tool_call_id: "call_1".into(), }, ToolSelection { tool_name: "fetch".into(), parameters: serde_json::json!({}), reasoning: "This reasoning should be picked up".into(), alternatives: vec![], tool_call_id: "call_2".into(), }, ]; let reasoning_text = selections_with_first_empty .iter() .find_map(|sel| (!sel.reasoning.is_empty()).then_some(sel.reasoning.clone())); assert_eq!( reasoning_text.as_deref(), Some("This reasoning should be picked up") );

References

When refactoring, ensure that tests for generic functionality are updated to cover the new data structures, preventing loss of test coverage.

Two fixes for tool result handling in the Worker: 1. Preserve reasoning text from select_tools() in the RespondResult content field so it appears in the assistant_with_tool_calls message pushed by execute_tool_calls. Without this, the LLM's reasoning context was lost when using the select_tools path. 2. Merge consecutive tool_result messages into a single User message in rig_adapter's convert_messages(). When parallel tools execute, each produces a separate ChatMessage with role: Tool. Without merging, these become consecutive User messages which Anthropic rejects. Now consecutive tool results are merged into one User message with multiple ToolResult content items. Includes regression tests for both fixes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

The previous code only checked the first ToolSelection's reasoning, missing cases where the first selection has empty reasoning but subsequent ones do not. Switch to find_map to get the first non-empty reasoning across all selections. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…earai#1069) * fix(worker): prevent orphaned tool_results and fix parallel merging Two fixes for tool result handling in the Worker: 1. Preserve reasoning text from select_tools() in the RespondResult content field so it appears in the assistant_with_tool_calls message pushed by execute_tool_calls. Without this, the LLM's reasoning context was lost when using the select_tools path. 2. Merge consecutive tool_result messages into a single User message in rig_adapter's convert_messages(). When parallel tools execute, each produces a separate ChatMessage with role: Tool. Without merging, these become consecutive User messages which Anthropic rejects. Now consecutive tool results are merged into one User message with multiple ToolResult content items. Includes regression tests for both fixes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(worker): use find_map for first non-empty reasoning extraction The previous code only checked the first ToolSelection's reasoning, missing cases where the first selection has empty reasoning but subsequent ones do not. Switch to find_map to get the first non-empty reasoning across all selections. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

github-actions Bot added scope: llm LLM integration scope: worker Container worker size: M 50-199 changed lines labels Mar 12, 2026

github-actions Bot added risk: medium Business logic, config, or moderate-risk modules contributor: core 20+ merged PRs labels Mar 12, 2026

zmanian enabled auto-merge (squash) March 12, 2026 19:47

gemini-code-assist Bot reviewed Mar 12, 2026

View reviewed changes

zmanian force-pushed the fix/635-orphaned-tool-results branch from c27245f to c0c95ec Compare March 12, 2026 20:26

zmanian requested review from henrypark133 and ilblackdragon March 12, 2026 21:17

github-actions Bot added size: L 200-499 changed lines and removed size: M 50-199 changed lines labels Mar 15, 2026

ilblackdragon approved these changes Mar 16, 2026

View reviewed changes

zmanian merged commit 58a3eb1 into staging Mar 16, 2026
14 checks passed

zmanian deleted the fix/635-orphaned-tool-results branch March 16, 2026 07:51

ironclaw-ci Bot mentioned this pull request Mar 16, 2026

chore: promote staging to staging-promote/57c397bd-23120362128 (2026-03-16 08:20 UTC) #1239

Merged

ironclaw-ci Bot mentioned this pull request Mar 17, 2026

chore: release v0.19.0 #973

Merged

ilblackdragon mentioned this pull request Mar 21, 2026

fix: orphaned tool_results and parallel tool_result merging in Worker #635

Closed

3 tasks done

Navigation Menu

Repository navigation

fix(worker): prevent orphaned tool_results and fix parallel merging - #1069

fix(worker): prevent orphaned tool_results and fix parallel merging#1069
zmanian merged 2 commits into
stagingfrom
fix/635-orphaned-tool-results

zmanian commented Mar 12, 2026

Uh oh!

gemini-code-assist Bot commented Mar 12, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Mar 12, 2026

Uh oh!

gemini-code-assist Bot Mar 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

zmanian commented Mar 12, 2026

Summary

Test plan

Uh oh!

gemini-code-assist Bot commented Mar 12, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants