Repository navigation
knowledge(vllm-omni): 1 change(s) from refine - #367
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2c74e3bd0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| # Model Executor — Orchestration Loop Rules | ||
|
|
||
| ## VLLM-OMNI-PR8675-ORCH-LOOP-DEFAULT — 双 orchestration loop 必须共用同一 StagePool 读路径，成员变更唤醒 dispatcher |
There was a problem hiding this comment.
This creates a second owner for an existing contract: components/serving/_index.md:37 already routes event-driven orchestration to rules-engine-lifecycle.md:177-182, while configuration/environment-variable-contract.md:22 also documents this flag. Those retained pages still prescribe the legacy default, direct reader call, periodic reconciliation, and flag-controlled serving drain, directly contradicting this new page. Because normal routing reaches those existing pages rather than this model-executor page, reviewers will continue enforcing the obsolete behavior; update/retire the existing serving and configuration conclusions union-first instead of adding a parallel rule.
AGENTS.md reference: AGENTS.md:L11-L17
Useful? React with 👍 / 👎.
| - 禁止：重新引入 per-pipeline 默认管道（`_event_driven_orch_default_for_pipeline` 或 `event_driven_orch_default` ctor 参数）；在任一 loop 里绕过 pool 读 helper 直接 `client.get_output_async()`；用手工挑选的 parity 清单替代把 orchestrator scenario 测试在两条 loop 上参数化；把成员变更唤醒表述为保证即时 reconcile 的同步语义；把默认切换表述为吞吐/延迟改善——其证据边界是空闲 CPU 下降，吞吐与延迟预期不变。 | ||
| - 验收：单测在 env 未设置与 `VLLM_OMNI_EVENT_DRIVEN_ORCH=0` 两种条件下运行；`tests/engine/test_orchestrator.py` 与 `test_orchestrator_error_handling.py` 通过参数化 fixture 覆盖两条 loop；新增读取或成员路径时断言 pooling payload rehydration 在两条 loop 行为一致，且成员变更后 reader 集合最终与 pool 成员一致（允许等待在途 output-routing 完成，不要求瞬时）；启动日志仍记录所选 loop 模式与 reader/poller 计数。^[PR #8675] | ||
|
|
||
| <!-- kb:rule status=active since=v0.30.0 --> |
There was a problem hiding this comment.
At this commit's timestamp, upstream PR #8675 is still open and describes commit d4157eb on a vLLM 0.31.0 base, so the all-pipeline default and new StagePool.read_llm_raw_output contract are not v0.30.0 behavior. Marking the rule active since=v0.30.0 exposes proposed branch behavior as a current released invariant, causing reviews of main/v0.30 to demand APIs and defaults that are absent there; keep it inactive until merge or bind activation to the actual merged SHA/release.
AGENTS.md reference: knowledge/AGENTS.md:L16-L16
Useful? React with 👍 / 👎.
Generated automatically by the Copilot knowledge service (changeset
vllm-omni-refine-542884d4acca).repos/vllm-omni/components/model-executor/rules-orchestration-loop.mdGenerator:
zcode:GLM-5.3· judge:codex:gpt-6-sol:medium.The publisher merges it only if its local gate passes on the exact merge result.