Skip to content

[SigEvents] Fix model order issue [BLOCKED]#264050

Draft
achyutjhunjhunwala wants to merge 13 commits intoelastic:mainfrom
achyutjhunjhunwala:fix-individual-task-running-model-selector-issue
Draft

[SigEvents] Fix model order issue [BLOCKED]#264050
achyutjhunjhunwala wants to merge 13 commits intoelastic:mainfrom
achyutjhunjhunwala:fix-individual-task-running-model-selector-issue

Conversation

@achyutjhunjhunwala
Copy link
Copy Markdown
Contributor

@achyutjhunjhunwala achyutjhunjhunwala commented Apr 17, 2026

DO NOT MERGE THE PR

Summary

closes -

The sig-events per-step model dropdowns (KI Extraction / KI Queries / Discovery) were sourcing options from the wrong inference feature and pre-selecting the global GenAI default instead of the step's first recommended model. This PR scopes each split button to its own feature and — after review — stops hand-rolling any ordering or selection client-side, trusting the backend as the single source of truth.

How we got here

When (UTC) PR Effect on this flow
Apr 8 16:39 #261588 Per-step connector overrides landed, but the dropdown option list still came from the parent useGenAIConnectors.
Apr 8 21:12 #262071 useLoadConnectors started prepending GEN_AI_SETTINGS_DEFAULT_AI_CONNECTOR, pushing feature-recommended endpoints down.
Apr 9 10:50 #262018 Rewired useGenAIConnectors to featureId: 'streams' — a parent with recommendedEndpoints: [], so sig-events recs never surfaced.
Apr 17 08:19 #263843 Moved model resolution to the backend, added isRecommended flag, and made the HTTP route prepend the global default.

Fix

Per-step feature scoping (core fix)

  • Each split button reads options from its own per-step feature id (STREAMS_SIG_EVENTS_KI_EXTRACTION_..., ..._KI_QUERY_GENERATION_..., ..._DISCOVERY_...) via useInferenceFeatureConnectors(featureId).
  • streams_view.tsx wires the three per-step lists into GenerateSplitButton / InsightsSplitButton; the old shared allConnectors prop is retired.
  • Shared toInferenceConnector adapter extracted into public/hooks/to_inference_connector.ts so the GenAI parent hook and the per-feature sig-events hook agree on one mapping.

Trust the backend for ordering (no more client-side magic)

  • use_inference_feature_connectors.ts no longer sorts by isRecommended and no longer hand-picks a "first recommended" value. It renders the backend response verbatim and sets resolvedConnectorId = aiConnectors[0]?.id.
  • The HTTP route already orders admin SO override → global default (if set) → recommendedEndpoints → rest of catalog, so our dropdown correctly shows the admin's global default on top when set, and the step's recommendeds on top when not.
  • DEFAULT_ONLY=true handling also falls out for free — the HTTP route collapses the response to just the default and we render it as-is.

Scenarios

Running example for the columns below: KI Extraction. Registry recommendeds = [GPT-OSS-120B, GPT-5.2, Sonnet 4.6]. Other connectors referenced: Claude Haiku, Gemini 2.5 Pro, GPT-4.1.

# Scenario Dropdown today Task runner today Matches new requirement? Expected behaviour
1 Global default set to a non-recommended (e.g. Claude Haiku) [GPT-OSS-120B, GPT-5.2, Sonnet 4.6, Claude Haiku, …rest], badge on GPT-OSS-120B picks GPT-OSS-120B ❌ both wrong Dropdown: [Claude Haiku, GPT-OSS-120B, GPT-5.2, Sonnet 4.6, …rest], badge on Claude Haiku. Runner: picks Claude Haiku.
2 Global default matches a recommended (e.g. Sonnet 4.6, registry rec #3) [GPT-OSS-120B, GPT-5.2, Sonnet 4.6, …rest], badge on GPT-OSS-120B picks GPT-OSS-120B ❌ both wrong Dropdown: [Sonnet 4.6, GPT-OSS-120B, GPT-5.2, …rest] (Sonnet 4.6 hoisted, deduped), badge on Sonnet 4.6. Runner: picks Sonnet 4.6.
3 Global default unset [GPT-OSS-120B, GPT-5.2, Sonnet 4.6, …rest], badge on GPT-OSS-120B picks GPT-OSS-120B ✅ both correct Unchanged from today.
4 SO override for feature, no global default (e.g. Model Settings → [Gemini 2.5 Pro, GPT-4.1]) [Gemini 2.5 Pro, GPT-4.1], badge on Gemini 2.5 Pro picks Gemini 2.5 Pro ✅ both correct (SO wins by design) Unchanged from today.
5 DEFAULT_ONLY=true, default set (e.g. Claude Haiku) [Claude Haiku] only picks GPT-OSS-120B (recommended #1; ignores "Disallow") ❌ task runner wrong Dropdown: unchanged. Runner: picks Claude Haiku; throws when default is unset.
6 Global default set AND SO override (e.g. default=Claude Haiku, SO=[Gemini 2.5 Pro, GPT-4.1]) [Gemini 2.5 Pro, GPT-4.1], badge on Gemini 2.5 Pro picks Gemini 2.5 Pro ✅ both correct (SO is a more specific directive than the global default) Unchanged from today.

Dropdown rows 1, 2, 5 are made correct by this PR (client sort removed, backend ordering trusted). Task-runner rows 1, 2, 5 will be made correct by the inference-plugin PR once it merges.

How to test

  1. yarn start (restart if Kibana was already running on this branch).
  2. Open Streams → Significant Events Discovery.
  3. No global default, no SO override: the Generate and Discover insights split buttons show their feature's three recommendeds on top (in registry order), first one badged Default, rest of the catalog below.
  4. Global default set to a non-recommended (e.g. Claude Haiku): the admin default appears at position 1 with the Default badge, followed by the feature's three recommendeds, then the rest of the catalog. (Background task runners still pick recommended links to styles and js #1 until the inference-plugin PR lands — expected.)
  5. Global default set to a feature-recommended (e.g. Sonnet 4.6 for KI Extraction): the admin default is hoisted to position 1 and deduped against the recommendeds; badge on the admin default.
  6. "Disallow other models" ON in the default-model settings: the dropdown collapses to just the global default.
  7. Inference Settings SO override for a sig-events feature: dropdown collapses to the SO endpoints; the global default is not shown (SO wins).

Screenshot

Was BLOCKED by this PR


The PR was developed with Claude Code — claude-opus-4-7

@achyutjhunjhunwala achyutjhunjhunwala self-assigned this Apr 17, 2026
@achyutjhunjhunwala achyutjhunjhunwala added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting labels Apr 17, 2026
@achyutjhunjhunwala achyutjhunjhunwala added backport:version Backport to applied version labels v9.4.0 Feature:SigEvents Significant events feature, related to streams and rules/alerts (RnA) Team:SigEvents Project team working on Significant Events v9.5.0 and removed backport:skip This PR does not require backporting labels Apr 17, 2026
@achyutjhunjhunwala achyutjhunjhunwala changed the title [SigEvents] Fix model order issue [WIP] [SigEvents] Fix model order issue Apr 17, 2026
@achyutjhunjhunwala achyutjhunjhunwala marked this pull request as ready for review April 17, 2026 14:17
@achyutjhunjhunwala achyutjhunjhunwala requested review from a team as code owners April 17, 2026 14:17
@macroscopeapp
Copy link
Copy Markdown
Contributor

macroscopeapp Bot commented Apr 17, 2026

Approvability

Verdict: Needs human review

The PR is explicitly marked as '[BLOCKED]' in the title, indicating it's not ready for merge. Additionally, it changes connector resolution behavior affecting model selection across multiple features, and the author does not own any of the changed files which are all owned by @elastic/obs-onboarding-team or @elastic/obs-sig-events-team.

You can customize Macroscope's approvability policy. Learn more.

Copy link
Copy Markdown
Contributor

@ruflin ruflin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did a manual test and works as expected.

@achyutjhunjhunwala achyutjhunjhunwala changed the title [SigEvents] Fix model order issue [SigEvents] Fix model order issue [BLOCKED] Apr 17, 2026
@achyutjhunjhunwala
Copy link
Copy Markdown
Contributor Author

/ci-ralph can you get me a green build. In case you see build failures are not because of my changes, please merge upstream to retrigger the build

@achyutjhunjhunwala
Copy link
Copy Markdown
Contributor Author

/ralph check the test failures of build 430853 and fix them

Build URL: https://buildkite.com/elastic/kibana-pull-request/builds/430853

Use ci/bk-build-info.sh 430853 to fetch detailed build failure information.

Increase lens bundle size limit from 86000 to 86500 to fix build
failure caused by the merge with main (current size: 86083).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@achyutjhunjhunwala achyutjhunjhunwala requested a review from a team as a code owner April 21, 2026 08:12
@achyutjhunjhunwala
Copy link
Copy Markdown
Contributor Author

Build Fix: Lens Bundle Size Limit

Build #430853 failed because the lens plugin page load bundle size (86083 bytes) exceeded the configured limit (86000 bytes). This overage was introduced by the merge with `main`, not by the PR's own changes.

Fix: Updated `packages/kbn-optimizer/limits.yml` to increase the lens bundle size limit from 86000 → 86500.

Commit: f4b466f

🤖 Generated with Claude Code

…ssue

Resolves conflicts from main's PR elastic#263822 (KiGenerationProvider refactor):

- packages/kbn-optimizer/limits.yml: took main's lens: 90000 (higher ceiling).
- use_connector_config.ts: accepted main's deletion (state moved into
  KiGenerationProvider).
- use_inference_feature_connectors.ts: kept our hook shape (connectors +
  resolvedConnectorId) with resolvedConnectorId = aiConnectors[0]?.id so
  the admin's global default wins over feature recommendedEndpoints when
  set, matching the alignment with the inference team.
- streams_view.tsx: took main's useKiGeneration() destructure, dropped the
  useAIFeatures/allConnectors coupling, and derived connectorError /
  isConnectorCatalogUnavailable from the three per-step hooks.
- knowledge_indicators_table.tsx: same per-step derivation; passes
  featuresConnectors/queriesConnectors (not allConnectors) into
  GenerateSplitButton.
- ki_generation_context.tsx: extended ConnectorState to expose connectors
  and error from each per-step hook.

Type check passes on both streams_app and streams projects.
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Apr 22, 2026

🤖 Jobs for this PR can be triggered through checkboxes. 🚧

ℹ️ To trigger the CI, please tick the checkbox below 👇

  • Click to trigger kibana-pull-request for this PR!
  • Click to trigger kibana-deploy-project-from-pr for this PR!
  • Click to trigger kibana-deploy-cloud-from-pr for this PR!
  • Click to trigger kibana-entity-store-performance-from-pr for this PR!
  • Click to trigger kibana-storybooks-from-pr for this PR!

@achyutjhunjhunwala
Copy link
Copy Markdown
Contributor Author

Moving this PR back to Draft as there is some work which needs to be done

@elasticmachine
Copy link
Copy Markdown
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
streamsApp 1820 1821 +1

Async chunks

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

id before after diff
streamsApp 2.0MB 2.0MB -11.0B

History

cc @achyutjhunjhunwala

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:version Backport to applied version labels Feature:SigEvents Significant events feature, related to streams and rules/alerts (RnA) release_note:skip Skip the PR/issue when compiling release notes Team:SigEvents Project team working on Significant Events v9.4.0 v9.5.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants