Skip to content

Conversation

nomisRev
Copy link
Contributor

@nomisRev nomisRev commented Jun 6, 2025

Thank you for opening a pull request! Please add a brief description of the proposed change here.

Flow and suspend is typically redundant, but in this case no actual suspend was being used outside of Flow, so suspend could simply be removed. I moved all eager executing code into the Flow collector to be executed lazily, this aligns all implementations. There was also an instance of an unscoped launch, which is now properly scoped to the Flow execution.

Also, please tick the appropriate points in the checklist below.


Type of the change

  • New feature
  • Bug fix
  • Documentation fix

Checklist for all pull requests

  • 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 steps for pull requests adding a new feature
  • An issue describing the proposed change exists
  • The pull request includes a link to the issue
  • The change was discussed and approved in the issue
  • Docs have been added / updated

Copy link

github-actions bot commented Jun 6, 2025

Qodana for JVM

5 new problems were found

Inspection name Severity Problems
Vulnerable imported dependency 🔶 Warning 4
Return or assignment can be lifted out ◽️ Notice 1

☁️ View the detailed Qodana report

Detected 116 dependencies

Third-party software list

This page lists the third-party software dependencies used in koog-agents

Dependency Version Licenses
annotations 13.0 Apache-2.0
annotations 23.0.0 Apache-2.0
atomicfu-js 0.26.1 Apache-2.0
config 1.4.3 Apache-2.0
dokka-core 2.0.0 Apache-2.0
dokka-gradle-plugin 2.0.0 Apache-2.0
fus-statistics-gradle-plugin 2.1.21 Apache-2.0
jackson-annotations 2.12.7 Apache-2.0
jackson-core 2.12.7 Apache-2.0
jackson-databind 2.12.7.1 Apache-2.0
jackson-dataformat-xml 2.12.7 Apache-2.0
jackson-module-jaxb-annotations 2.12.7 Apache-2.0
jackson-module-kotlin 2.12.7 Apache-2.0
jakarta.activation-api 1.2.1 BSD-3-Clause
jakarta.xml.bind-api 2.3.2 BSD-3-Clause
jet-sign 45.47 Apache-2.0
kotlin-dom-api-compat 2.1.21 Apache-2.0
kotlin-gradle-plugin-api 2.1.21 Apache-2.0
kotlin-gradle-plugin-model 2.1.21 Apache-2.0
kotlin-gradle-plugin 2.1.21 Apache-2.0
kotlin-logging-js 7.0.7 Apache-2.0
kotlin-logging-jvm 7.0.7 Apache-2.0
kotlin-logging 7.0.7 Apache-2.0
kotlin-reflect 2.0.21 Apache-2.0
kotlin-reflect 2.1.21 Apache-2.0
kotlin-sdk-jvm 0.5.0 MIT
kotlin-stdlib-js 2.1.21 Apache-2.0
kotlin-stdlib 2.0.21 Apache-2.0
kotlin-stdlib 2.1.21 Apache-2.0
kotlinx-coroutines-core-js 1.10.2 Apache-2.0
kotlinx-coroutines-core-jvm 1.10.2 Apache-2.0
kotlinx-coroutines-core-jvm 1.8.0 Apache-2.0
kotlinx-coroutines-core 1.10.2 Apache-2.0
kotlinx-coroutines-reactive 1.10.2 Apache-2.0
kotlinx-coroutines-slf4j 1.10.2 Apache-2.0
kotlinx-datetime-js 0.6.2 Apache-2.0
kotlinx-datetime-jvm 0.6.2 Apache-2.0
kotlinx-datetime 0.6.2 Apache-2.0
kotlinx-io-bytestring-js 0.7.0 Apache-2.0
kotlinx-io-bytestring-jvm 0.7.0 Apache-2.0
kotlinx-io-bytestring 0.7.0 Apache-2.0
kotlinx-io-core-js 0.7.0 Apache-2.0
kotlinx-io-core-jvm 0.7.0 Apache-2.0
kotlinx-io-core 0.7.0 Apache-2.0
kotlinx-serialization-core-js 1.7.3 Apache-2.0
kotlinx-serialization-core-jvm 1.7.3 Apache-2.0
kotlinx-serialization-core 1.7.3 Apache-2.0
kotlinx-serialization-json-io-js 1.7.3 Apache-2.0
kotlinx-serialization-json-io-jvm 1.7.3 Apache-2.0
kotlinx-serialization-json-io 1.7.3 Apache-2.0
kotlinx-serialization-json-js 1.7.3 Apache-2.0
kotlinx-serialization-json-jvm 1.7.3 Apache-2.0
kotlinx-serialization-json 1.7.3 Apache-2.0
ktor-client-cio-jvm 3.0.3 Apache-2.0
ktor-client-content-negotiation-js 3.0.3 Apache-2.0
ktor-client-content-negotiation-jvm 3.0.3 Apache-2.0
ktor-client-content-negotiation 3.0.3 Apache-2.0
ktor-client-core-js 3.0.3 Apache-2.0
ktor-client-core-jvm 3.0.3 Apache-2.0
ktor-client-core 3.0.3 Apache-2.0
ktor-client-js-js 3.0.3 Apache-2.0
ktor-client-logging-js 3.0.3 Apache-2.0
ktor-client-logging-jvm 3.0.3 Apache-2.0
ktor-client-logging 3.0.3 Apache-2.0
ktor-events-js 3.0.3 Apache-2.0
ktor-events-jvm 3.0.3 Apache-2.0
ktor-events 3.0.3 Apache-2.0
ktor-http-cio-js 3.0.3 Apache-2.0
ktor-http-cio-jvm 3.0.3 Apache-2.0
ktor-http-cio 3.0.3 Apache-2.0
ktor-http-js 3.0.3 Apache-2.0
ktor-http-jvm 3.0.3 Apache-2.0
ktor-http 3.0.3 Apache-2.0
ktor-io-js 3.0.3 Apache-2.0
ktor-io-jvm 3.0.3 Apache-2.0
ktor-io 3.0.3 Apache-2.0
ktor-network-jvm 3.0.3 Apache-2.0
ktor-network-tls-jvm 3.0.3 Apache-2.0
ktor-serialization-js 3.0.3 Apache-2.0
ktor-serialization-jvm 3.0.3 Apache-2.0
ktor-serialization-kotlinx-js 3.0.3 Apache-2.0
ktor-serialization-kotlinx-json-js 3.0.3 Apache-2.0
ktor-serialization-kotlinx-json-jvm 3.0.3 Apache-2.0
ktor-serialization-kotlinx-json 3.0.3 Apache-2.0
ktor-serialization-kotlinx-jvm 3.0.3 Apache-2.0
ktor-serialization-kotlinx 3.0.3 Apache-2.0
ktor-serialization 3.0.3 Apache-2.0
ktor-server-cio-jvm 3.0.3 Apache-2.0
ktor-server-core-js 3.0.3 Apache-2.0
ktor-server-core-jvm 3.0.3 Apache-2.0
ktor-server-core 3.0.3 Apache-2.0
ktor-server-sse-js 3.0.3 Apache-2.0
ktor-server-sse-jvm 3.0.3 Apache-2.0
ktor-server-sse 3.0.3 Apache-2.0
ktor-server-websockets-jvm 3.0.2 Apache-2.0
ktor-sse-js 3.0.3 Apache-2.0
ktor-sse-jvm 3.0.3 Apache-2.0
ktor-sse 3.0.3 Apache-2.0
ktor-utils-js 3.0.3 Apache-2.0
ktor-utils-jvm 3.0.3 Apache-2.0
ktor-utils 3.0.3 Apache-2.0
ktor-websocket-serialization-js 3.0.3 Apache-2.0
ktor-websocket-serialization-jvm 3.0.3 Apache-2.0
ktor-websocket-serialization 3.0.3 Apache-2.0
ktor-websockets-js 3.0.3 Apache-2.0
ktor-websockets-jvm 3.0.3 Apache-2.0
ktor-websockets 3.0.3 Apache-2.0
lettuce-core 6.5.5.release MIT
logback-classic 1.5.13 EPL-1.0
LGPL-2.0-or-later
logback-core 1.5.13 EPL-1.0
LGPL-2.0-or-later
netty-common 4.1.118.final Apache-2.0
reactive-streams 1.0.4 MIT-0
reactor-core 3.6.6 Apache-2.0
slf4j-api 2.0.16 MIT
stax2-api 4.2.1 BSD-2-Clause
BSD-3-Clause
woodstox-core 6.2.4 Apache-2.0
Contact Qodana team

Contact us at [email protected]

@nomisRev
Copy link
Contributor Author

nomisRev commented Jun 8, 2025

The change made by Qodana are unrelated to my changes, but seems like a correct fix.

Should we make these changes separately, and rebase this PR?

@Rizzen
Copy link
Member

Rizzen commented Jun 8, 2025

@nomisRev
Hello! Thank you for that catch, I really like the suggested approach!
But I'm a bit concerned if suspend removal would obstruct custom users LLMClients. I mean of course they could use flow builder, but it looks like implicitly suspend method.

@EugeneTheDev @Ololoshechkin WDYT?

@EugeneTheDev
Copy link
Contributor

This looks good to me. Making the function both suspend and returning Flow from it seems indeed redundant

@Ololoshechkin
Copy link
Collaborator

@nomisRev please revert the Qodana change, and rebase to develop. As soon as your branch has qodana.yaml from develop, this change wouldn't happen :)

@nomisRev nomisRev force-pushed the remove-suspend-from-llmclient-streaming branch from 32809db to 59e3b3e Compare June 12, 2025 07:05
@nomisRev
Copy link
Contributor Author

nomisRev commented Jun 12, 2025

@Ololoshechkin Rebased, and all unrelated changes are now gone 👍

@Rizzen Rizzen merged commit 1547cb4 into develop Jun 12, 2025
5 checks passed
@Rizzen Rizzen deleted the remove-suspend-from-llmclient-streaming branch June 12, 2025 07:53
BLannoo pushed a commit to BLannoo/koog that referenced this pull request Jun 26, 2025
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.

4 participants