Skip to content

Commit 2b47837

Browse files
committed
fix(source-chat): make streaming cancellable on client disconnect and persist user message
Convert the source chat graph node and streaming endpoint to async so generation can be cancelled when the client disconnects. Persist the user message to the checkpoint up front via aupdate_state so it survives a mid-generation disconnect. Add a HybridSqliteSaver to delegate LangGraph async checkpointer calls to the existing sync SQLite connection. Update characterization tests for the async path and add a disconnect cancellation test.
1 parent 5f4f592 commit 2b47837

3 files changed

Lines changed: 132 additions & 113 deletions

File tree

api/routers/source_chat.py

Lines changed: 32 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
import json
33
from typing import AsyncGenerator, List, Optional
44

5-
from fastapi import APIRouter, HTTPException, Path
5+
from fastapi import APIRouter, HTTPException, Path, Request
66
from fastapi.responses import StreamingResponse
77
from langchain_core.messages import HumanMessage
88
from langchain_core.runnables import RunnableConfig
@@ -336,51 +336,38 @@ async def delete_source_chat_session(
336336

337337

338338
async def stream_source_chat_response(
339-
session_id: str, source_id: str, message: str, model_override: Optional[str] = None
339+
request: Request,
340+
session_id: str,
341+
source_id: str,
342+
message: str,
343+
model_override: Optional[str] = None,
340344
) -> AsyncGenerator[str, None]:
341345
"""Stream the source chat response as Server-Sent Events."""
346+
config = RunnableConfig(
347+
configurable={"thread_id": session_id, "model_id": model_override}
348+
)
349+
invoke_task: Optional[asyncio.Task] = None
342350
try:
343-
# Get current state
344-
# Use sync get_state() in a thread since SqliteSaver doesn't support async
345-
current_state = await asyncio.to_thread(
346-
source_chat_graph.get_state,
347-
config=RunnableConfig(configurable={"thread_id": session_id}),
351+
# Persist the user message to the checkpoint up front so it survives a
352+
# mid-generation disconnect (the frontend refetches the checkpoint on
353+
# cancel/complete and would otherwise drop the user's message).
354+
await source_chat_graph.aupdate_state(
355+
config, {"messages": [HumanMessage(content=message)]}
348356
)
349357

350-
# Prepare state for execution
351-
state_values = current_state.values if current_state else {}
352-
state_values["messages"] = state_values.get("messages", [])
353-
state_values["source_id"] = source_id
354-
state_values["model_override"] = model_override
355-
356-
# Add user message to state
357-
user_message = HumanMessage(content=message)
358-
state_values["messages"].append(user_message)
359-
360358
# Send user message event
361359
user_event = {"type": "user_message", "content": message, "timestamp": None}
362360
yield f"data: {json.dumps(user_event)}\n\n"
363361

364-
# Run the synchronous LangGraph invoke in a thread so it doesn't block the
365-
# event loop (mirrors the get_state() calls above). While the LLM generates,
366-
# emit an SSE comment every KEEPALIVE_INTERVAL_SECONDS so the connection
367-
# never goes idle — otherwise proxies (incl. the Next.js rewrite in front of
368-
# FastAPI) may drop it and the reply only appears after a refetch.
369-
# The lambda pins down which `invoke` overload is used; asyncio.to_thread
370-
# can't resolve overloaded callables on its own. The ignore is a langgraph
362+
# Run the async graph with ainvoke so generation is cancellable. Only the
363+
# per-message config is passed as input; the messages (incl. the user
364+
# message above) are read from the checkpoint. The ignore is a langgraph
371365
# typing limitation: it accepts a partial state dict at runtime, but the
372366
# signature requires the full state type.
373367
invoke_task = asyncio.create_task(
374-
asyncio.to_thread(
375-
lambda: source_chat_graph.invoke(
376-
input=state_values, # type: ignore[arg-type]
377-
config=RunnableConfig(
378-
configurable={
379-
"thread_id": session_id,
380-
"model_id": model_override,
381-
}
382-
),
383-
)
368+
source_chat_graph.ainvoke(
369+
input={"source_id": source_id, "model_override": model_override}, # type: ignore[call-overload]
370+
config=config,
384371
)
385372
)
386373
while True:
@@ -391,6 +378,9 @@ async def stream_source_chat_response(
391378
# Re-raises on graph error, caught by the outer try/except below.
392379
result = invoke_task.result()
393380
break
381+
if await request.is_disconnected():
382+
# Client went away — stop generating instead of burning tokens.
383+
return
394384
# SSE comment — ignored by clients, keeps the connection alive.
395385
yield ": ping\n\n"
396386

@@ -424,10 +414,17 @@ async def stream_source_chat_response(
424414
logger.error(f"Error in source chat streaming: {str(e)}")
425415
error_event = {"type": "error", "message": error_message}
426416
yield f"data: {json.dumps(error_event)}\n\n"
417+
finally:
418+
# Stop generation if the generator is torn down mid-flight (client
419+
# disconnect or server cancellation) so the model doesn't keep running.
420+
if invoke_task is not None and not invoke_task.done():
421+
invoke_task.cancel()
422+
await asyncio.gather(invoke_task, return_exceptions=True)
427423

428424

429425
@router.post("/sources/{source_id}/chat/sessions/{session_id}/messages")
430426
async def send_message_to_source_chat(
427+
http_request: Request,
431428
request: SendMessageRequest,
432429
source_id: str = Path(..., description="Source ID"),
433430
session_id: str = Path(..., description="Session ID"),
@@ -453,6 +450,7 @@ async def send_message_to_source_chat(
453450
# Return streaming response
454451
return StreamingResponse(
455452
stream_source_chat_response(
453+
http_request,
456454
session_id=full_session_id,
457455
source_id=full_source_id,
458456
message=request.message,

open_notebook/graphs/source_chat.py

Lines changed: 53 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ def _source_content_is_available(
4545
return isinstance(full_text, str) and bool(full_text.strip())
4646

4747

48-
def call_model_with_source_context(
48+
async def call_model_with_source_context(
4949
state: SourceChatState, config: RunnableConfig
5050
) -> dict:
5151
"""
@@ -58,50 +58,26 @@ def call_model_with_source_context(
5858
4. Tracks context indicators for referenced insights/content
5959
"""
6060
try:
61-
return _call_model_with_source_context_inner(state, config)
61+
return await _call_model_with_source_context_inner(state, config)
6262
except OpenNotebookError:
6363
raise
6464
except Exception as e:
6565
error_class, user_message = classify_error(e)
6666
raise error_class(user_message) from e
6767

6868

69-
def _call_model_with_source_context_inner(
69+
async def _call_model_with_source_context_inner(
7070
state: SourceChatState, config: RunnableConfig
7171
) -> dict:
7272
source_id = state.get("source_id")
7373
if not source_id:
7474
raise ValueError("source_id is required in state")
7575

76-
# Build source context using build_source_context (run async code in new loop)
77-
def build_context():
78-
"""Build context in a new event loop"""
79-
new_loop = asyncio.new_event_loop()
80-
try:
81-
asyncio.set_event_loop(new_loop)
82-
return new_loop.run_until_complete(
83-
build_source_context(
84-
source_id=source_id,
85-
max_tokens=50000, # Reasonable limit for source context
86-
)
87-
)
88-
finally:
89-
new_loop.close()
90-
asyncio.set_event_loop(None)
91-
92-
# Get the built context
93-
try:
94-
# Try to get the current event loop
95-
asyncio.get_running_loop()
96-
# If we're in an event loop, run in a thread with a new loop
97-
import concurrent.futures
98-
99-
with concurrent.futures.ThreadPoolExecutor() as executor:
100-
future = executor.submit(build_context)
101-
context_data = future.result()
102-
except RuntimeError:
103-
# No event loop running, safe to create a new one
104-
context_data = build_context()
76+
# Build source context (awaiting directly keeps the node cancellable).
77+
context_data = await build_source_context(
78+
source_id=source_id,
79+
max_tokens=50000, # Reasonable limit for source context
80+
)
10581

10682
# Extract source and insights from context
10783
source = None
@@ -149,47 +125,16 @@ def build_context():
149125
)
150126
payload = [SystemMessage(content=system_prompt)] + state.get("messages", [])
151127

152-
# Handle async model provisioning from sync context
153-
def run_in_new_loop():
154-
"""Run the async function in a new event loop"""
155-
new_loop = asyncio.new_event_loop()
156-
try:
157-
asyncio.set_event_loop(new_loop)
158-
return new_loop.run_until_complete(
159-
provision_langchain_model(
160-
str(payload),
161-
config.get("configurable", {}).get("model_id")
162-
or state.get("model_override"),
163-
"chat",
164-
max_tokens=8192,
165-
)
166-
)
167-
finally:
168-
new_loop.close()
169-
asyncio.set_event_loop(None)
170-
171-
try:
172-
# Try to get the current event loop
173-
asyncio.get_running_loop()
174-
# If we're in an event loop, run in a thread with a new loop
175-
import concurrent.futures
176-
177-
with concurrent.futures.ThreadPoolExecutor() as executor:
178-
future = executor.submit(run_in_new_loop)
179-
model = future.result()
180-
except RuntimeError:
181-
# No event loop running, safe to use asyncio.run()
182-
model = asyncio.run(
183-
provision_langchain_model(
184-
str(payload),
185-
config.get("configurable", {}).get("model_id")
186-
or state.get("model_override"),
187-
"chat",
188-
max_tokens=8192,
189-
)
190-
)
128+
# Provision the model asynchronously (cancellable)
129+
model = await provision_langchain_model(
130+
str(payload),
131+
config.get("configurable", {}).get("model_id")
132+
or state.get("model_override"),
133+
"chat",
134+
max_tokens=8192,
135+
)
191136

192-
ai_message = model.invoke(payload)
137+
ai_message = await model.ainvoke(payload)
193138

194139
# Clean thinking content from AI response (e.g., <think>...</think> tags)
195140
content = extract_text_content(ai_message.content)
@@ -211,12 +156,47 @@ def _format_source_context(context_data: Dict) -> str:
211156
return format_source_context(context_data)
212157

213158

159+
class HybridSqliteSaver(SqliteSaver):
160+
"""Sync SqliteSaver with async delegates for langgraph's async run path.
161+
162+
The source-chat node is async so the model call can be cancelled when the
163+
client disconnects; langgraph's ``ainvoke``/``aupdate_state`` then require
164+
an async checkpointer. SqliteSaver is sync-only (its async methods raise
165+
NotImplementedError), so delegate each async method to the corresponding
166+
sync one on a worker thread. The module-level sync connection — and the
167+
sync ``get_state`` callers elsewhere — keep working unchanged.
168+
"""
169+
170+
async def aget(self, config):
171+
return await asyncio.to_thread(self.get, config)
172+
173+
async def aget_tuple(self, config):
174+
return await asyncio.to_thread(self.get_tuple, config)
175+
176+
async def alist(self, config, *, filter=None, before=None, limit=None):
177+
items = await asyncio.to_thread(
178+
lambda: list(self.list(config, filter=filter, before=before, limit=limit))
179+
)
180+
for item in items:
181+
yield item
182+
183+
async def aput(self, config, checkpoint, metadata, new_versions):
184+
return await asyncio.to_thread(
185+
self.put, config, checkpoint, metadata, new_versions
186+
)
187+
188+
async def aput_writes(self, config, writes, task_id, task_path=""):
189+
return await asyncio.to_thread(
190+
self.put_writes, config, writes, task_id, task_path
191+
)
192+
193+
214194
# Create SQLite checkpointer
215195
conn = sqlite3.connect(
216196
LANGGRAPH_CHECKPOINT_FILE,
217197
check_same_thread=False,
218198
)
219-
memory = SqliteSaver(conn)
199+
memory = HybridSqliteSaver(conn)
220200

221201
# Create the StateGraph
222202
source_chat_state = StateGraph(SourceChatState)

tests/test_chat_routers_characterization.py

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
tests/test_crud_404.py.
1515
"""
1616

17-
import time
17+
import asyncio
1818
from types import SimpleNamespace
1919
from unittest.mock import AsyncMock, MagicMock, patch
2020

@@ -346,21 +346,62 @@ async def test_stream_source_chat_emits_keepalive_while_invoke_runs():
346346
with patch.object(
347347
source_chat_router, "KEEPALIVE_INTERVAL_SECONDS", 0.01
348348
), patch.object(source_chat_router, "source_chat_graph") as mock_graph:
349-
mock_graph.get_state.return_value = _graph_state({"messages": []})
349+
mock_graph.aupdate_state = AsyncMock()
350350

351-
def slow_invoke(*_args, **_kwargs):
352-
time.sleep(0.1)
351+
async def slow_ainvoke(*_args, **_kwargs):
352+
await asyncio.sleep(0.1)
353353
return {"messages": []}
354354

355-
mock_graph.invoke.side_effect = slow_invoke
355+
mock_graph.ainvoke.side_effect = slow_ainvoke
356+
357+
request = MagicMock()
358+
request.is_disconnected = AsyncMock(return_value=False)
356359

357360
chunks = []
358361
async for chunk in stream_source_chat_response(
359-
"chat_session:abc", "source:xyz", "hello"
362+
request, "chat_session:abc", "source:xyz", "hello"
360363
):
361364
chunks.append(chunk)
362365

363366
assert chunks[0].startswith('data: {"type": "user_message"')
364367
assert chunks[-1].startswith('data: {"type": "complete"')
365368
# Keepalive comments are emitted between the user_message and completion.
366369
assert ": ping\n\n" in chunks
370+
371+
372+
@pytest.mark.asyncio
373+
async def test_stream_source_chat_cancels_invoke_on_disconnect():
374+
"""When the client disconnects mid-generation, generation is cancelled
375+
server-side and the stream ends without a completion event."""
376+
from api.routers import source_chat as source_chat_router
377+
from api.routers.source_chat import stream_source_chat_response
378+
379+
cancelled = asyncio.Event()
380+
381+
async def blocking_ainvoke(*_args, **_kwargs):
382+
try:
383+
await asyncio.Event().wait() # blocks until cancelled
384+
except asyncio.CancelledError:
385+
cancelled.set()
386+
raise
387+
388+
with patch.object(
389+
source_chat_router, "KEEPALIVE_INTERVAL_SECONDS", 0.01
390+
), patch.object(source_chat_router, "source_chat_graph") as mock_graph:
391+
mock_graph.aupdate_state = AsyncMock()
392+
mock_graph.ainvoke.side_effect = blocking_ainvoke
393+
394+
request = MagicMock()
395+
request.is_disconnected = AsyncMock(return_value=True)
396+
397+
chunks = []
398+
async for chunk in stream_source_chat_response(
399+
request, "chat_session:abc", "source:xyz", "hello"
400+
):
401+
chunks.append(chunk)
402+
403+
# The user message is still yielded up front, but the stream stops without
404+
# a completion event and the in-flight invoke task was cancelled.
405+
assert chunks[0].startswith('data: {"type": "user_message"')
406+
assert not any(c.startswith('data: {"type": "complete"') for c in chunks)
407+
assert cancelled.is_set()

0 commit comments

Comments
 (0)