[Sig Events] Add significant events management skill + tools#271430
[Sig Events] Add significant events management skill + tools#271430mykolaharmash wants to merge 9 commits into
Conversation
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@mykolaharmash, it looks like you're updating the parameters for a rule type! Please review the guidelines for making additive changes to rule type parameters and determine if your changes require an intermediate release. |
|
A documentation preview will be available soon. Request a new doc build by commenting
If your PR continues to fail for an unknown reason, the doc build pipeline may be broken. Elastic employees can check the pipeline status here. |
2c414f8 to
8b47bb4
Compare
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds Significant Events (SigEvents) management support to Streams Agent Builder by introducing a new skill and new built-in tools for searching, creating, and updating SigEvents, while centralizing shared SigEvent verdict/impact schemas in @kbn/streams-schema.
Changes:
- Added
significant-events-managementskill content and registered it as an allow-listed built-in skill. - Implemented new SigEvents tools:
event_search,event_create, andevent_verdict_update, including telemetry events for create/update. - Centralized verdict/impact option constants + schemas in
@kbn/streams-schemaand updated UI to consume shared verdict options.
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| x-pack/platform/plugins/shared/streams_app/public/components/sig_events/significant_events_discovery/components/sig_events_tab/index.tsx | Switches verdict filter options to shared schema constants. |
| x-pack/platform/plugins/shared/streams_app/public/components/sig_events/significant_events_discovery/components/sig_events_tab/filter_constants.ts | Aligns verdict typing with schema; adds impact color mapping. |
| x-pack/platform/plugins/shared/streams/server/routes/internal/sig_events/events/route.ts | Adjusts lifecycle route to handle missing discovery_slug. |
| x-pack/platform/plugins/shared/streams/server/lib/telemetry/ebt/types.ts | Adds telemetry prop types for event create and verdict update tools. |
| x-pack/platform/plugins/shared/streams/server/lib/telemetry/ebt/service.ts | Registers new telemetry event types. |
| x-pack/platform/plugins/shared/streams/server/lib/telemetry/ebt/schemas.ts | Adds telemetry schemas for event create and verdict update. |
| x-pack/platform/plugins/shared/streams/server/lib/telemetry/ebt/events.ts | Declares new telemetry event types mapping to schemas. |
| x-pack/platform/plugins/shared/streams/server/lib/telemetry/ebt/constants.ts | Defines string constants for new telemetry event types. |
| x-pack/platform/plugins/shared/streams/server/lib/telemetry/ebt/client.ts | Adds client methods to report new telemetry events. |
| x-pack/platform/plugins/shared/streams/server/agent_builder/tools/register_tools.ts | Registers new SigEvents tools and exports tool IDs. |
| x-pack/platform/plugins/shared/streams/server/agent_builder/tools/register_tools.test.ts | Extends tool registration test coverage for new tool IDs. |
| x-pack/platform/plugins/shared/streams/server/agent_builder/tools/event_verdict_update/tool.ts | Adds built-in tool to update an event’s verdict (with telemetry). |
| x-pack/platform/plugins/shared/streams/server/agent_builder/tools/event_verdict_update/tool.test.ts | Adds tests for the verdict update tool wrapper. |
| x-pack/platform/plugins/shared/streams/server/agent_builder/tools/event_verdict_update/handler.ts | Implements verdict update handler creating a new event version. |
| x-pack/platform/plugins/shared/streams/server/agent_builder/tools/event_verdict_update/handler.test.ts | Adds unit tests for verdict update handler logic. |
| x-pack/platform/plugins/shared/streams/server/agent_builder/tools/event_search/tool.ts | Adds built-in tool to search significant events. |
| x-pack/platform/plugins/shared/streams/server/agent_builder/tools/event_search/tool.test.ts | Adds tests for the event search tool wrapper. |
| x-pack/platform/plugins/shared/streams/server/agent_builder/tools/event_search/handler.ts | Implements search handler mapping tool params to client API. |
| x-pack/platform/plugins/shared/streams/server/agent_builder/tools/event_search/handler.test.ts | Adds unit tests for search handler param mapping. |
| x-pack/platform/plugins/shared/streams/server/agent_builder/tools/event_create/tool.ts | Adds built-in tool to create significant events (with telemetry + confirmation). |
| x-pack/platform/plugins/shared/streams/server/agent_builder/tools/event_create/tool.test.ts | Adds tests for the event create tool wrapper. |
| x-pack/platform/plugins/shared/streams/server/agent_builder/tools/event_create/handler.ts | Implements event creation handler producing a SigEvent document. |
| x-pack/platform/plugins/shared/streams/server/agent_builder/tools/event_create/handler.test.ts | Adds unit tests for event creation handler. |
| x-pack/platform/plugins/shared/streams/server/agent_builder/skills/sig_events_management/skill.md.text | Adds workflow guidance and examples for SigEvents management. |
| x-pack/platform/plugins/shared/streams/server/agent_builder/skills/sig_events_management/index.ts | Defines and exposes the new skill with its tool dependencies. |
| x-pack/platform/plugins/shared/streams/server/agent_builder/skills/sig_events_management/description.text | Adds the skill description text. |
| x-pack/platform/plugins/shared/streams/server/agent_builder/skills/register_skills.ts | Registers the new SigEvents management skill. |
| x-pack/platform/packages/shared/kbn-streams-schema/src/sig_events/index.ts | Re-exports verdict/impact schemas, types, and options for reuse. |
| x-pack/platform/packages/shared/kbn-streams-schema/src/sig_events/events/index.ts | Introduces shared verdict/impact schemas and relaxes optional SigEvent fields. |
| x-pack/platform/packages/shared/kbn-streams-schema/index.ts | Exposes new SigEvent schema exports at package root. |
| x-pack/platform/packages/shared/agent-builder/agent-builder-server/allow_lists.ts | Allow-lists the new built-in skill. |
| x-pack/platform/packages/shared/agent-builder/agent-builder-common/tools/constants.ts | Adds tool IDs for SigEvents search/create/verdict update. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
5e4fd7f to
817a07c
Compare
crespocarlos
left a comment
There was a problem hiding this comment.
I haven't tested it, just had a quick look at the code. Looks pretty cool.
| STREAMS_SEARCH_KNOWLEDGE_INDICATORS_TOOL_ID, | ||
| STREAMS_SEARCH_EVENTS_TOOL_ID, | ||
| STREAMS_CREATE_EVENT_TOOL_ID, | ||
| STREAMS_EVENT_VERDICT_UPDATE_TOOL_ID, |
There was a problem hiding this comment.
Verdict will be dropped soon https://github.com/elastic/streams-program/pull/1390. We'll only have events. Just flagging it. I'm not sure if we should already discount it here or as a follow-up. up to you.
There was a problem hiding this comment.
But events still have a verdict property, right? The tool only changes the verdict on an event, doesn't create a verdict document.
There was a problem hiding this comment.
The term verdict itself will be dropped too. Probably status makes more sense
There was a problem hiding this comment.
Renamed to status in e001d97.
Haven't touched the SigEvent schema, assuming it will be renamed as part of your change.
| - `criticality`: | ||
| - Number in range 0..100 indicating system criticality. | ||
| - Suggested scale: | ||
| - 0-30 low | ||
| - 31-60 medium | ||
| - 61-80 high | ||
| - 81-100 critical |
There was a problem hiding this comment.
just FYI: criticality will be renamed to severity soon.
| - `impact`: | ||
| - Always set to exactly one of: `critical`, `high`, `medium`, `low`. |
There was a problem hiding this comment.
Making LLMs assign impact makes me a bit nervous. This was something done for the prototype, but we need to review if we want this
There was a problem hiding this comment.
Let's see, if we decide to get rid of it, I'll follow up with the skill change.
| "criticality": 86, | ||
| "confidence": 0.84, | ||
| "impact": "high", | ||
| "recommended_action": "Scale checkout pods and increase upstream timeout budget temporarily.", |
There was a problem hiding this comment.
This diverges from what the workflow does. In fact, I feel like this field is a dead weight. It's deterministic in the workflow and I'm not sure it adds more value than recommendations
There was a problem hiding this comment.
got it, I'll make it optional on the schema remove it from the skill for now
| '@timestamp': now, | ||
| created_at: now, | ||
| event_id: eventId, | ||
| discovery_slug: `agent-event-${eventId.slice(0, 8)}`, |
There was a problem hiding this comment.
This will also diverge from what discovery workflows expect. We'll need to find better ways for this as a follow-up.
There was a problem hiding this comment.
Yeah, agree, this field bothers me as well. I'll update the tool and the skill once we settle on a better solution, for now I added it because we use discovery_slug as a grouping key.
| @@ -0,0 +1,167 @@ | |||
| You manage Significant Events (SigEvents) for Streams. | |||
There was a problem hiding this comment.
q: was this based on the current investigator or judge agent instructions?
it will be great to use this in the workflows
There was a problem hiding this comment.
Not really based on the agents, I just tried to tune it to the conversation flow with agent discovering and proactively suggesting sig events to the user. LLM generated the first draft and I tweaked it after experimenting with the chat.
| recommendations: string[]; | ||
| } | ||
|
|
||
| export async function createEventToolHandler({ |
There was a problem hiding this comment.
To confirm, we'll have events without a corresponding discovery and detections. correct? I wonder what will happen to the workflows once it sees these docs.
There was a problem hiding this comment.
Yes, correct. Would it make sense to add some ignore logic into the workflow for this kind of "manual" events?
| message: i18n.translate( | ||
| 'xpack.streams.agentBuilder.tools.eventCreate.confirmation.message', | ||
| { | ||
| defaultMessage: 'Create significant event "{title}"?', |
There was a problem hiding this comment.
Should we mention the stream in this message?
| verdict: EventVerdict; | ||
| }): Promise<{ event_id: string; updated: number; ignored: number; verdict: EventVerdict }> { | ||
| const { hits } = await eventClient.findById(eventId); | ||
| const latest = hits[hits.length - 1]; |
There was a problem hiding this comment.
Why not sort by timestamp?
There was a problem hiding this comment.
Not sure I get what you mean, it's sorted inside runFindByIdEsqlQuery with @timestamp ASC
|
One thing I just thought is that we need to ensure these changes are reflected in the workflow's agents. We also need these changes on the skill side, keeping up with updates on schemas and indices. This skill replaces a bunch of lines of instructions and will help standardize what agents need to do WRT sigevent discovery. |
crespocarlos
left a comment
There was a problem hiding this comment.
Tested the basic flow, everything works. Left some nits
| STREAMS_SEARCH_KNOWLEDGE_INDICATORS_TOOL_ID, | ||
| STREAMS_SEARCH_EVENTS_TOOL_ID, | ||
| STREAMS_CREATE_EVENT_TOOL_ID, | ||
| STREAMS_EVENT_VERDICT_UPDATE_TOOL_ID, |
There was a problem hiding this comment.
The term verdict itself will be dropped too. Probably status makes more sense
| import { v4 as uuidv4 } from 'uuid'; | ||
| import type { EventClient } from '../../../lib/sig_events/events'; | ||
|
|
||
| type EventVerdict = 'promoted' | 'acknowledged' | 'demoted'; |
| created_at: now, | ||
| event_id: eventId, | ||
| discovery_slug: `agent-event-${eventId.slice(0, 8)}`, | ||
| verdict: eventInput.verdict ?? 'promoted', |
There was a problem hiding this comment.
Should the default be promoted?
There was a problem hiding this comment.
I think so, if the user decides explicitly to create a sig event, I assume it's about something ongoing that they just discovered.
Though there might be cases when user wants to save a sig event as a draft and do additional investigation before promoting it, we might need to introduce this additional draft state. WDYT?
There was a problem hiding this comment.
We could leave it as-is and iterate. I'm keen to use it in the sigevent agents; eventually, these things will be refined.
|
@crespocarlos I pushed renaming to |
💛 Build succeeded, but was flaky
Failed CI Steps
Test Failures
Metrics [docs]Async chunks
History
|
This PR adds a new Significant Events management skill plus built-in tools for searching, creating, and updating event verdicts.
significant-events-managementskill with workflow guidance and examplesCleanShot.2026-05-27.at.13.59.28-converted.2.mp4