[Dashboards] POC Migrate openLazyFlyout to the new flyout service#19
Conversation
| flyoutRef.close(); | ||
| }; | ||
|
|
||
| // this |
There was a problem hiding this comment.
The default session value here is set to "start".
Because openLazyFlyout relies on this service, any flyout opened through it will also default to starting a new session unless a different value is explicitly passed.
| className: 'kbnPresentationLazyFlyout', | ||
| 'aria-labelledby': ariaLabelledBy, | ||
| onClose, | ||
| title: 'title placeholder', |
There was a problem hiding this comment.
The new flyout service requires passing a title prop.
For now I added a placeholder value so the flyout renders correctly and avoids runtime errors.
According to the documentation, parent (main) flyouts should use the standard euiFlyoutHeader structure, which we already render in our layouts.
The title generated by the new flyout system is intended only for child flyouts and is rendered inside the euiFlyoutMenu markup.
In a future implementation we should ensure that this auto-generated menu title is not shown for parent flyouts, as it does not match the expected design.
| flyoutProps: { | ||
| 'data-test-subj': 'dashboardPanelSelectionFlyout', | ||
| triggerId: 'dashboardAddTopNavButton', | ||
| title: i18n.translate('dashboard.solutionToolbar.addPanelFlyout.headingText', { |
## Summary Closes elastic#254714 - Adds Claude reviewer using [GH Agentic Workflows](https://github.github.com/gh-aw/introduction/how-they-work/) - Adds workflow handling of @/claude mentions in comments - Adds a `code-reviewer` subagent for reuse in workflows. - Proxies through LiteLLM for Opus 4.7 model ### Testing https://github.com/elastic/kibana/actions/workflows/reviewer-claude.lock.yml This branch is directly in `elastic/kibana` rather than my fork and has been running the workflow for reviews during development with `pull_request` as the event, rather than the final state of `pull_request_target` events. You can see comments Claude has posted and responses to being mentioned. "No issues found" comments have been moved to a `no-op` instead, the status checks reflect when the agent runs anyways. Will need to follow up with a proper `pull_request_target` run after merge. There are some security settings automatically applied during compilation that we cannot verify yet. Since we're gated behind the label, there shouldn't be any issues immediately post merge with the workflow running unnecessarily. ### Note for Reviewers - It isn't necessary to review the two `lock` files. They are compiled and generated files, but must be committed for the workflow to run. - Open to different model versions, don't have a strong preference towards Opus 4.7 ## Cache and cost analysis Tested ten Opus 4.7 (`llm-gateway/claude-opus-4-7`) reviewer runs with `ENABLE_PROMPT_CACHING_1H` enabled. The cache is active, but the data includes dirty cases: prompt changes, long gaps outside the useful cache window, regular PR review runs, and comment-reply runs. | Run | Event | Head | Cost | Turns | Cache read | Cache write | Output | Effective tokens | | --- | --- | --- | ---: | ---: | ---: | ---: | ---: | ---: | | [#15](https://github.com/elastic/kibana/actions/runs/25011054153) | `pull_request` | `2076d4f` | $1.04 | 14 | 809,672 | 80,897 | 5,169 | 182,559 | | [#16](https://github.com/elastic/kibana/actions/runs/25011557405) | `pull_request` | `382357b` | $1.32 | 21 | 1,495,919 | 70,406 | 5,226 | 240,928 | | [#17](https://github.com/elastic/kibana/actions/runs/25012064350) | `pull_request` | `e93fe0b` | $1.33 | 23 | 1,378,836 | 50,413 | 13,022 | 240,413 | | [#18](https://github.com/elastic/kibana/actions/runs/25013588252) | `pull_request` | `04ded1d` | $1.20 | 24 | 1,453,092 | 39,648 | 8,876 | 220,490 | | [#19](https://github.com/elastic/kibana/actions/runs/25030187214) | `pull_request` | `88a2c7e` | $1.41 | 16 | 1,165,558 | 99,949 | 8,171 | 249,210 | | [#21](https://github.com/elastic/kibana/actions/runs/25030484848) | `pr_review_comment` | `88a2c7e` | $0.45 | 9 | 512,383 | 19,390 | 3,035 | 82,782 | | [elastic#23](https://github.com/elastic/kibana/actions/runs/25031202052) | `pull_request` | `66b198f` | $1.39 | 18 | 1,230,107 | 78,104 | 11,420 | 246,818 | | [elastic#26](https://github.com/elastic/kibana/actions/runs/25033088024) | `pull_request` | `d59d8c0` | $1.50 | 22 | 1,428,302 | 90,868 | 8,885 | 269,265 | | [elastic#27](https://github.com/elastic/kibana/actions/runs/25034181833) | `pull_request` | `d1052ef` | $1.73 | 24 | 2,016,691 | 81,113 | 8,737 | 317,759 | | [elastic#28](https://github.com/elastic/kibana/actions/runs/25034226594) | `pr_review_comment` | `d1052ef` | $1.18 | 23 | 1,453,250 | 36,528 | 7,788 | 213,038 | | Run | First request cache read | First request cache write | Notes | | --- | ---: | ---: | --- | | 15 | 0 | 51,751 | Cold 1-hour cache write. | | 16 | 35,886 | 15,905 | Stable prefix reused. | | 17 | 35,886 | 15,907 | Same prefix hit pattern. | | 18 | 35,886 | 15,911 | Same prefix hit pattern. | | 19 | 0 | 53,676 | Cold again after long gap + prompt/head changes. | | 21 | 45,471 | 8,375 | Comment-reply flow reused 19 prefix and wrote a smaller suffix. | | 23 | 37,245 | 16,587 | New commit/prompt still reused part of the 1-hour prefix. | | 26 | 0 | 53,761 | Cold again after another gap + prompt/head changes. | | 27 | 37,245 | 16,511 | Warm PR run on a new head/prompt hash. | | 28 | 45,465 | 8,461 | Comment-reply flow reused 27 prefix, but still ran long. | Summary: - All observed cache creation was 1-hour (`ephemeral_1h_input_tokens`); no 5-minute writes were observed. - Average observed cost is about $1.26/run across all ten runs, or about $1.37 for normal `pull_request` runs excluding comment replies. - Comment replies are not inherently cheap: run 21 was $0.45 because it was narrow and 9 turns, while run 28 was $1.18 because it took 23 turns. - Cold starts appear after long gaps or prompt churn (runs 15, 19, 26); warm runs still reuse partial prefixes even when the commit/prompt hash changes. - Next tuning is comparing 5-minute vs 1-hour caching, testing cheaper models against review quality, and auditing trace data for unnecessary output or artifact reads. --------- Co-authored-by: Tyler Smalley <tyler.smalley@elastic.co> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Resolves elastic#241020 I have branched off of from this PR .
Flyout System Roadmap
The plan is to move all Kibana flyouts to the New System, meaning:
session prop.Terms for clarity:
New System – EUI Flyout with a session prop.
New Service –
core.overlays.openSystemFlyout, which provides access to the EUI Flyout with the session prop in non-React contexts. The currentcore.overlays.openFlyoutmethod will eventually be deprecated.The summary of my research is in this doc.
Session prop
In this PR I set
session: "start"insideopenLazyFlyout.This ensures that every flyout opened through this helper starts a new session and is treated as the main flyout in the workflow.
SystemFlyoutServiceusessession: "start"as its default value, whereas the documentation mentionssession: "inherit"is the default. This happens in the EUI component because we don't want to change the behavior of existing flyouts that are rendered using the EUI React component directly. For flyouts that are rendered withcore.overlays.openSystemFlyout, it is safe to assume the developer wants a system flyout, so we set session to "start" for those calls. More information in the summary in this doc.Types
I also updated openLazyFlyout to use
OverlaySystemFlyoutOpenOptions, which provides the correct types for the session property. Previously it was usingOverlayFlyoutOpenOptions, where the session type was limited to "never", so the typing didn’t match the new flyout system.SystemFlyoutService
I updated openLazyFlyout to use the New Service – core.overlays.openSystemFlyout. This service gives access to the EUI Flyout with the session prop, even in non-React contexts
How titles are rendered in the new flyout system
I also investigated how titles are rendered in the new flyout system.
Even though we pass a title via flyoutMenuProps.title, the title inside our flyouts is often nested deeper and doesn’t have its own dedicated header element.
This aligns with a known issue in the new flyout system:
main flyouts currently render the title in the flyout menu bar, coming from flyoutMenuProps.title, even though the design requires using the standard EuiFlyoutHeader title.
This is the same bug reported in the EUI repository — title handling is still being refined, because the system currently places the title in the menu bar instead of the header.
I’m mentioning it here so we don’t spend additional time researching this during the POC.
Responsiveness
I also tested how the flyouts behave on different mobile views and in various dashboard contexts.
Flyouts for POC
In the POC, I decided to test the full path: Add → Add Panel Flyout → ES|QL Panel Flyout → Create Variable Control Flyout, in order to reproduce the session flow and see how the flyouts behave in sequence.
The video of their behaviour is shown below:
Screen.Recording.2025-11-18.at.16.32.22.mov