[onechat] add tool progression events by pgayvallet · Pull Request #233724 · elastic/kibana

pgayvallet · 2025-09-02T11:40:29Z

Summary

Fix https://github.com/elastic/search-team/issues/10841

Add a new tool_progression type of event

export interface ToolProgressEventData {
  tool_call_id: string;
  message: string;
}

Change the structure of persisted tool calls to include a list of progression events
Expose a dedicated event emitter to tool handlers for them to send progression events
Add example of such events for the .search tool

pgayvallet · 2025-09-02T11:50:58Z

/ci

…progression-events

pgayvallet · 2025-09-02T18:30:55Z

/ci

pgayvallet · 2025-09-02T18:43:11Z

/ci

pgayvallet

self-review

pgayvallet · 2025-09-02T18:32:10Z

x-pack/platform/packages/shared/onechat/onechat-common/chat/conversation.ts

 /**
 * Defines all possible types for round steps.
 */
-export type ConversationRoundStep<T = ToolResult[]> = ToolCallStep<T> | ReasoningStep;


Out of the scope of the issue, but I took the opportunity to remove the arguably not useful generics around conversation round types.

pgayvallet · 2025-09-02T18:34:50Z

x-pack/platform/packages/shared/onechat/onechat-common/chat/events.ts

+export interface ToolProgressEventData {
+  tool_call_id: string;
+  message: string;
+}


The new tool_progress event, having the tool_call_id to be able to identify which tool call it's attached to (similar to tool_result events), and a plain text message.

We can make that evolve into something more structured later, but for now, simpler is better, and full text is gonna be fine ihmo.

pgayvallet · 2025-09-02T18:36:16Z

x-pack/platform/packages/shared/onechat/onechat-genai-utils/langchain/tools.ts

+      if (sendEvent) {
+        const toolCallId = config.configurable?.tool_call_id ?? config.toolCall?.id ?? 'unknown';


This one took time to find, but we do have the info of the tool_call_id within langchain wrappers, so we can "automatically" attach it to the underlying tool events.

pgayvallet · 2025-09-02T18:36:42Z

x-pack/platform/packages/shared/onechat/onechat-genai-utils/tools/search/graph.ts

  const toolNode = new ToolNode<typeof StateAnnotation.State.messages>(tools);

  const selectAndValidateIndex = async (state: StateType) => {
+    events?.reportProgress(progressMessages.selectingTarget());


Example of reporting progress for the search tool

pgayvallet · 2025-09-02T18:37:59Z

x-pack/platform/packages/shared/onechat/onechat-server/agents/runner.ts

-  /** ID of this run */
-  runId: string;


Was not being useful in any way after 6 months (it was meant to be used as "traceId" but we are using OTel directly for that now...), so removed it

pgayvallet · 2025-09-02T18:41:36Z

x-pack/platform/plugins/shared/onechat/server/services/conversation/converters.ts

+        'results' in step
+          ? JSON.parse(step.results).map((result: ToolResult) => {
+              if (step.type === ConversationRoundStepType.toolCall) {
+                return {
+                  ...step,
+                  progression: step.progression ?? [],
+                };
+              }
+              return step;
+            })
+          : [],


We'll switch to a more "elegant" way of populating non-backfilled fields later. But the idea is to add that progression: [] attribute during deserialization instead of making it optional on our domain model.

jedrazb

Only one question about ser/de progression, how can i validate that this works? would ftr tests cover this?

jedrazb · 2025-09-03T10:36:05Z

x-pack/platform/plugins/shared/onechat/server/services/conversation/converters.ts

+          ? JSON.parse(step.results).map((result: ToolResult) => {
+              if (step.type === ConversationRoundStepType.toolCall) {
+                return {
+                  ...step,


why do we return step (outer object) instead of result in the map function? result is not used anywhere which feels like a bug?

I'm lost why we are mixing up results, step and progression under results, while in the ToolCallWithResult results and progression are on the same level

Yeah it's definitely a bug, nice catch. I did too many things refactoring what Soren did while adding that backfilling for the progress array... I'll fix that, thank you

Fixed in fa9770e

…progression-events

pgayvallet · 2025-09-04T07:30:34Z

Only one question about ser/de progression, how can i validate that this works? would ftr tests cover this?

We can't have FTR tests for anything leveraging the chat API / an LLM atm, unfortunately.

How I tested was:

I look at the converse/async SSE response from the debugger console, and observe we are emitting the tool_progress events
once the current round is over, I check the call reloading the conversation to make sure that the progression array is populated in the round's tool call

jedrazb

LGTM

elasticmachine · 2025-09-04T10:11:20Z

💚 Build Succeeded

Buildkite Build
Commit: 2f88fe7

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`@kbn/onechat-common`	161	167	+6
`@kbn/onechat-genai-utils`	103	104	+1
total			+7

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`onechat`	829.0KB	829.0KB	+31.0B

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`@kbn/onechat-server`	2	4	+2

Unknown metric groups

API count

id	before	after	diff
`@kbn/onechat-common`	243	252	+9
`@kbn/onechat-genai-utils`	115	116	+1
`@kbn/onechat-server`	127	126	-1
total			+9

History

💚 Build #335093 succeeded 5b160c7
💔 Build #334902 failed 102f6af

joemcelroy · 2025-09-04T15:28:44Z

tested works well!

## Summary Fix elastic/search-team#10841 - Add a new `tool_progression` type of event ```ts export interface ToolProgressEventData { tool_call_id: string; message: string; } ``` - Change the structure of persisted tool calls to include a list of progression events - Expose a dedicated event emitter to tool handlers for them to send progression events - Add example of such events for the `.search` tool <img width="944" height="303" alt="Screenshot 2025-09-02 at 20 09 15" src="https://github.com/user-attachments/assets/776a7b13-9d68-4255-9a5c-d27189db208e" /> --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

pgayvallet added 2 commits September 2, 2025 13:39

[onechat] add tool progression events

957db18

fix unit tests

102f6af

pgayvallet added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting v9.2.0 labels Sep 2, 2025

pgayvallet added 3 commits September 2, 2025 20:10

implement progress for search tool

8111e18

fix unit tests

85a17d5

Merge remote-tracking branch 'upstream/main' into onechat-10841-tool-…

ef34480

…progression-events

pgayvallet commented Sep 2, 2025

View reviewed changes

pgayvallet marked this pull request as ready for review September 2, 2025 18:45

pgayvallet requested a review from a team as a code owner September 2, 2025 18:45

jedrazb reviewed Sep 3, 2025

View reviewed changes

pgayvallet added 3 commits September 4, 2025 08:25

Merge remote-tracking branch 'upstream/main' into onechat-10841-tool-…

5089269

…progression-events

fix persistence serialization bug

fa9770e

lint

2f5bcf8

pgayvallet force-pushed the onechat-10841-tool-progression-events branch from 5b160c7 to 2f5bcf8 Compare September 4, 2025 07:27

pgayvallet requested a review from jedrazb September 4, 2025 07:28

[CI] Auto-commit changed files from 'node scripts/yarn_deduplicate'

2f88fe7

jedrazb approved these changes Sep 4, 2025

View reviewed changes

joemcelroy approved these changes Sep 4, 2025

View reviewed changes

joemcelroy merged commit 2e77415 into elastic:main Sep 5, 2025
12 checks passed

		if (sendEvent) {
		const toolCallId = config.configurable?.tool_call_id ?? config.toolCall?.id ?? 'unknown';

Conversation

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

Uh oh!

Summary

Uh oh!

pgayvallet commented Sep 2, 2025

Uh oh!

pgayvallet commented Sep 2, 2025

Uh oh!

pgayvallet commented Sep 2, 2025

Uh oh!

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

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

pgayvallet commented Sep 4, 2025

Uh oh!

jedrazb left a comment

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Sep 4, 2025

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Async chunks

Public APIs missing exports

API count

History

Uh oh!

joemcelroy commented Sep 4, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

pgayvallet commented Sep 2, 2025 •

edited

Loading

jedrazb left a comment •

edited

Loading