feat: add single-threaded event loop for Promise.all and async array callbacks#33
Merged
TheUncharted merged 3 commits intomasterfrom Mar 13, 2026
Merged
feat: add single-threaded event loop for Promise.all and async array callbacks#33TheUncharted merged 3 commits intomasterfrom
TheUncharted merged 3 commits intomasterfrom
Conversation
…e the VM AI-generated code like `Promise.all(arr.map(async fn => await external()))` previously crashed the VM because Rust's for-loop couldn't be suspended mid-iteration. Users can now use async array callbacks with external function calls — each await suspends and resumes sequentially via a continuation-based execution model.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a continuation mechanism to the VM to support async callbacks in array methods (e.g., Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant VM as VM
participant ContinuationStack as Continuation\r\nStack
participant Callback as Callback
Client->>VM: execute_array_map([item1, item2], asyncCallback)
activate VM
VM->>VM: detect async callback
VM->>ContinuationStack: start_continuation_map(callback, [item1, item2])
activate ContinuationStack
ContinuationStack->>Callback: invoke(item1)
activate Callback
Callback->>VM: await external call (suspend)
deactivate Callback
VM->>Client: suspend (external call pending)
deactivate VM
Client->>VM: resume with result1
activate VM
VM->>ContinuationStack: process_continuation()
ContinuationStack->>ContinuationStack: store result1, advance index
ContinuationStack->>Callback: invoke(item2)
activate Callback
Callback->>VM: await external call (suspend)
deactivate Callback
VM->>Client: suspend (external call pending)
deactivate VM
Client->>VM: resume with result2
activate VM
VM->>ContinuationStack: process_continuation()
ContinuationStack->>ContinuationStack: store result2, complete
ContinuationStack->>VM: return aggregated results
VM->>Client: complete with aggregated results
deactivate VM
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
Contributor
Benchmark Results |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/zapcode-core/src/vm/mod.rs`:
- Around line 397-414: process_continuation currently only checks that
callback_frame_index was popped (by comparing self.frames.len()), but you must
also ensure execution has returned to the original caller frame depth to avoid
advancing stale continuations on unwinds; update the match arms for
Continuation::ArrayMap and Continuation::ArrayForEach to also extract the saved
caller_frame_depth and then only proceed when self.frames.len() ==
caller_frame_depth (otherwise return Ok(false)), ensuring both the
callback_frame_index check and the caller_frame_depth check gate continuation
completion.
- Around line 417-418: The line assigning callback_result uses
self.pop().unwrap_or(Value::Undefined) which masks stack underflow; change it to
surface an error instead of defaulting to Value::Undefined by checking
self.pop() and returning or propagating a stack-underflow error (e.g. convert to
self.pop().ok_or(VMError::StackUnderflow) or similar) so the caller of the
containing function (where callback_result is computed) receives a Result/Err on
underflow; adjust the containing function's signature/propagation to return that
error rather than silently using Value::Undefined.
- Around line 419-437: The current unwrapping in the callback result handling
(inside mod.rs where callback_result is inspected and self.continuations.pop()
is used on rejection) incorrectly treats any object with a "status" field as an
internal promise; update the check to only unwrap objects that have the internal
promise marker (e.g. require a __promise__ boolean flag alongside status) before
treating as {status: "resolved"/"rejected"} so ordinary user objects are left
untouched, and keep the existing behavior of popping continuations and returning
ZapcodeError::RuntimeError on rejected internal promises.
In `@crates/zapcode-core/tests/async_await.rs`:
- Around line 653-865: Add new tests alongside the existing map tests: a
positive test function (e.g. test_array_for_each_async_callback_with_external)
that uses items.forEach(async (item) => { await fetchData(item); }) with
start_with_externals("fetchData") and asserts the sequence of suspended external
calls and final VmState::Complete (and any side-effect results if forEach
returns undefined), an edge-case test (e.g. test_array_for_each_async_empty)
where items: string[] = [] that asserts immediate VmState::Complete and correct
empty/no-op behavior, and a negative test (e.g.
test_array_async_unsupported_methods_filter_reduce) that evaluates code using
async callbacks with .filter() and .reduce() and asserts the VM returns the
explicit guard error (match VmState::Complete(Value::Error(...)) or appropriate
panic expectations) to cover the guard paths introduced for async callbacks;
follow the naming pattern used by existing tests (test_array_map_*) and include
a sandbox-escape case if applicable per guidelines.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c3c03731-c608-4999-ab4b-2907771528b7
📒 Files selected for processing (8)
README.mdcrates/zapcode-core/benches/execution.rscrates/zapcode-core/src/snapshot.rscrates/zapcode-core/src/vm/mod.rscrates/zapcode-core/tests/async_await.rsexamples/python/basic/main.pyexamples/rust/basic/basic.rsexamples/typescript/basic/main.ts
Comment on lines
+417
to
+418
| let callback_result = self.pop().unwrap_or(Value::Undefined); | ||
|
|
Contributor
There was a problem hiding this comment.
Do not swallow stack underflow when reading callback return values.
Using unwrap_or(Value::Undefined) here masks stack-discipline bugs and can silently produce wrong map/forEach outputs.
Suggested fix
- let callback_result = self.pop().unwrap_or(Value::Undefined);
+ let callback_result = self.pop()?;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let callback_result = self.pop().unwrap_or(Value::Undefined); | |
| let callback_result = self.pop()?; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/zapcode-core/src/vm/mod.rs` around lines 417 - 418, The line assigning
callback_result uses self.pop().unwrap_or(Value::Undefined) which masks stack
underflow; change it to surface an error instead of defaulting to
Value::Undefined by checking self.pop() and returning or propagating a
stack-underflow error (e.g. convert to self.pop().ok_or(VMError::StackUnderflow)
or similar) so the caller of the containing function (where callback_result is
computed) receives a Result/Err on underflow; adjust the containing function's
signature/propagation to return that error rather than silently using
Value::Undefined.
User objects like {status: "resolved", value: 42} were incorrectly unwrapped
as internal promises — now only objects with the __promise__ marker are treated
as promises, preventing silent data corruption in async map callbacks.
Also guard against stale continuations firing on stack unwinds by checking
caller_frame_depth alongside callback_frame_index.
Tests added for forEach async callbacks, async guard errors on filter/reduce,
and the user-object unwrap fix.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
crates/zapcode-core/src/vm/mod.rs (1)
425-425:⚠️ Potential issue | 🟠 MajorStack underflow should propagate as an error, not default to
Undefined.Using
unwrap_or(Value::Undefined)masks stack-discipline bugs and can silently produce incorrect map/forEach results.Suggested fix
- let callback_result = self.pop().unwrap_or(Value::Undefined); + let callback_result = self.pop()?;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/zapcode-core/src/vm/mod.rs` at line 425, The code currently masks stack underflow by using let callback_result = self.pop().unwrap_or(Value::Undefined);; change this to propagate an error instead: replace unwrap_or with .ok_or(...) or .ok_or_else(...) to return a VM error (e.g. VMError::StackUnderflow) and use the ? operator so the surrounding function returns Result; specifically update the assignment of callback_result to something like let callback_result = self.pop().ok_or(VMError::StackUnderflow)?; and ensure the enclosing function signature and error type support returning that error.
🧹 Nitpick comments (1)
crates/zapcode-core/src/vm/mod.rs (1)
658-665: Consider adding a resource limit check for continuation depth.The
continuationsvector grows without an explicit limit check. While it's implicitly bounded by stack depth, adding a guard would be consistent with other resource-limit enforcement in the VM.Example check
+ // Guard against deeply nested continuations + if self.continuations.len() >= 1024 { + return Err(ZapcodeError::RuntimeError( + "continuation depth limit exceeded".to_string(), + )); + } + self.continuations.push(Continuation::ArrayMap {As per coding guidelines: "Resource limits (memory, time, stack depth, allocation count) must be enforced during execution."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/zapcode-core/src/vm/mod.rs` around lines 658 - 665, Before pushing the new Continuation::ArrayMap onto self.continuations, add a guard that checks the current continuation depth against the VM's configured resource limit (e.g. a max_continuations or continuation_depth limit on self or self.resource_limits); if the limit is reached return an appropriate error/trap (same error type used elsewhere for resource limits) instead of pushing, and only push after incrementing/validating the count. Apply this check at the push site where Continuation::ArrayMap { callback, source: arr, ... } is created so it consistently enforces continuation depth like other VM resource limits.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/zapcode-core/src/vm/mod.rs`:
- Around line 431-448: The block using matches!(map.get("__promise__"),
Some(Value::Bool(true))) and the following else { match map.get("status") { ...
} } is misformatted; run rustfmt (cargo fmt) or reflow the block so it follows
rustfmt rules (e.g., proper brace placement and spacing) — adjust the
conditional/match formatting around the map.get("status") match arm and ensure
removal of any stray spacing so lines with map.get("value"), map.get("reason"),
self.continuations.pop(), and the ZapcodeError::RuntimeError(...) return are
formatted correctly and pass CI.
In `@crates/zapcode-core/tests/async_await.rs`:
- Around line 1010-1016: The failing rustfmt is due to the single-line assertion
formatting around the object check (the
assert!(map.get("__promise__").is_none(), "user object should not have
__promise__"); in the test matching arm for Value::Object); run cargo fmt or
reformat that assertion to the crate's expected multi-line style (split long
assertion and message across lines) so the tests compile with rustfmt, then
commit the formatted change.
---
Duplicate comments:
In `@crates/zapcode-core/src/vm/mod.rs`:
- Line 425: The code currently masks stack underflow by using let
callback_result = self.pop().unwrap_or(Value::Undefined);; change this to
propagate an error instead: replace unwrap_or with .ok_or(...) or
.ok_or_else(...) to return a VM error (e.g. VMError::StackUnderflow) and use the
? operator so the surrounding function returns Result; specifically update the
assignment of callback_result to something like let callback_result =
self.pop().ok_or(VMError::StackUnderflow)?; and ensure the enclosing function
signature and error type support returning that error.
---
Nitpick comments:
In `@crates/zapcode-core/src/vm/mod.rs`:
- Around line 658-665: Before pushing the new Continuation::ArrayMap onto
self.continuations, add a guard that checks the current continuation depth
against the VM's configured resource limit (e.g. a max_continuations or
continuation_depth limit on self or self.resource_limits); if the limit is
reached return an appropriate error/trap (same error type used elsewhere for
resource limits) instead of pushing, and only push after incrementing/validating
the count. Apply this check at the push site where Continuation::ArrayMap {
callback, source: arr, ... } is created so it consistently enforces continuation
depth like other VM resource limits.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a996a37d-b7f3-4e61-9e3b-af064acf3c8a
📒 Files selected for processing (2)
crates/zapcode-core/src/vm/mod.rscrates/zapcode-core/tests/async_await.rs
TheUncharted
changed the title
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
1 participant
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.
Summary
AI-generated code like
Promise.all(arr.map(async fn => await external()))previously crashed the VM because Rust's for-loop couldn't be suspended mid-iteration. Inspired by Node.js and Python asyncio, the VM now has a continuation-based event loop that handles async callbacks in.map()and.forEach()— eachawaitsuspends and resumes sequentially, no threads needed.Changes
Continuationenum to the VM withArrayMapandArrayForEachvariantsprocess_continuation()— fires when a callback frame pops, collects results, sets up next callback or finalizescallback_frame_indexto precisely detect when a callback completes (not just any frame at same depth)Test plan
test_array_map_async_callback_with_external— 3 elements, 3 sequential suspensionstest_array_map_async_empty— empty array returns immediatelytest_array_map_async_single_element— single element edge casetest_array_map_sync_still_works— regression testtest_sequential_external_calls_in_loop— sequential awaits patternasync_map_3benchmark added (11.6 µs median)cargo test)Summary by CodeRabbit
New Features
Documentation
Tests & Examples