Skip to content

knowledge(vllm-omni): 2 change(s) from refine - #429

Merged
tzhouam merged 1 commit into
mainfrom
kb/vllm-omni/vllm-omni-refine-2b2522148089
Oct 26, 2026
Merged

tzhouam merged 1 commit into
mainfrom
kb/vllm-omni/vllm-omni-refine-2b2522148089

Conversation

@tzhouam

@tzhouam tzhouam commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator

Generated automatically by the Copilot knowledge service (changeset vllm-omni-refine-2b2522148089).

op page rule
add repos/vllm-omni/components/configuration/rules-engine-args.md VLLM-OMNI-PR8585-DIFFARGS-FROMCLI
add repos/vllm-omni/components/configuration/rules-compile-hash.md VLLM-OMNI-PR8899-COMPILE-HASH

Generator: 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.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-26T17:41:09.347864Z 4326a14 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@tzhouam
tzhouam merged commit 42c3776 into main Oct 26, 2026
3 checks passed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4326a145eb

ℹ️ 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".

## VLLM-OMNI-PR8899-COMPILE-HASH — torch compile hash 必须排除 per-launch UUID 且模型侧编译要可观测

- 触发:修改会进入 vLLM `ModelConfig` hash factors 的 Omni 字段(`stage_connector_config`、`omni_kv_config` 里的 `connector_config`、含 `wakeup_scope` 等 per-launch `extra` key 的 transfer config),或模型在 vLLM compile wrapper 之外直接调用 `torch.compile`(如 `qwen3_code_predictor.py` 的 `_setup_compile`)。
- 强制:`OmniModelConfig.compute_hash()` 必须在对象浅拷贝上先用 `connector_hash_factors()`(`vllm_omni/distributed/omni_connectors/utils/config.py`,当前排除集合 `PER_LAUNCH_EXTRA_KEYS = {"wakeup_scope"}`)过滤 `stage_connector_config` 与 `omni_kv_config` 中的 `connector_config`(浅拷贝共享内层 dict,替换前先复制该 dict),再委托 upstream `compute_hash()`;模型实际读取的 connector 设置(如 `extra.codec_chunk_frames`)必须继续改变 hash。`torch.compile` 是惰性的:模型侧直接编译要用 `vllm_omni/utils/torch_compile.py` 的 `log_torch_compile_stats` 包裹触发首次编译执行的调用块(本例 `_setup_compile` 内包住 `_warmup_buckets()`),而不是包裹 `torch.compile` 调用本身,以记录该 block 的 Inductor FX graph cache hit/miss/bypass 与耗时。把环境变量纳入 inventory(本例 `TORCHINDUCTOR_CACHE_DIR` 归入 PLATFORM_EXTERNAL)必须同步 `environment_variable_inventory.py` 分类与数量快照。

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Split model-side compile logging into its execution owner

For changes limited to model-side compilation, the new log_torch_compile_stats requirement is stored under the wrong owner: qwen3_code_predictor.py::_setup_compile belongs to model execution, whereas this page and its new index link are confined to configuration. The declared ownership map also assigns vllm_omni/model_executor/ and vllm_omni/utils/ to model-executor. Consequently, an owner-scoped review of the compile/warmup path will not discover this requirement through its owner's rules. Keep hash-factor handling here, but extract the lazy-compilation logging trigger, obligation, and acceptance checks into a model-executor rule page and update that owner's index. This follows the root AGENTS.md requirement to route each conclusion to its nearest code owner rather than combine heterogeneous owners.

AGENTS.md reference: AGENTS.md:L12-L17

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant