Skip to content

Fix AsyncResponse.log_to_db - #1668

Merged
simonw merged 2 commits into
simonw:mainfrom
roli-lpci:fix/async-response-log-to-db
Sep 19, 2026
Merged

simonw merged 2 commits into
simonw:mainfrom
roli-lpci:fix/async-response-log-to-db

Conversation

@roli-lpci

@roli-lpci roli-lpci commented Sep 6, 2026 •

Copy link
Copy Markdown
Contributor

AsyncResponse inherited the synchronous log_to_db() implementation, which passed its awaitable duration_ms() and datetime_utc() results to SQLite after completion and raised sqlite3.ProgrammingError.

This preserves the synchronous log_to_db() call shape after awaiting the response, reuses the existing completed-response conversion used by the CLI, and raises the established await-required ValueError before completion.

The regression covers persisted response and conversation IDs, structured reasoning parts, and the precompletion guard.

Tests: pytest tests/test_logs_store.py::TestAsyncLogging tests/test_async_parity.py -q; offline echo persistence; Ruff; Black; MyPy; cog --check docs/logging.md.

@roli-lpci
roli-lpci force-pushed the fix/async-response-log-to-db branch from a0207a0 to 967bf6b Compare September 6, 2026 06:07
@roli-lpci
roli-lpci marked this pull request as draft September 6, 2026 06:08
@roli-lpci
roli-lpci marked this pull request as ready for review September 6, 2026 06:12
@simonw
simonw merged commit c9ccd3f into simonw:main Sep 19, 2026
1 check passed
@simonw simonw added the bug Something isn't working label Sep 19, 2026
simonw added a commit that referenced this pull request Sep 22, 2026
simonw pushed a commit that referenced this pull request Oct 18, 2026
…#1727 (#1734)

_BaseChainResponse.log_to_db() converted each AsyncResponse with
asyncio.run(), which raises RuntimeError inside a running event loop,
for example straight after await chain.text().

Since #1668 both Response.log_to_db() and AsyncResponse.log_to_db()
write synchronously, so the chain now calls each response's own method.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants