Update docs and API#12
Conversation
…nt object stream in sub_interpreter
…tream concurrency safety info
Reviewer's GuideMakes SuspendObjectStream shareable and documented as concurrency-safe via the CLCA pattern, updates fork_interpreter to reuse the parent object_io by default, removes a deprecated pointer-advancing alias, and adds English/Chinese documentation pages and navigation entries for the CLCA design pattern and updated runtime semantics. Sequence diagram for fork_interpreter shared SuspendObjectStream behaviorsequenceDiagram
participant Parent as WorkflowInterpreter_parent
participant Child as WorkflowInterpreter_child
participant Stream as SuspendObjectStream
Parent->>Parent: fork_interpreter(compose, middleware, object_io=None)
alt object_io is None
Parent->>Stream: use Parent.object_io
Parent->>Child: __init__(compose, object_io=Parent.object_io, exception_ignored=Parent._exc_ignored, middleware=mdw, extra_args=Parent._ava_args[1:])
else object_io is provided
Parent->>Child: __init__(compose, object_io=object_io, exception_ignored=Parent._exc_ignored, middleware=mdw, extra_args=Parent._ava_args[1:])
end
Note over Parent,Child: Parent and child now share the same SuspendObjectStream when object_io is None
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Deploying amritasense with
|
| Latest commit: |
c87b042
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c2947ccf.amritasense.pages.dev |
| Branch Preview URL: | https://chore-ref.amritasense.pages.dev |
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In the
CheckpointSignalexample,resume()useswith self._lock:on anaiologic.Lock, which is an async lock; this should be madeasync def resume(...)with anasync with self._lock:(or otherwise use the correct locking API) so the sample code is runnable and consistent with theSignalexample.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the `CheckpointSignal` example, `resume()` uses `with self._lock:` on an `aiologic.Lock`, which is an async lock; this should be made `async def resume(...)` with an `async with self._lock:` (or otherwise use the correct locking API) so the sample code is runnable and consistent with the `Signal` example.
## Individual Comments
### Comment 1
<location path="src/amrita_sense/runtime/workflow.py" line_range="271" />
<code_context>
middleware (Callable[[WorkflowInterpreter], Awaitable[Any]] | None | object): The middleware to be used for the sub-interpreter.
If UNSET, it will use the same middleware as the parent. If None, it will not use any middleware.
- object_io (io_T | None): The object I/O stream for the sub-interpreter. If None, it will create a new SuspendObjectStream.
+ object_io (io_T | None): The object I/O stream for the sub-interpreter. If None, it will be set to a shared object stream.
Returns:
</code_context>
<issue_to_address>
**issue:** Docstring description of `object_io=None` behavior may not match actual implementation and supported types.
Given the implementation `object_io or self.object_io`:
- If `self.object_io` is not a shared `SuspendObjectStream` (or otherwise safely shareable), it will still be reused.
- If both `object_io` and `self.object_io` are `None`, nothing is allocated.
To avoid misleading callers, the docstring should clarify that the parent’s `object_io` is reused, and that safe sharing is only guaranteed for `SuspendObjectStream` (or an explicitly supported subset), rather than implying a new shared stream is always set up.
</issue_to_address>
### Comment 2
<location path="docs/zh/guide/practice/clca-design-pattern.md" line_range="133" />
<code_context>
+- **一次唤醒全部**:`signal()` 完成共享 `Future`,触发所有已注册的回调,全部等待者同时被唤醒。
+- **跨循环安全**:`aiologic.Lock` 保护所有共享状态的读写,每个协程最终 `await` 的是自己所在事件循环内的 `Future`,天然避免了跨循环等待问题。
+
+## 实践二:CheckpointSignal —— 外设式检查点挂起
+
+时序图:
</code_context>
<issue_to_address>
**issue (typo):** “外设式”看起来像是笔误,建议改为“外部式”以与上下文和英文版保持一致。
前文写的是“当控制权在外部时”,英文小节标题也是“External Checkpoint Suspend”。“外设式”一般指硬件外设,语义不符,建议改为“外部式检查点挂起”以避免歧义。
```suggestion
## 实践二:CheckpointSignal —— 外部式检查点挂起
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
|
As I see it, |
Summary by Sourcery
Document and enable safe sharing of SuspendObjectStream across interpreters using the CLCA concurrency pattern, and clean up deprecated runtime API.
Enhancements:
Documentation: