-
Notifications
You must be signed in to change notification settings - Fork 341
feat: SSE streaming for service-mode answer generation #2393
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: main
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 |
|---|---|---|
|
|
@@ -245,6 +245,58 @@ def query(self, query: str | list[str], *, top_k: int) -> list[list[dict[str, An | |
| except (ValidationError, ValueError) as exc: | ||
| raise RuntimeError(f"Service query returned invalid response: {exc}") from exc | ||
|
|
||
| async def aanswer_stream( | ||
| self, | ||
| query: str, | ||
| *, | ||
| top_k: int = 5, | ||
| include_chunks: bool = False, | ||
| include_metadata: bool = False, | ||
| reasoning_enabled: bool | None = None, | ||
| reference: str | None = None, | ||
| judge: bool = False, | ||
| ) -> AsyncIterator[dict[str, Any]]: | ||
| """Stream ``POST /v1/answer/stream`` SSE events for one answer request.""" | ||
| url = f"{self._base_url}/v1/answer/stream" | ||
| body: dict[str, Any] = { | ||
| "query": query, | ||
| "top_k": top_k, | ||
| "include_chunks": include_chunks, | ||
| "include_metadata": include_metadata, | ||
| "judge": judge, | ||
| } | ||
| if reasoning_enabled is not None: | ||
| body["reasoning_enabled"] = reasoning_enabled | ||
| if reference is not None: | ||
| body["reference"] = reference | ||
|
|
||
| async with httpx.AsyncClient( | ||
| timeout=httpx.Timeout(None, connect=30.0), | ||
| headers=self._auth_headers, | ||
| ) as client: | ||
| async with client.stream("POST", url, json=body) as response: | ||
| if response.status_code >= 400: | ||
| detail = (await response.aread()).decode(errors="replace")[:500] | ||
| raise RuntimeError(f"Service answer stream failed: HTTP {response.status_code}: {detail}") | ||
|
|
||
| event_type = "" | ||
| data_buf = "" | ||
| async for line in response.aiter_lines(): | ||
| if line.startswith("event:"): | ||
| event_type = line[6:].strip() | ||
| elif line.startswith("data:"): | ||
| data_buf = line[5:].strip() | ||
| elif line == "" and data_buf: | ||
| try: | ||
| payload = json.loads(data_buf) | ||
| except json.JSONDecodeError: | ||
| data_buf = "" | ||
| event_type = "" | ||
| continue | ||
|
Comment on lines
+291
to
+295
Contributor
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.
When an SSE event contains malformed JSON, this branch silently discards the event and resets its type, so callers receive neither the event nor an actionable stream error. Raise a descriptive client error instead of continuing with missing stream data. Knowledge Base Used: Service: HTTP/MCP API surface Prompt To Fix With AIThis is a comment left during a code review.
Path: nemo_retriever/src/nemo_retriever/service/client.py
Line: 291-295
Comment:
**Surface malformed SSE payloads**
When an SSE event contains malformed JSON, this branch silently discards the event and resets its type, so callers receive neither the event nor an actionable stream error. Raise a descriptive client error instead of continuing with missing stream data.
**Knowledge Base Used:** [Service: HTTP/MCP API surface](https://app.greptile.com/nvidia-public-github/-/custom-context/knowledge-base/nvidia/nemo-retriever/-/docs/service-api.md)
How can I resolve this? If you propose a fix, please make it concise. |
||
| yield {"event": event_type or "message", **payload} | ||
| data_buf = "" | ||
| event_type = "" | ||
|
Comment on lines
+248
to
+298
Contributor
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. The new Rule Used: New functionality must include corresponding unit ... (source) Knowledge Base Used: Service: HTTP/MCP API surface Prompt To Fix With AIThis is a comment left during a code review.
Path: nemo_retriever/src/nemo_retriever/service/client.py
Line: 248-298
Comment:
**Test the public stream client**
The new `aanswer_stream` method has no tests exercising its production httpx stream, status handling, or SSE parser; the added endpoint tests use `TestClient` and a separate duplicated parser, allowing client-side regressions to pass CI.
**Rule Used:** New functionality must include corresponding unit ... ([source](.greptile))
**Knowledge Base Used:** [Service: HTTP/MCP API surface](https://app.greptile.com/nvidia-public-github/-/custom-context/knowledge-base/nvidia/nemo-retriever/-/docs/service-api.md)
How can I resolve this? If you propose a fix, please make it concise. |
||
|
|
||
| # ------------------------------------------------------------------ | ||
| # Job lifecycle | ||
| # ------------------------------------------------------------------ | ||
|
|
||
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.
When a provider splits
</think>across streamed deltas, this branch clears the partial delimiter and leaves the filter in thinking mode, suppressing every subsequent visible token even though the finaldoneevent contains the batch-parsed answer.Knowledge Base Used: Model Backends
Prompt To Fix With AI