fix: thread yielded signals through assign + return#61
Merged
Conversation
`r = await coro` and `return await coro` silently lost the awaited
value when the inner coroutine had its own internal `await` (e.g.
`await asyncio.sleep(0)`). Real-shape async code surfaced this
quickly: any helper that wraps a sync result with a cooperative
yield (`async def aw(v): await asyncio.sleep(0); return v`) and is
then awaited returned None instead of v.
Two parallel bugs, same root cause: continuation frames were
ordered wrong relative to the awaited expression's existing cont.
## eval_assign was prepending :cont_bind_sent (lib/pyex/interpreter/bindings.ex)
The original pattern was:
{{:yielded, val, cont}, env, ctx} ->
{{:yielded, val, [{:cont_bind_sent, name} | cont]}, env, ctx}
That works for `r = yield X` because `yield X` returns an empty
cont — prepend or append produces the same `[cont_bind_sent("r")]`.
For `r = await coro`, the await's cont already contains
`:cont_await_iter` that has to drive the coroutine to completion
*before* any value exists to bind. With prepend, resume processed
:cont_bind_sent first (binding nil) and only then advanced the
await — discarding the return value.
Fix: append. The cont becomes
`[cont_await_iter(coro_id), cont_bind_sent("r")]`. When
:cont_await_iter completes it routes the coroutine's return value
through `resume_generator_with_send`, which is exactly what
:cont_bind_sent consumes.
## eval_return wasn't propagating yielded signals at all
case eval(expr, env, ctx) do
{{:exception, _} = signal, env, ctx} -> {signal, env, ctx}
{value, env, ctx} -> {{:returned, value}, env, ctx}
end
A `{:yielded, val, cont}` tuple from the awaited expression matched
the catch-all and got wrapped as a returned VALUE. So the
function literally returned the yielded-tuple instead of the
awaited value.
Fix: add a yielded clause that propagates the yield with a new
`:cont_return_value` frame appended to the inner cont. Resume
handlers send the awaited value through as the function's return.
Both shapes now have regression tests in
test/pyex/async_conformance_test.exs covering: bind-on-await,
return-on-await, and a three-level await chain that stresses
ordering at every level.
5346 + 3 = 5349 tests, 0 failures. Dialyzer clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
`r = await coro` and `return await coro` silently lost the awaited value when the inner coroutine had its own internal `await` (e.g. `await asyncio.sleep(0)`). Real-shape async code surfaces this fast: any helper that wraps a sync result with a cooperative yield (`async def aw(v): await asyncio.sleep(0); return v`) and is then awaited returned None instead of `v`.
Two parallel bugs, same root cause
Continuation frames were ordered wrong relative to the awaited expression's existing cont.
eval_assignwas prepending:cont_bind_sent```elixir
{{:yielded, val, cont}, env, ctx} ->
{{:yielded, val, [{:cont_bind_sent, name} | cont]}, env, ctx}
```
Works for `r = yield X` because `yield X` returns an empty cont — prepend or append produces the same `[cont_bind_sent("r")]`.
For `r = await coro`, the await's cont already contains `:cont_await_iter` which has to drive the coroutine to completion before any value exists to bind. With prepend, resume processed `:cont_bind_sent` first (binding nil) and only then advanced the await — discarding the return value.
Fix: append. The cont becomes `[cont_await_iter(coro_id), cont_bind_sent("r")]`. When `:cont_await_iter` completes it routes the coroutine's return value through `resume_generator_with_send`, which is exactly what `:cont_bind_sent` consumes.
eval_returnwasn't propagating yielded signals at all```elixir
case eval(expr, env, ctx) do
{{:exception, _} = signal, env, ctx} -> {signal, env, ctx}
{value, env, ctx} -> {{:returned, value}, env, ctx}
end
```
A `{:yielded, val, cont}` tuple from the awaited expression matched the catch-all and got wrapped as a returned VALUE. So the function literally returned the yielded-tuple instead of the awaited value.
Fix: yielded clause that propagates the yield with a new `:cont_return_value` frame appended. Resume handlers send the awaited value through as the function's return.
Tests
Three regression tests in `test/pyex/async_conformance_test.exs` covering bind-on-await, return-on-await, and a three-level await chain that stresses ordering at every level.
Why these slipped through
Phase 1.5's conformance suite covered `await coro` standalone, `await` in expression positions (binop, f-string, comprehension), and `gather`. But every existing test with a non-trivial `await` was using either:
So the cont chain stayed simple — `:cont_await_iter` was always the last frame. The bugs only fire when the inner coroutine itself yields, which means a `:cont_await_iter` frame and a `:cont_bind_sent`/`:cont_return_value` frame coexist. A real-shape agent demo (parallel tool calls wrapped with `asyncio.sleep(0)` cooperative yields) caught it on first run.
Test plan
🤖 Generated with Claude Code