Conversation
|
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 (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdded step-waiting and message delivery to workflows: new wait/result types, host APIs (sleep/recv-message/send-message), partition log events/effects, timer handling in effect worker, queueing/resuming logic in partition reducer, SDK recv/sleep helpers, tests, and platform-specific keyring and CI/nix updates. Changes
Sequence Diagram(s)sequenceDiagram
participant Guest as WASM Guest (wflow)
participant SDK as Wflow SDK (WflowCtx)
participant Host as wash_plugin_wflow (Host)
participant Ingress as WflowIngress / PartitionLog
participant Worker as TokioEffectWorker
participant Reducer as Partition Reducer
Guest->>SDK: call sleep(ms) / recv()
SDK->>Host: next_step() -> op_id
alt sleep
SDK->>Host: sleep(job_id, step_id, duration_ms) -> yields WaitStep (Timer)
else recv
SDK->>Host: recv-message(job_id, step_id) -> yields WaitStep (Message)
end
Host->>Ingress: append PartitionEffect WaitTimer/WaitMessage or JobMessage (if pre-buffered)
Worker->>Worker: if WaitTimer store pending_timers (no immediate Run)
Note over Reducer,Ingress: Partition log includes Wait effects / JobMessage
Ingress->>Reducer: handle JobTimerFired or JobMessage entries
Reducer->>Reducer: consume pending_messages or match timer -> produce RunJob effect
Reducer->>Worker: emit RunJob effect
Worker->>Guest: resume job/run attempt (effect worker executes)
Guest->>Host: completes step (StepEffect / Success)
Host->>Ingress: append JobEffectResult (Success)
Reducer->>Reducer: finalize job run state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/wflow_sdk/lib.rs (1)
99-120: Consider returningTdirectly for API consistency.The
effect()method returnsResult<O, JobErrorX>(unwrapped value), whilerecv()returnsResult<Json<T>, JobErrorX>(wrapped inJson). This inconsistency may surprise users. Consider unwrapping likeeffect()does:♻️ Suggested change for consistency
- pub fn recv<T>(&self) -> Result<Json<T>, JobErrorX> + pub fn recv<T>(&self) -> Result<T, JobErrorX> where T: serde::de::DeserializeOwned, { // ... existing code ... - Ok(Json(value)) + Ok(value) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wflow_sdk/lib.rs` around lines 99 - 120, The recv function currently returns Result<Json<T>, JobErrorX> which is inconsistent with effect() that returns Result<O, JobErrorX>; change recv<T>(&self) to return Result<T, JobErrorX>, deserialize value_json into T and return the inner value (not wrapped in Json). Update the function signature and the final Ok(...) to return the deserialized T, keeping existing error mapping (JobErrorX) and using std::any::type_name::<T>() in the error message as before; ensure Json<T> usages (if any callers or types) are adjusted accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/wflow_sdk/lib.rs`:
- Around line 99-120: The recv function currently returns Result<Json<T>,
JobErrorX> which is inconsistent with effect() that returns Result<O,
JobErrorX>; change recv<T>(&self) to return Result<T, JobErrorX>, deserialize
value_json into T and return the inner value (not wrapped in Json). Update the
function signature and the final Ok(...) to return the deserialized T, keeping
existing error mapping (JobErrorX) and using std::any::type_name::<T>() in the
error message as before; ensure Json<T> usages (if any callers or types) are
adjusted accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d28102a4-8140-4486-85e1-615637fbefd3
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lockand included by**/*
📒 Files selected for processing (18)
src/daybook_core/Cargo.tomlsrc/daybook_core/rt.rssrc/daybook_core/secrets.rssrc/test_wflows/lib.rssrc/wash_plugin_wflow/lib.rssrc/wash_plugin_wflow/wit/main.witsrc/wflow/ingress.rssrc/wflow/test.rssrc/wflow/test/recv_message.rssrc/wflow/test/sleep_then_succeed.rssrc/wflow_core/partition/effects.rssrc/wflow_core/partition/job_events.rssrc/wflow_core/partition/log.rssrc/wflow_core/partition/reduce.rssrc/wflow_core/partition/state.rssrc/wflow_sdk/lib.rssrc/wflow_tokio/partition/effect_worker.rssrc/wflow_tokio/partition/reducer.rs
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores