-
Notifications
You must be signed in to change notification settings - Fork 22
Add SSE streaming for chat responses #8 #70
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,9 @@ | ||
| import json | ||
| import os | ||
| from typing import List, Optional, Dict, Any | ||
| from fastapi import FastAPI, HTTPException | ||
| from fastapi.responses import StreamingResponse | ||
| from pydantic import BaseModel | ||
| import asyncio | ||
| import json | ||
| import logging | ||
|
|
@@ -946,6 +952,55 @@ async def event_generator(): | |
| raise HTTPException(status_code=500, detail="Internal server error") from e | ||
|
|
||
|
|
||
| @app.post("/chat/stream") | ||
| async def chat_stream(request: ChatRequest): | ||
| if not GEMINI_API_KEY: | ||
| raise HTTPException(status_code=500, detail="GEMINI_API_KEY is not configured.") | ||
|
|
||
| chat_id = request.chat_id or "default" | ||
| if chat_id not in sessions: | ||
| model = get_model() | ||
| sessions[chat_id] = model.start_chat(history=[]) | ||
|
|
||
| chat_session = sessions[chat_id] | ||
|
|
||
| async def event_stream(): | ||
| full_text_parts: List[str] = [] | ||
| try: | ||
| response_stream = None | ||
| if hasattr(chat_session, "send_message_async"): | ||
| response_stream = await chat_session.send_message_async(request.message, stream=True) | ||
| else: | ||
| response_stream = chat_session.send_message(request.message, stream=True) | ||
|
|
||
| async for chunk in response_stream: | ||
| text = getattr(chunk, "text", None) | ||
| if text is None and isinstance(chunk, dict): | ||
| text = chunk.get("text") | ||
| if text is None: | ||
| continue | ||
|
|
||
| full_text_parts.append(text) | ||
| payload = json.dumps({"delta": text}) | ||
| yield f"event: delta\ndata: {payload}\n\n" | ||
| except Exception as exc: # pragma: no cover - defensive path | ||
| error_payload = json.dumps({"error": str(exc)}) | ||
| yield f"event: error\ndata: {error_payload}\n\n" | ||
|
Comment on lines
+986
to
+988
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win Do not expose raw upstream exceptions in the public SSE contract. Provider exception strings can disclose internal request or configuration details. Emit a stable public error code/message and log the original exception server-side; update the test to assert that safe contract instead.
🧰 Tools🪛 ast-grep (0.44.1)[info] 189-189: use jsonify instead of json.dumps for JSON output (use-jsonify) 🪛 Ruff (0.15.21)[warning] 189-189: Do not catch blind exception: (BLE001) 📍 Affects 2 files
🤖 Prompt for AI Agents |
||
| return | ||
|
|
||
| final_payload = json.dumps({"chat_id": chat_id, "text": "".join(full_text_parts)}) | ||
| yield f"event: done\ndata: {final_payload}\n\n" | ||
|
|
||
| return StreamingResponse( | ||
| event_stream(), | ||
| media_type="text/event-stream", | ||
| headers={ | ||
| "Cache-Control": "no-cache", | ||
| "X-Accel-Buffering": "no", | ||
| }, | ||
| ) | ||
|
|
||
|
|
||
| @app.delete("/chat/{chat_id}") | ||
| async def delete_chat(chat_id: str): | ||
| try: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,9 @@ python-dotenv==0.21.0 | |
| google-generativeai==0.8.3 | ||
| pydantic>=2.5.3 | ||
| stellar-sdk>=12.0.0 | ||
| pytest | ||
| pytest-asyncio | ||
|
Comment on lines
+7
to
+8
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
fd -a 'requirements*.txt|pyproject.toml|pytest.ini|tox.ini|.python-version|.tool-versions|Dockerfile' . -E venv
rg -n --hidden -g '!venv/**' \
'pytest(-asyncio)?|setup-python|python-version' \
requirements.txt .github 2>/dev/null || trueRepository: Deen-Bridge/dnb-ai Length of output: 1573 🏁 Script executed: sed -n '1,120p' requirements.txt && printf '\n---\n' && sed -n '1,120p' .github/workflows/ci.yml && printf '\n---\n' && sed -n '1,120p' pytest.iniRepository: Deen-Bridge/dnb-ai Length of output: 3537 Pin 🤖 Prompt for AI Agents |
||
| httpx | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| PyYAML==6.0.2 | ||
| numpy==2.2.4 | ||
| redis>=5.0.0,<6.0.0 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| import pytest | ||
| from fastapi.testclient import TestClient | ||
|
|
||
| import main | ||
|
|
||
|
|
||
| class FakeChunk: | ||
| def __init__(self, text: str): | ||
| self.text = text | ||
|
|
||
|
|
||
| class FakeChatSession: | ||
| def __init__(self): | ||
| self.history = [] | ||
|
|
||
| async def send_message_async(self, message, stream=True): | ||
| self.history.append({"role": "user", "text": message}) | ||
|
|
||
| async def generator(): | ||
| yield FakeChunk("Hello") | ||
| yield FakeChunk(" world") | ||
|
|
||
| return generator() | ||
|
|
||
|
|
||
| class FailingChatSession(FakeChatSession): | ||
| async def send_message_async(self, message, stream=True): | ||
| self.history.append({"role": "user", "text": message}) | ||
|
|
||
| async def generator(): | ||
| yield FakeChunk("Hello") | ||
| raise RuntimeError("upstream failed") | ||
|
|
||
| return generator() | ||
|
|
||
|
|
||
| @pytest.fixture(autouse=True) | ||
| def reset_app_state(monkeypatch): | ||
| main.sessions.clear() | ||
| monkeypatch.setattr(main, "GEMINI_API_KEY", "fake-key") | ||
| monkeypatch.setattr(main, "CITATION_VERIFY_MODE", "off") | ||
| monkeypatch.setattr(main, "get_model", lambda: None) | ||
|
|
||
|
|
||
| def test_chat_stream_endpoint_emits_sse_events(monkeypatch): | ||
| chat_session = FakeChatSession() | ||
| monkeypatch.setattr(main, "get_model", lambda: type("Model", (), {"start_chat": lambda self, history=[]: chat_session})()) | ||
|
|
||
| client = TestClient(main.app) | ||
| response = client.post("/chat/stream", json={"message": "Hello", "chat_id": "stream-test"}) | ||
|
|
||
| assert response.status_code == 200 | ||
| assert response.headers["content-type"].startswith("text/event-stream") | ||
| assert "event: delta" in response.text | ||
| assert "Hello" in response.text | ||
| assert "event: done" in response.text | ||
| assert '"chat_id": "stream-test"' in response.text | ||
|
|
||
|
|
||
| def test_chat_stream_endpoint_emits_error_event_on_upstream_failure(monkeypatch): | ||
| chat_session = FailingChatSession() | ||
| monkeypatch.setattr(main, "get_model", lambda: type("Model", (), {"start_chat": lambda self, history=[]: chat_session})()) | ||
|
|
||
| client = TestClient(main.app) | ||
| response = client.post("/chat/stream", json={"message": "Hello", "chat_id": "broken-stream"}) | ||
|
|
||
| assert response.status_code == 200 | ||
| assert response.headers["content-type"].startswith("text/event-stream") | ||
| assert "event: error" in response.text | ||
| assert "upstream failed" in response.text |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the
errorevent too.The endpoint emits
event: error, but this contract only describesdeltaanddone; clients need the failure payload and terminal semantics to handle upstream failures correctly.🤖 Prompt for AI Agents