[Sig Events] Per-step connector overrides for onboarding and insights discovery#261588
Conversation
.../components/sig_events/significant_events_discovery/components/streams_view/streams_view.tsx
Outdated
Show resolved
Hide resolved
1271b19 to
41ffb00
Compare
...ig_events/significant_events_discovery/components/streams_view/onboarding_config_popover.tsx
Outdated
Show resolved
Hide resolved
41ffb00 to
b8dbfae
Compare
b8dbfae to
d41dadb
Compare
|
@coderabbitai full review |
|
/ci |
✅ Actions performedFull review triggered. |
1 similar comment
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughExtended task definitions (significant_events_queries_generation, features_identification, onboarding) and the onboarding route to accept optional Possibly related PRs
Suggested labels
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@x-pack/platform/plugins/shared/streams_app/public/components/sig_events/significant_events_discovery/components/streams_view/streams_view.tsx`:
- Around line 92-110: The effect currently backfills onboardingConfig.connectors
with resolved defaults (via setOnboardingConfig in the useEffect), which removes
the ability to represent "no override"; remove this automatic write so
connectors remains empty unless the user explicitly chooses one. Specifically,
stop updating onboardingConfig.connectors inside the useEffect that watches
featuresConnectors.resolvedConnector and queriesConnectors.resolvedConnector (or
change it to update a separate transient defaults object rather than calling
setOnboardingConfig), and ensure any request/serialization logic reads resolved
defaults at request time when onboardingConfig.connectors.features/queries is
undefined rather than writing those defaults into onboardingConfig; keep the
initial connectors = {} and only call setOnboardingConfig to set connectors when
a user action selects an override.
In
`@x-pack/platform/plugins/shared/streams/server/lib/tasks/task_definitions/onboarding.ts`:
- Around line 118-142: The current freshness check around areFeaturesUpToDate
allows reusing a recent features-identification task even when the current run
passed a different connectors.features override; update the logic so the reuse
is disabled whenever a connector override is present or make areFeaturesUpToDate
consider connector identity. Concretely, modify the branch around
areFeaturesUpToDate(...) to either (a) call areFeaturesUpToDate with an
additional connectorId argument (connectors?.features) and have that function
verify the stored task used the same connector, or (b) add an explicit guard
that if connectors?.features is set then skip reuse and always call
scheduleFeaturesIdentificationTask(...) and waitForSubtask(...); reference
functions: areFeaturesUpToDate, scheduleFeaturesIdentificationTask,
waitForSubtask and symbol connectors.features and featuresTaskId.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 4ef04eae-80f4-40b3-9342-2781634367e8
📒 Files selected for processing (9)
x-pack/platform/plugins/shared/streams/server/lib/sig_events/tasks/significant_events_queries_generation.tsx-pack/platform/plugins/shared/streams/server/lib/tasks/task_definitions/features_identification/index.tsx-pack/platform/plugins/shared/streams/server/lib/tasks/task_definitions/onboarding.tsx-pack/platform/plugins/shared/streams/server/routes/internal/streams/onboarding/route.tsx-pack/platform/plugins/shared/streams_app/public/components/sig_events/significant_events_discovery/components/streams_view/onboarding_config_popover.tsxx-pack/platform/plugins/shared/streams_app/public/components/sig_events/significant_events_discovery/components/streams_view/streams_view.tsxx-pack/platform/plugins/shared/streams_app/public/components/sig_events/significant_events_discovery/components/streams_view/translations.tsx-pack/platform/plugins/shared/streams_app/public/hooks/sig_events/use_inference_feature_connectors.tsx-pack/platform/plugins/shared/streams_app/public/hooks/use_onboarding_api.ts
.../components/sig_events/significant_events_discovery/components/streams_view/streams_view.tsx
Outdated
Show resolved
Hide resolved
x-pack/platform/plugins/shared/streams/server/lib/tasks/task_definitions/onboarding.ts
Show resolved
Hide resolved
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughThis change introduces optional per-step connector override capabilities to the streams onboarding workflow. On the server side, three task types—SignificantEventsQueriesGeneration, FeaturesIdentification, and Onboarding—now accept optional Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@x-pack/platform/plugins/shared/streams/server/lib/tasks/task_definitions/onboarding.ts`:
- Around line 60-80: areFeaturesUpToDate currently only checks task status and
recency and must also verify the features connector matches the currently
selected connector; change the areFeaturesUpToDate signature to accept the
expected features connector (or expected connectors object), then after fetching
featuresTask read the persisted connector from
featuresTask.params?.connectors?.features (or params.connectors) and return
false if it differs before performing the age check against
FEATURES_IDENTIFICATION_RECENCY_MS; apply the same connector-matching check to
the analogous function/usage later in the file (the block around the other
features recency check).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: e54f79dc-26dd-4c80-b85d-195d0431140c
📒 Files selected for processing (9)
x-pack/platform/plugins/shared/streams/server/lib/sig_events/tasks/significant_events_queries_generation.tsx-pack/platform/plugins/shared/streams/server/lib/tasks/task_definitions/features_identification/index.tsx-pack/platform/plugins/shared/streams/server/lib/tasks/task_definitions/onboarding.tsx-pack/platform/plugins/shared/streams/server/routes/internal/streams/onboarding/route.tsx-pack/platform/plugins/shared/streams_app/public/components/sig_events/significant_events_discovery/components/streams_view/onboarding_config_popover.tsxx-pack/platform/plugins/shared/streams_app/public/components/sig_events/significant_events_discovery/components/streams_view/streams_view.tsxx-pack/platform/plugins/shared/streams_app/public/components/sig_events/significant_events_discovery/components/streams_view/translations.tsx-pack/platform/plugins/shared/streams_app/public/hooks/sig_events/use_inference_feature_connectors.tsx-pack/platform/plugins/shared/streams_app/public/hooks/use_onboarding_api.ts
x-pack/platform/plugins/shared/streams/server/lib/tasks/task_definitions/onboarding.ts
Show resolved
Hide resolved
|
/ci |
|
@elasticmachine merge upstream |
|
/ci |
|
@elasticmachine merge upstream |
|
/ci |
|
@cesco-f I am still reviewing the PR, but thought of sharing this earlier. The UX for the dropdown looks different from what @boriskirov has proposed in his wireframes in the linked Figma. Is this intentional ? |
Hey @achyutjhunjhunwala! Yes, we decided to split the functionality from the final UX experience, @boriskirov will take care of that in a separate PR. |
Yes, I will open another one with the UX improvements, once this gets merged, as a follow up @achyutjhunjhunwala |
.../components/sig_events/significant_events_discovery/components/streams_view/streams_view.tsx
Show resolved
Hide resolved
…ig_events/significant_events_discovery/components/streams_view/streams_view.tsx Co-authored-by: Achyut Jhunjhunwala <achyut.jhunjhunwala@gmail.com>
...tform/plugins/shared/streams_app/public/hooks/sig_events/use_inference_feature_connectors.ts
Show resolved
Hide resolved
...tform/plugins/shared/streams_app/public/hooks/sig_events/use_inference_feature_connectors.ts
Outdated
Show resolved
Hide resolved
|
@cesco-f Thanks for these changes, i have added couple of short comments. 1 additional UX issue i found was, lets say user open the popover and deselects both the options (KI and Rules). Closes the pop up. Now even though they have selected streams, onboard streams is not enabled and the user has no way of identifying why. One solution to this could be, cc: @boriskirov
|
Thanks for the great review @achyutjhunjhunwala, I've addressed your comments! I'll leave the UX issue to @boriskirov. |
...ig_events/significant_events_discovery/components/streams_view/onboarding_config_popover.tsx
Outdated
Show resolved
Hide resolved
flash1293
left a comment
There was a problem hiding this comment.
All of the files in here are actually sigevents code, not sure why they trigger code review for the onboarding team
There was a problem hiding this comment.
@CoenWarmer should this file be owned by the onboarding team?
I think it's because of the moon file. |
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Module Count
Async chunks
History
|


Summary
resolveConnectorForFeature()when no override is provided.Screen.Recording.2026-04-08.at.14.26.20.mov