KG-437 Use structured ModelInfo instead of 'provider:model' string #893

fazlerahmanejazi · 2025-10-01T01:38:02Z

Motivation and Context

KG-437

Swaps the "provider:model" string for a typed ModelInfo data class across agent events.
Added ModelInfo in agents-utils (no circular deps)
LLModel.toModelInfo() lives in prompt-llm
Kept eventString for backward compatibility
Wired into serialization; tests updated and green

Breaking Changes

No breaking changes for consumers using the old string—it's still parsed. New code should prefer ModelInfo.

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 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

kpavlov

Thank you, @fazlerahmanejazi
I have Couple of notes, but in general looks good to me

kpavlov · 2025-10-01T08:41:36Z

agents/agents-utils/src/commonMain/kotlin/ai/koog/agents/utils/ModelInfo.kt

+     * Backwards compatibility: provides the same string format as current eventString
+     * Format: "provider:model"
+     */
+    public val eventString: String get() = "$provider:$model"


Since it's use-case specific, wouldn't it be better to move it to an extension function in the events module? Or let's not mention "events" use case here (rename)

This will be deprecated, keeping it for backwards compatibility(improved docs and added deprecation warnings).

kpavlov · 2025-10-01T08:44:17Z

agents/agents-utils/src/commonTest/kotlin/ai/koog/agents/utils/ModelInfoTest.kt

+
+    @Test
+    fun `test fromString with invalid format`() {
+        val modelInfo = ModelInfo.fromString("invalid-format")


Should we create such malformed ModelInfos, or isn't it better to throw a ParseException. We may define a static constant ModelInfo.UNDEFINED for undefined usecases

Good point! Added ModelInfo.UNDEFINED

kpavlov · 2025-10-01T08:52:12Z

The ModelInfo might be reused for #633

sdubov

Thank you for your contribution. I have several comments about naming and deprecation. It would be nice to fix them. Other than that LGTM!

sdubov · 2025-10-01T09:07:24Z

agents/agents-utils/src/commonMain/kotlin/ai/koog/agents/utils/ModelInfo.kt

+     * Backwards compatibility: provides the same string format as current eventString
+     * Format: "provider:model"
+     */
+    public val eventString: String get() = "$provider:$model"


Let's deprecate this as well. I think we need to drop it in the next release in favour of a new format string: provider/model_id below.

sdubov · 2025-10-01T09:10:07Z

agents/agents-utils/src/commonMain/kotlin/ai/koog/agents/utils/ModelInfo.kt

+     * Human-readable display name for the model
+     * Falls back to "provider/model" if displayName is not provided
+     */
+    public val humanReadableName: String get() = displayName ?: "$provider/$model"


Maybe name it modelIdentifierName or modelIdentifierString?

sdubov · 2025-10-01T09:33:31Z

.../agents-core/src/commonMain/kotlin/ai/koog/agents/core/feature/model/events/llmCallEvents.kt

+     * @deprecated Use model.eventString instead
+     */
+    @Deprecated("Use model.eventString instead", ReplaceWith("model.eventString"))
+    public val modelString: String get() = model.eventString


Maybe also add a deprecated constructor to keep backward compatibility:

/** * @deprecated Use constructor with model parameter of type [ModelInfo]: * LLMCallStartingEvent(runId, prompt, model, tools, eventId, timestamp) */ @Deprecated( message = "Please use constructor with model parameter of type [ModelInfo]: LLMCallStartingEvent(runId, prompt, model, tools, eventId, timestamp)", replaceWith = ReplaceWith("LLMCallStartingEvent(runId, prompt, model, tools, eventId, timestamp)") ) public constructor( runId: String, prompt: Prompt, model: String, tools: List<String>, eventId: String = LLMCallStartingEvent::class.simpleName!!, timestamp: Long = Clock.System.now().toEpochMilliseconds() ) : this(runId, prompt, ModelInfo.fromString(model), tools, eventId, timestamp)

Good suggestion, done!

sdubov

Thank you for fixes. Approve!

kpavlov

Thank you, @fazlerahmanejazi

fazlerahmanejazi · 2025-10-01T18:42:03Z

@sdubov @kpavlov Could you also approve the workflows for this PR so checks can run? Thanks!

kpavlov · 2025-10-02T03:43:19Z

.../agents-core/src/commonMain/kotlin/ai/koog/agents/core/feature/model/events/llmCallEvents.kt

+     */
+    @Deprecated(
+        message = "Please use constructor with model parameter of type [ModelInfo]: LLMCallStartingEvent(runId, prompt, model, tools, eventId, timestamp)",
+        replaceWith = ReplaceWith("LLMCallStartingEvent(runId, prompt, model, tools, eventId, timestamp)")


eventId was removed from method signature. ReplaceWith and docs need an update

done, both #905 and #893 would need a workflow run!

…ility constructors

…tegy

…Test

kpavlov approved these changes Oct 1, 2025

View reviewed changes

kpavlov requested review from kpavlov and sdubov October 1, 2025 08:46

kpavlov added the enhancement New feature or request label Oct 1, 2025

sdubov requested changes Oct 1, 2025

View reviewed changes

fazlerahmanejazi requested a review from sdubov October 1, 2025 11:58

sdubov approved these changes Oct 1, 2025

View reviewed changes

kpavlov approved these changes Oct 1, 2025

View reviewed changes

kpavlov reviewed Oct 2, 2025

View reviewed changes

Ololoshechkin force-pushed the develop branch from 4ba5129 to 88b9d96 Compare October 2, 2025 22:15

fazle added 5 commits October 3, 2025 18:13

use structured ModelInfo instead of 'provider:model' string

f2ed0c2

add UNDEFINED constant, deprecate eventString, add backwards compatib…

1d9a41a

…ility constructors

resolve merge conflicts

165b5a9

fix build

85fc3aa

updated replaceWith and docs

f619567

fazlerahmanejazi force-pushed the kg-437-agent-events-modelinfo branch from 031e1ee to f619567 Compare October 3, 2025 16:15

fazle added 2 commits October 3, 2025 23:57

fix: update Qodana linter to compatible version 2025.2

9fad50e

resolve flaky PersistenceRunsTwiceTest by using correct rollback stra…

0511f31

…tegy

fazlerahmanejazi force-pushed the kg-437-agent-events-modelinfo branch from 88621f4 to 0511f31 Compare October 3, 2025 23:15

fazle added 2 commits October 4, 2025 01:55

fix: wait for server to be ready before client connection in Debugger…

e38becf

…Test

fix lint

6b845fc

fazlerahmanejazi force-pushed the kg-437-agent-events-modelinfo branch from b31feb5 to 6b845fc Compare October 4, 2025 01:04

KG-437 Use structured ModelInfo instead of 'provider:model' string #893

Are you sure you want to change the base?

KG-437 Use structured ModelInfo instead of 'provider:model' string #893

Uh oh!

Conversation

fazlerahmanejazi commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation and Context

Breaking Changes

Type of the changes

Checklist

Additional steps for pull requests adding a new feature

Uh oh!

kpavlov left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kpavlov commented Oct 1, 2025

Uh oh!

sdubov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sdubov left a comment

Choose a reason for hiding this comment

Uh oh!

kpavlov left a comment

Choose a reason for hiding this comment

Uh oh!

fazlerahmanejazi commented Oct 1, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fazlerahmanejazi Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fazlerahmanejazi commented Oct 1, 2025 •

edited

Loading

kpavlov left a comment •

edited

Loading

fazlerahmanejazi Oct 2, 2025 •

edited

Loading