Skip to content

[Agent Builder] Lift streaming provider + remove 'new' cache key#267324

Merged
chrisbmar merged 22 commits into
elastic:mainfrom
chrisbmar:ab-14293-remove-new-cache-key
May 6, 2026
Merged

[Agent Builder] Lift streaming provider + remove 'new' cache key#267324
chrisbmar merged 22 commits into
elastic:mainfrom
chrisbmar:ab-14293-remove-new-cache-key

Conversation

@chrisbmar
Copy link
Copy Markdown
Contributor

@chrisbmar chrisbmar commented May 1, 2026

closes https://github.com/elastic/search-team/issues/14293
closes https://github.com/elastic/search-team/issues/14319

Problems

  • 'new' cache key. Frontend used 'new' as a placeholder cache key for unsaved conversations, renamed to the real id once the backend emitted one (~3s on average, which blocks other UI capabilities from being possible in that time). The cache rename + migration logic added ~300 lines of coordination and made per-conversation stream state hard to reason about.

  • Streaming provider scoped to the conversation tree. <SendMessageProvider> was mounted inside the conversation route - the sidebar and other surfaces couldn't see streaming state.

What this PR does

Removes 'new':

  • Backend PR (merged) accepts client-supplied UUIDs.
  • Frontend generates UUID v4 at submit, navigates to /conversations/<uuid> immediately, and writes the optimistic round under the real id from the outset.
  • Drops all 'new' references.

Lifts <SendMessageProvider> to the app level:

image
  • Mounted in mount.tsx (routed) and embeddable_conversations_provider.tsx (per-instance for embeddable, since each has its own QueryClient).
  • Sidebar reads via useSendMessageContext(); conversation tree uses the per-conversation scoped useSendMessage().
  • Lifted state: activeStream (which conversation is currently streaming) + byConversationId (per-conversation pending message, error, errorSteps).

Each conversation now owns its streaming lifecycle:

  • Mutations refactored from React Query lifecycle methods to a single-scope mutationFn with try / catch / finally.
  • Each invocation creates its own streamActions closure-bound to vars.conversationId.
  • Stream events target the right cache ID regardless of where the user has navigated.
  • Drops all ref patterns, and the messageControllerRef / isMutatingNewConversationRef / isRegeneratingRef refs that bridged state across lifecycle callbacks.

Per-conversation useConversation gate:

  • Disabled only for the conversation streaming or paused on HITL. Other conversations refetch normally (fixes this bug: useConversation gated on a global "is anything streaming" flag — switching to conversation B while A streamed prevented B from loading.)

Out of scope / follow-up

  • Concurrent streams: data plane is clean (per-conversation closures, lifted state); a follow-up PR will remove the global UI gates so streams can run independently 🚀 you can read more in the issue here.

Other cleanup

  • connectorSelection extracted from SendMessageProvider - it's a global setting, not streaming state.
  • retryOnMount: false on useConversation - prevents a refetch loop when a 404'd query has new observers mount (e.g., when a conversation is deleted and error state is shown).
  • LocationErrorClearer removed - errors now clear on explicit user actions: sidebar New Conversation button / conversation list / search modal selections etc. - this wasn't possible before because error state lived in the conversation tree and the sidebar knew nothing about it.
  • usePendingMessageState folded into byConversationId.

Architecture docs

  • New "Streams lifecycle (frontend)" section in CONTRIBUTOR_GUIDE.md.

Manual tests

  • Error handling
    (this uses a hardcoded error on the backend so retry doesn't work in the video but the FTR tests prove it, but you can see errors being cleared correctly when changing conversation)
error.handling.mov
  • HITL (correctly shows HITL prompt, disable ability to start the resume mutation whilst any other mutation is in flight, allow user to continue another conversation whilst 1 conversation is awaiting approval etc.)
HITL.mov
  • Sidebar (user can navigate between conversations even whilst one is streaming and they all continue to load. For now, submit is guarded by 'is any conversation streaming', soon this will be updated when concurrent streams are enabled)
sidebar.mov
  • Regenerate round response (whilst request is in flight, you can load other conversations, but not yet run other async requests until concurrent streams are enabled)
regenerate.round.response.mov
  • Rename / Delete conversation
rename.delete.mov
  • Can navigate to other conversations while a conversation is streaming (for now, all mutation abilities are guarded by 'is anything streaming' gate which means that if one conversation is streaming, then all other conversation inputs submit button are disabled - this will be fixed in the next PR)

@chrisbmar chrisbmar self-assigned this May 1, 2026
@chrisbmar chrisbmar added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting labels May 1, 2026
@chrisbmar
Copy link
Copy Markdown
Contributor Author

/ci

@chrisbmar
Copy link
Copy Markdown
Contributor Author

/ci

@chrisbmar
Copy link
Copy Markdown
Contributor Author

/ci

@chrisbmar
Copy link
Copy Markdown
Contributor Author

/ci

@chrisbmar
Copy link
Copy Markdown
Contributor Author

/ci

Comment on lines +208 to +212
// Re-exported so consumers can keep importing `useSendMessage` from this file. The actual
// implementation lives in `./use_send_message` to avoid a circular import with
// `use_conversation.ts` (the per-conversation scoped hook reads from `useConversation()`,
// while `useConversation()` reads from `useSendMessageContext` here).
export { useSendMessage } from './use_send_message';
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Self review: this will be fixed/updated in a future PR. It will caused about 7 or so file changes in this diff with the renaming so I've decided to just re-export for now.

agentReasoning: resumeAgentReasoning,
} = useResumeRoundMutation({
connectorId: connectorSelection.selectedConnector,
const [activeStream, setActiveStream] = useState<ActiveStream | undefined>(undefined);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

self-review: this is the shape that we want an active stream to be in for now. In a future PR, we will update this to be something like: Record<conversationId, ActiveStream | undefined> which is main change from the current code that will enable concurrent streams.

Today, we block any conversation from starting a stream if ANYTHING is streaming from any other conversation. With this change, each conversation will be able to check if THAT conversation is currently streaming - allowing each conversation to own it's own streaming lifecycle and not have to be concerned with any other conversation's streams.

@chrisbmar chrisbmar marked this pull request as ready for review May 6, 2026 08:51
@chrisbmar chrisbmar requested review from a team as code owners May 6, 2026 08:51
@kibanamachine
Copy link
Copy Markdown
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #41 / ESQL execution logic API @ess @serverless ES|QL rule type, alert suppression should suppress an alert during real rule executions

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
agentBuilder 1507 1508 +1

Async chunks

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

id before after diff
agentBuilder 1.1MB 1.1MB +1.3KB

History

cc @chrisbmar

@chrisbmar chrisbmar merged commit 90f832a into elastic:main May 6, 2026
76 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes v9.5.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants