Skip to content

Fix reasoning message handling in strategy #1166

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

Jump to bottom
Merged
merged 3 commits into from
Dec 1, 2025
Merged

Fix reasoning message handling in strategy #1166

merged 3 commits into from
Dec 1, 2025
+66 −11

Conversation

@devcrocod
Copy link
Contributor

@devcrocod devcrocod commented Nov 22, 2025

related to #1153

Motivation and Context

  • fix reasoning message in nodeLLMRequest
  • fix conditions on multiple requests
  • add onReasoningMessage and onMultipleReasoningMessage

Breaking Changes

None

Type of the changes

  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Tests improvement
  • Refactoring

Checklist

  • The pull request has a description of the proposed change
  • I read the Contributing Guidelines before opening the pull request
  • The pull request uses develop as the base branch
  • Tests for the changes have been added
  • All new and existing tests passed

Additional Context

To add tests need to modify and refactor mock executor

Copilot finished reviewing on behalf of devcrocod November 22, 2025 19:07
Copilot AI reviewed Nov 22, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes reasoning message handling in the agent strategy by introducing proper filtering capabilities and adding support for requesting multiple LLM responses without tools.

Key Changes:

  • Added skipReasoningMessage parameter to nodeLLMRequest to filter out reasoning messages when enabled
  • Introduced new edge filter functions onReasoningMessage and onMultipleReasoningMessages for handling reasoning message flows
  • Implemented requestLLMMultipleWithoutTools() methods in session classes to support multiple responses without tool usage
  • Fixed consistency issues by replacing .any() with .isNotEmpty() and adding missing empty checks

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
AIAgentNodes.kt Added skipReasoningMessage parameter and logic to filter reasoning messages from multiple LLM responses
AIAgentEdges.kt Added reasoning message edge filters and improved consistency of empty collection checks
AIAgentEdgeBuilder.kt Extended EdgeTransformationDslMarker annotation to classes for broader DSL marker coverage
AIAgentLLMWriteSession.kt Implemented requestLLMMultipleWithoutTools() to request and append multiple responses without tools
AIAgentLLMSession.kt Added base implementation of requestLLMMultipleWithoutTools() method
AIAgentSimpleStrategies.kt Reordered edge definitions for better code organization (no semantic change)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions
Copy link

github-actions bot commented Nov 22, 2025 •
edited
Loading

Qodana for JVM

1166 new problems were found

Inspection name Severity Problems
Check Kotlin and Java source code coverage 🔶 Warning 1155
Missing KDoc for public API declaration 🔶 Warning 11 
@@ Code coverage @@
+ 72% total lines covered
16784 lines analyzed, 12175 lines covered
# Calculated according to the filters of your coverage tool

☁️ View the detailed Qodana report

Contact Qodana team

Contact us at [email protected]

BLannoo
BLannoo requested changes Nov 25, 2025
View reviewed changes
Copy link
Contributor

@BLannoo BLannoo left a comment

Choose a reason for hiding this comment

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

If I understand it correctly the goal is to fix the issue described in #1153

And while this does look a lot cleaner solution and I would be interested in finding one more in this direction, it currently does not seem to fix the core issue:

Which is that

koog/examples/code-agent/step-01-minimal-agent/src/main/kotlin/Main.kt

Line 29 in a5c36d5

strategy = singleRunStrategy(ToolCalls.SEQUENTIAL),
fails when not providing ToolCalls.SEQUENTIAL as a parameter.

The error is still:
AIAgentStuckInTheNodeException: AI Agent has run into a problem: When executing agent graph, stuck in node nodeSendToolResult because output Reasoning(

Maybe merging in develop could help, because some other issue related to it was fixed recently, so the version in develop now works with: ToolCalls.SEQUENTIAL

But I am quite sure we would like it to work without that parameter too.

@devcrocod devcrocod force-pushed the devcrocod/fix-strategy-with-reasoning branch 3 times, most recently from 1f5bc4c to 8255de8 Compare November 26, 2025 15:25
@devcrocod devcrocod requested a review from BLannoo November 26, 2025 15:57
@devcrocod devcrocod force-pushed the devcrocod/fix-strategy-with-reasoning branch from 8255de8 to 549d800 Compare November 27, 2025 19:11
@devcrocod
Copy link
Contributor Author

devcrocod commented Nov 27, 2025

@BLannoo

And while this does look a lot cleaner solution and I would be interested in finding one more in this direction, it currently does not seem to fix the core issue:

I’ve fixed the issue, the exception was actually occurring in another node

@aozherelyeva
Copy link
Contributor

aozherelyeva commented Nov 28, 2025

Could you please cover changes with some basic unit-tests? Apart from this, the code looks good to me!

@devcrocod devcrocod force-pushed the devcrocod/fix-strategy-with-reasoning branch from c6daa9b to ed174ad Compare December 1, 2025 11:46
EugeneTheDev
EugeneTheDev approved these changes Dec 1, 2025
View reviewed changes
agents/agents-core/src/commonMain/kotlin/ai/koog/agents/core/agent/session/AIAgentLLMSession.kt
.let { preparePrompt(it, emptyList()) }

return executeSingle(promptWithDisabledTools, emptyList())
return executeMultiple(promptWithDisabledTools, emptyList()).first { it !is Message.Reasoning }
Copy link
Collaborator

@EugeneTheDev EugeneTheDev Dec 1, 2025

Choose a reason for hiding this comment

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

nit: seems like you can simplify this method by reusing requestLLMMultipleWithoutTools and only applying first

tiginamaria
tiginamaria approved these changes Dec 1, 2025
View reviewed changes
Copy link
Collaborator

@tiginamaria tiginamaria left a comment

Choose a reason for hiding this comment

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

Thank you! Looks good!

@devcrocod
Fix reasoning messages in nodes
b9d0b17 
- Add `onReasoningMessage` edge transformation for reasoning message filtering
- Reorder `onMultipleToolCalls` edge to prioritize execution flow
- Expand `EdgeTransformationDslMarker` target scope to include classes and annotate `AIAgentEdgeBuilderIntermediate`
- Add `skipReasoningMessage` support and `requestLLMMultipleWithoutTools` implementation
- Document `skipReasoningMessage` behavior in `nodeLLMRequest` API function
@devcrocod
Remove skipReasoningMessage parameter from LLM request functions an…
9264b61 
…d refactor logic
@devcrocod
Refactor LLM request functions to simplify logic by using superclass …
0828a67 
…implementation
@devcrocod devcrocod force-pushed the devcrocod/fix-strategy-with-reasoning branch from ed174ad to 0828a67 Compare December 1, 2025 17:00
BLannoo
BLannoo approved these changes Dec 1, 2025
View reviewed changes
Copy link
Contributor

@BLannoo BLannoo left a comment

Choose a reason for hiding this comment

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

Validated the codeagent step01 and step02 to work after the fix (including the SINGLE_RUN_SEQUENTIAL version)

@devcrocod devcrocod merged commit f247a0f into develop Dec 1, 2025
15 of 18 checks passed
@devcrocod devcrocod deleted the devcrocod/fix-strategy-with-reasoning branch December 1, 2025 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@BLannoo BLannoo BLannoo approved these changes

@tiginamaria tiginamaria tiginamaria approved these changes

@EugeneTheDev EugeneTheDev EugeneTheDev approved these changes

@aozherelyeva aozherelyeva aozherelyeva approved these changes

@denis-domanskii denis-domanskii Awaiting requested review from denis-domanskii

@Ololoshechkin Ololoshechkin Awaiting requested review from Ololoshechkin

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@devcrocod @aozherelyeva @BLannoo @tiginamaria @EugeneTheDev