feat(Lightspeed): Add stop button to interrupt a streaming conversation#2587
Conversation
karthikjeeyar
requested review from
a team,
rohitkrai03 and
yangcao77
as code owners
Changed Packages
|
Review Summary by QodoAdd stop button to interrupt streaming conversations
WalkthroughsDescription• Add interrupt endpoint to stop streaming conversations • Implement stop button UI with request tracking • Add auto-refetch for conversations with pending topic summaries • Prevent sending empty messages on Enter key press Diagramflowchart LR
UI["Stop Button in UI"]
Hook["useStopConversation Hook"]
Client["LightspeedApiClient.stopMessage"]
Backend["Backend /v1/query/interrupt"]
Core["Lightspeed-Core Server"]
UI -- "triggers" --> Hook
Hook -- "calls" --> Client
Client -- "POST request" --> Backend
Backend -- "forwards" --> Core
Core -- "interrupts stream" --> Backend
Backend -- "returns success" --> Client
File Changes1. workspaces/lightspeed/plugins/lightspeed-backend/__fixtures__/lcsHandlers.ts
|
Code Review by Qodo
1.
|
karthikjeeyar
changed the title
karthikjeeyar
changed the title
![rhdh-qodo-merge[bot]](https://avatars.githubusercontent.com/in/1961108?s=60&v=4){kind=small}
|
The topic summary LGTM! Two cents:
|
karthikjeeyar
force-pushed
the
add-stop-button
branch
from
baef089 to
2b7da0d
Compare
|
@aprilma419 I am now retaining the last used query if the conversation is interrupted. For the partial response, we will have a followup PR in LCORE to support it. |
Jdubrick
left a comment
Jdubrick
left a comment
There was a problem hiding this comment.
small nit, generally lgtm though
There was a problem hiding this comment.
Screen.Recording.2026-03-26.at.2.36.51.PM.mov
Works as expected, But One small observation —when a conversation gets interrupted, the side panel shows a loading icon. It feels a bit odd; I would like more clarity or feedback there.
Even after you continue the conversation in the same chat, it remains the same in the side panel
|
To @HusneShabbir 's point, instead of the loader icon, if feasible, we can show a skeleton loading state. |
yangcao77
left a comment
yangcao77
left a comment
There was a problem hiding this comment.
generally lgtm from backend perspective. I will hold off the lgtm label and wait for frontend side approval as well.
I assume you are not running LCORE from my Open LCORE PR (that adds the async topic_summary), thats why you are seeing the loading spinner always. Refer to the "How to test" section in this PR description and checkout to my open PR and run the LCORE to see the topic summary eventually. |
karthikjeeyar
force-pushed
the
add-stop-button
branch
from
2b7da0d to
fdebc96
Compare
@aprilma419 I too thought about this while implementing, but chatbot uses PF's Menu item from react-core under the hood. Unfortunately, it doesn't support skeletons for loading state, hence I have added loading icon. |
karthikjeeyar
force-pushed
the
add-stop-button
branch
from
fdebc96 to
afccc2a
Compare
| } | ||
| }); | ||
|
|
||
| router.post('/v1/query/interrupt', async (request, response) => { |
There was a problem hiding this comment.
Why do we need a separate API for stopping the streaming response? Why not use an AbortController?
There was a problem hiding this comment.
Using AbortController will be just a client side cancellation, and it is not a true cancellation on LLM request that was initiated by the user.
@Jdubrick has added a new interrupt API /interrupt on the LCORE side to immediately cancel the in-flight LLM request, and this is to properly stop the model inference and the token loop. As soon as we interrupt, the streaming response will now contain a new event_type called interrupted that signals the client to break out the loop.
There was a problem hiding this comment.
Yeah, so the client is not using AbortController right now and I think the lightspeed backend can detect abort controller and cancel the request properly without the need for frontend to call interrupt explicitly.
There was a problem hiding this comment.
For interrupting an existing call, we need a request_id. This request_id is only sent inside the stream ( start event). The client parses it and sends POST /v1/query/interrupt with that id; the backend forwards to LCORE.
We also keep the stream open so we can receive event === 'interrupted' for a clean stop and, for handling temp chats, apply conversation_id, calling onComplete handlers, invalidation react-query caches etc. Aborting tears down the stream, so that interrupted event often never arrives.
The backend can see when the client connection closes, but it does not automatically have request_id on that event, the id is in the response body stream. Detecting this abort on the backend side would require extra work like server-side stream parsing to find this request_id and call the interrupt call. In my honest opinion having the Client calling it with request_id simplifies things and lets the existing stream to come to an end gracefully.
Here is the current flow:
sequenceDiagram
participant C as Lightspeed client
participant B as Backstage backend
participant L as LCORE
C->>B: POST /v1/query (stream)
B->>L: POST /v1/streaming_query
L-->>B: stream chunks
B-->>C: pipe stream
Note over C: Client parses stream
L-->>C: event === start + request_id
Note over C: User clicks on stop button — client has request_id
C->>B: POST /v1/query/interrupt with request_id
B->>L: POST /streaming_query/interrupt with request_id
L-->>L: Cancel that run
L-->>C: stream includes event interrupted
Note Over C: client handles temp-chat migration, react-query invalidations etc
There was a problem hiding this comment.
+1, having the endpoint on the LCORE side and we hit it with the Lightspeed plugin lets us keep the conversation handling, persistence, and responses (such as what April suggested above, half-completed etc with a special response) consistent. LCORE also is the direct communicator with Llama Stack, so we can ensure that the request is properly interrupted on the LLM side.
As for how the plugin itself handles it, I +1 Karthik's explanation
karthikjeeyar
force-pushed
the
add-stop-button
branch
from
afccc2a to
e056966
Compare
|
6 participants
Hey, I just made a Pull Request!
Fixes:
https://redhat.atlassian.net/browse/RHIDP-12490
https://redhat.atlassian.net/browse/RHDHBUGS-2745
This PR contains the following changes:
Enterwith a emtpy stringLightspeed_stop_button.mov
How to test:
✔️ Checklist