Skip to content

Commit 6c43e95

Browse files
authored
fix(chat): persist user message and partial reply when stream is aborted (eneo-ai#349) (eneo-ai#412)
* fix(chat): persist user message and partial reply when stream is aborted (eneo-ai#349) Two-phase persistence so an aborted SSE stream no longer loses the user's question or the partially-streamed assistant reply. Phase 1: SessionService.create_question_placeholder inserts a Questions row with the user's question and an empty answer before the LLM stream begins. The router's existing setup transaction commits this row before any byte streams out, so it is durable regardless of what happens during streaming. Phase 2: complete_question_with_answer (normal completion) or the new module-level persist_partial_question_answer (abort path) update the row. The abort helper opens a fresh sessionmanager.session() instead of riding the request-scoped AsyncSession, which FastAPI tears down concurrently with the SSE close. The streaming generator's finally fires the partial save via asyncio.create_task — no await across GeneratorExit, no reliance on a dying session. Pattern applied to both assistant_service._handle_response and the group_chat_service selector-echo path. The main group-chat happy path delegates to assistant_service.ask and is covered automatically. Frontend: ChatService.askQuestion no longer early-returns on abort, letting reloadHistory run so the sidebar reflects the persisted state without a manual refresh. Removed the now-orphan SessionService.add_question_to_session — zero callers in src, tests, frontend, or docs after the swap. 2523 unit tests remain green; 6 new unit tests + 4 new integration tests against real Postgres cover the new paths. Closes eneo-ai#349. * style(sessions): apply ruff format to session_service.py * fix(chat): harden abort persistence per review (eneo-ai#412) Addresses four issues raised in PR review: 1. count_tokens safety. tiktoken can raise on unknown model names, which previously killed the asyncio.create_task call in the streaming generator's finally and silently collapsed the entire partial save. New safe_count_tokens helper catches and falls back to 0. Used both in create_question_placeholder (seeds num_tokens_question) and in the two _handle_response finally blocks. 2. Skip redundant UPDATE when no content streamed. If the LLM errors before yielding any chunks (or the user aborts that early), the placeholder already captures the question and an answer="" UPDATE is a no-op. Add `if not completed and response_string` guard. Document that placeholder rows can appear with answer="" after pre-stream LLM errors — that is intentional, the row reflects what the user asked. 3. Strong-ref background tasks. asyncio.create_task internally holds only a weak reference; without a strong ref a GC pass can cancel the task mid-flight (silent data loss on exactly the path this PR exists to protect). New schedule_background_save helper keeps a module-level set and removes via add_done_callback. Both assistant_service and group_chat_service now route through it. 4. Backfill num_tokens_question on placeholder insert. create_question_placeholder now seeds num_tokens_question via safe_count_tokens(question, model), so usage analytics no longer undercount aborted requests. The normal- completion path overwrites with provider-reported counts as before. Adds 5 new unit tests: - placeholder seeds num_tokens_question from count_tokens - placeholder falls back to 0 when count_tokens raises - placeholder records 0 when no completion model - streaming generator skips partial save when response_string is empty - streaming generator schedules partial save even when count_tokens raises - schedule_background_save keeps strong ref until task completion
1 parent b36b2b5 commit 6c43e95

7 files changed

Lines changed: 1424 additions & 252 deletions

File tree

backend/src/intric/assistants/assistant_service.py

Lines changed: 226 additions & 172 deletions
Large diffs are not rendered by default.

backend/src/intric/group_chat/application/group_chat_service.py

Lines changed: 72 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -337,62 +337,88 @@ async def _handle_response(
337337
completion_model: "CompletionModel",
338338
session: "SessionInDB",
339339
stream: bool,
340+
question_id: "UUID",
340341
assistant_selector_tokens: int = 0,
341342
):
342343
"""Handle response for group chat selector, matching assistant_service"""
343344

345+
# Capture tenant_id outside the generator so the abort-path background save
346+
# doesn't depend on self.user being safely accessible during teardown.
347+
tenant_id = self.user.tenant_id
348+
344349
if stream:
345350

346351
async def response_stream():
347-
chunk_response = response.split()
348352
response_string = ""
349-
for i, chunk in enumerate(chunk_response):
350-
if i < len(chunk_response):
351-
chunk_text = chunk + " "
352-
else:
353-
chunk_text = chunk
354-
355-
response_string += chunk_text
356-
# yield empty references and chunk text, matching assistant_service format
357-
yield Completion(
358-
text=chunk_text,
359-
response_type=ResponseType.TEXT,
360-
reference_chunks=[],
353+
completed = False
354+
355+
try:
356+
chunk_response = response.split()
357+
for i, chunk in enumerate(chunk_response):
358+
if i < len(chunk_response):
359+
chunk_text = chunk + " "
360+
else:
361+
chunk_text = chunk
362+
363+
response_string += chunk_text
364+
# yield empty references and chunk text, matching assistant_service format
365+
yield Completion(
366+
text=chunk_text,
367+
response_type=ResponseType.TEXT,
368+
reference_chunks=[],
369+
)
370+
await asyncio.sleep(0.05)
371+
372+
# NOTE: refactor question_token_count to include the whole contructed prompt.
373+
question_token_count = count_tokens(question, completion_model.name)
374+
token_count = count_tokens(response, completion_model.name)
375+
await self.session_service.complete_question_with_answer(
376+
question_id=question_id,
377+
answer=response,
378+
num_tokens_question=question_token_count
379+
+ assistant_selector_tokens,
380+
num_tokens_answer=token_count,
381+
completion_model=completion_model, # pyright: ignore[reportArgumentType] # domain.CompletionModel vs ai_models.CompletionModel; structurally compatible at runtime
382+
info_blob_chunks=[],
383+
logging_details=None,
361384
)
362-
await asyncio.sleep(0.05)
363-
364-
# NOTE: refactor question_token_count to include the whole contructed prompt.
365-
question_token_count = count_tokens(question, completion_model.name)
366-
token_count = count_tokens(response, completion_model.name)
367-
await self.session_service.add_question_to_session(
368-
question=question,
369-
answer=response,
370-
num_tokens_question=question_token_count
371-
+ assistant_selector_tokens,
372-
num_tokens_answer=token_count,
373-
session=session,
374-
completion_model=completion_model, # pyright: ignore[reportArgumentType] # domain.CompletionModel vs ai_models.CompletionModel; structurally compatible at runtime
375-
info_blob_chunks=[],
376-
files=[],
377-
logging_details=None,
378-
)
385+
completed = True
386+
finally:
387+
# Selector-echo stream did not reach normal completion. The
388+
# placeholder already captures the question; only schedule a
389+
# background UPDATE when there's actual content to persist.
390+
if not completed and response_string:
391+
from intric.sessions.session_service import (
392+
persist_partial_question_answer,
393+
safe_count_tokens,
394+
schedule_background_save,
395+
)
396+
397+
partial_tokens_answer = safe_count_tokens(
398+
response_string, completion_model.name
399+
)
400+
schedule_background_save(
401+
persist_partial_question_answer(
402+
tenant_id=tenant_id,
403+
question_id=question_id,
404+
answer=response_string,
405+
num_tokens_answer=partial_tokens_answer,
406+
)
407+
)
379408

380409
return response_stream()
381410
else:
382411
# NOTE: refactor question_token_count to include the whole contructed prompt.
383412
question_token_count = count_tokens(question, completion_model.name)
384413
token_count = count_tokens(response, completion_model.name)
385-
await self.session_service.add_question_to_session(
386-
question=question,
414+
await self.session_service.complete_question_with_answer(
415+
question_id=question_id,
387416
answer=response,
388417
num_tokens_question=question_token_count + assistant_selector_tokens,
389418
num_tokens_answer=token_count,
390-
session=session,
391419
completion_model=completion_model, # pyright: ignore[reportArgumentType] # domain.CompletionModel vs ai_models.CompletionModel; structurally compatible at runtime
392420
info_blob_chunks=[],
393-
files=[],
394421
logging_details=None,
395-
assistant_id=None,
396422
)
397423
return response
398424

@@ -466,12 +492,22 @@ async def ask_group_chat(
466492
assert (
467493
first_completion_model is not None
468494
) # assistant must have a model to be usable
495+
# Persist a placeholder Question row before the selector echo streams out, so
496+
# the user's question survives even if the stream is aborted.
497+
question_id = await self.session_service.create_question_placeholder(
498+
question=question,
499+
session=session,
500+
files=[],
501+
assistant_id=None,
502+
completion_model=first_completion_model, # pyright: ignore[reportArgumentType] # domain.CompletionModel vs ai_models.CompletionModel; structurally compatible at runtime
503+
)
469504
final_response = await self._handle_response(
470505
response=response_from_selector,
471506
question=question,
472507
completion_model=first_completion_model, # pyright: ignore[reportArgumentType] # domain.CompletionModel vs ai_models.CompletionModel; structurally compatible at runtime
473508
session=session,
474509
stream=stream,
510+
question_id=question_id,
475511
assistant_selector_tokens=selection_result.assistant_selector_tokens,
476512
)
477513
response = AssistantResponse(
@@ -484,6 +520,7 @@ async def ask_group_chat(
484520
tools=UseTools(assistants=[]),
485521
description=None,
486522
web_search_results=[],
523+
question_id=question_id,
487524
)
488525
else:
489526
response = await self.assistant_service.ask(

backend/src/intric/questions/questions_repo.py

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525

2626
if TYPE_CHECKING:
2727
from intric.completion_models.infrastructure.web_search import WebSearchResult
28+
from intric.logging.logging import LoggingDetails
29+
from intric.questions.question import ToolCallInfo
2830

2931

3032
class QuestionRepository:
@@ -129,6 +131,77 @@ async def _add_web_search_results(
129131
async def get(self, id: UUID):
130132
return await self.delegate.get(id)
131133

134+
async def update_with_answer(
135+
self,
136+
*,
137+
question_id: UUID,
138+
tenant_id: UUID,
139+
answer: str,
140+
num_tokens_question: int | None = None,
141+
num_tokens_answer: int | None = None,
142+
completion_model_id: UUID | None = None,
143+
tool_calls: list["ToolCallInfo"] | None = None,
144+
info_blob_chunks: list[InfoBlobChunkInDBWithScore] | None = None,
145+
generated_files: list[File] | None = None,
146+
web_search_results: list["WebSearchResult"] | None = None,
147+
logging_details: "LoggingDetails | None" = None,
148+
) -> None:
149+
"""Update an existing placeholder Question row with the final or partial answer.
150+
151+
Used both for normal stream completion (full answer + token counts + late-bound rows)
152+
and for partial persistence on abort (just the answer text + estimated token counts).
153+
154+
tenant_id is required in the WHERE clause to defend against cross-tenant writes if a
155+
caller ever supplies a stale question_id.
156+
"""
157+
logging_details_id: object = None
158+
if logging_details is not None:
159+
log_stmt = ( # pyright: ignore[reportUnknownVariableType] # logging_table is imperatively mapped
160+
sa.insert(logging_table)
161+
.values(**logging_details.model_dump())
162+
.returning(logging_table)
163+
)
164+
log_result = await self.session.execute(log_stmt) # pyright: ignore[reportUnknownArgumentType, reportUnknownVariableType]
165+
logging_row = log_result.scalar_one() # pyright: ignore[reportUnknownVariableType]
166+
logging_details_id = logging_row.id # pyright: ignore[reportUnknownMemberType, reportUnknownVariableType]
167+
168+
update_values: dict[str, Any] = {"answer": answer}
169+
if num_tokens_question is not None:
170+
update_values["num_tokens_question"] = num_tokens_question
171+
if num_tokens_answer is not None:
172+
update_values["num_tokens_answer"] = num_tokens_answer
173+
if completion_model_id is not None:
174+
update_values["completion_model_id"] = completion_model_id
175+
if tool_calls is not None:
176+
update_values["tool_calls"] = [tc.model_dump() for tc in tool_calls]
177+
if logging_details_id is not None:
178+
update_values["logging_details_id"] = logging_details_id
179+
180+
update_stmt = (
181+
sa.update(Questions)
182+
.where(Questions.id == question_id)
183+
.where(Questions.tenant_id == tenant_id)
184+
.values(**update_values)
185+
)
186+
await self.session.execute(update_stmt)
187+
188+
if info_blob_chunks:
189+
await self._add_references(
190+
question_id=question_id, # type: ignore[arg-type] # helper annotated as int but ID is UUID
191+
chunks=info_blob_chunks,
192+
)
193+
if generated_files:
194+
await self._add_files(
195+
question_id=question_id, # type: ignore[arg-type] # helper annotated as int but ID is UUID
196+
files=list(generated_files),
197+
file_type="assistant",
198+
)
199+
if web_search_results:
200+
await self._add_web_search_results(
201+
web_search_results=list(web_search_results),
202+
question_id=question_id,
203+
)
204+
132205
async def add(
133206
self,
134207
question: QuestionAdd,

0 commit comments

Comments
 (0)