Skip to content

fix: stream Ollama responses through AsyncClient instead of a sync generator - #108

Open
pm25coder wants to merge 1 commit into
knight22-21:mainfrom
pm25coder:fix/ollama-stream-nonblocking
Open

pm25coder wants to merge 1 commit into
knight22-21:mainfrom
pm25coder:fix/ollama-stream-nonblocking

Conversation

@pm25coder

Copy link
Copy Markdown
Contributor

_ollama_stream() awaited nothing: it iterated ollama.Client.chat(stream=True), which returns a synchronous generator, so every next() blocked the loop for the length of that network read. The async def wrapper only made the values awaitable, not the reads.

Switched the stream to ollama.AsyncClient, which returns an async iterator for the same request.

Change

  • devagent/core/llm.py: _ollama_stream() now uses an async client and does async for chunk in await client.chat(**kwargs).
  • The sync and async clients are built from one _ollama_connection_options() helper. They serve the same config from different call sites, so duplicating the host/key logic would have let the streaming path drift onto a different endpoint than the non-streaming one.
  • tests/test_ollama_stream_nonblocking.py (new): fake clients, no Ollama server contacted.

The non-streaming paths are untouched — complete() / complete_with_tools() are sync and are already called through run_in_executor, so _ollama_stream() was the only Ollama call running on the loop.

Test

test_ollama_stream_leaves_the_event_loop_responsive consumes the stream while a ticker coroutine counts its turns; the fake client sleeps between chunks as a network read would.

Same test, both directions:

_ollama_stream() ticker turns
before (sync generator) 0 — created, never scheduled
after (AsyncClient) 15

test_ollama_stream_yields_every_non_empty_chunk pins the chunk contract (text in order, empty content dropped) and passes on both trees, so the fix cannot quietly change what callers receive.

Local runs

  • pytest tests/test_ollama_stream_nonblocking.py tests/test_phase27.py tests/test_phase28.py tests/test_phase16.py tests/test_phase23.py -q — 62 passed.
  • Full suite, main at 5345a78: 7 failed, 835 passed, 1 skipped.
  • Full suite, this branch: 7 failed, 837 passed, 1 skipped.

The same 7 tests fail on both trees (test_phase17.py, test_phase38.py) with UnicodeEncodeError: 'gbk' codec can't encode character '\xe2' — a subprocess writing a non-ASCII character under a GBK console on this Windows host, unrelated to this change and left alone.

Closes #103

…nerator

_ollama_stream() is an async def that iterated ollama.Client.chat(stream=True),
which returns a synchronous generator: every next() blocked the event loop for
the length of that network read, so a streaming Ollama reply froze every other
coroutine for the whole response.

Await AsyncClient.chat() instead. Both clients are now built from one
_ollama_connection_options() helper, so the streaming and non-streaming paths
cannot drift onto different endpoints.

tests/test_ollama_stream_nonblocking.py drives the stream with fake clients
and measures the loop: a ticker coroutine gets 0 turns on the old code and
double-digit turns on the new one, while the chunk contract (text in order,
empty content dropped) is pinned in the same file.

Closes knight22-21#103
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

_ollama_stream() synchronous generator blocks the event loop during streaming

1 participant