Revert #2127: contains patches / 回滚 #2127：包含补丁

[adibarra]

Reverts #2127 because it contains inference-engine patches — it live-patches SGLang's disaggregation/prefill.py (resolve_waiting_queue_bootstrap) at setup time, which #2155 now forbids without a docs/waiver/ entry.

Full revert of squash commit 0bd3fa41 (18 files, −4140). No conflicts.

中文说明

回滚 #2127，因其包含推理引擎补丁——在 setup 阶段对 SGLang 的 disaggregation/prefill.py（resolve_waiting_queue_bootstrap）进行运行时修改，违反 #2155（未经 docs/waiver/ 豁免不得修改推理引擎）。完整回滚 squash 提交 0bd3fa41（18 个文件，−4140），无冲突。

🤖 Generated with Claude Code

[github-actions]

Thanks for the contribution! Please reach out to respective companies' CODEOWNER to fill in the latest PR_REVIEW_CHECKLIST.md before pinging core maintainer on Slack for review. In order for the signoff PR check bot to trigger, you must follow the PR_REVIEW_CHECKLIST.md template correctly, including the phrase As a PR reviewer and CODEOWNER, I have reviewed this and have.

For PR verification, add the full-sweep-fail-fast label (strongly recommended) to this PR — the benchmark sweep only runs on labeled PRs. Use full-sweep-enabled only if you need matrix jobs to keep running past a failure.

PR authors are responsible for ensuring that after merging, all GitHub Action jobs fully pass. A lot of the time, failures are just flakes and simply re-running the failed jobs will fix it. See GitHub's docs on re-running failed jobs

---

感谢你的贡献！请联系相应公司的 CODEOWNER 填写最新的 PR_REVIEW_CHECKLIST.md，然后再在 Slack 上联系核心维护者进行审阅。为了触发 signoff PR 检查机器人，你必须正确遵循 PR_REVIEW_CHECKLIST.md 模板，包括保留英文语句 As a PR reviewer and CODEOWNER, I have reviewed this and have。

如需进行 PR 验证，请为此 PR 添加 full-sweep-fail-fast 标签（强烈推荐）— 基准测试 sweep 仅在带有标签的 PR 上运行。仅当需要矩阵任务在失败后继续运行时才使用 full-sweep-enabled。

PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动（flake），重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档

[claude]

🔴 This revert deletes the perf-changelog.yaml entry for #2127 (config-key dsv4-fp4-mi355x-sglang-disagg-agentic-hicache, 6 non-blank lines), but the changelog is append-only by design and CI will block the PR. Both utils/validate_perf_changelog.py:169-172 (ChangelogValidationError when len(head_entries) < len(base_entries)) and utils/process_changelog.py:28-39 (ValueError on any --prefixed diff line with non-whitespace content) fail on this diff, and both are invoked in .github/workflows/run-sweep.yml (check-changelog and setup jobs) which are triggered by the PR's edit to perf-changelog.yaml. Fix: instead of deleting the #2127 entry, APPEND a new entry (referencing PR #2164) describing the revert.

Extended reasoning...

What the bug is

This PR is a full revert of #2127. Alongside reverting the recipe / launcher / aggregation code, the diff also deletes the perf-changelog.yaml entry that #2127 added (the - config-keys: [dsv4-fp4-mi355x-sglang-disagg-agentic-hicache] block plus its five bullet/pr-link lines — 6 non-blank content lines total). But perf-changelog.yaml is enforced as append-only, so this deletion will fail CI unconditionally.

The specific code path that triggers it

Two validators in .github/workflows/run-sweep.yml guard the changelog:

1. utils/validate_perf_changelog.py:169-172 (compare_entries) — parses the YAML at base_ref (origin/main, which contains the [AMD][AgentX] DeepSeek V4 multi-node Agentic framework and recipe / DeepSeek V4 多节点 Agentic 框架与配方 #2127 entry per commit 0bd3fa4) and at head_ref (this PR, which removes it) and raises ChangelogValidationError("perf-changelog.yaml entries were deleted") when len(head_entries) < len(base_entries). This runs in the check-changelog job.

2. utils/process_changelog.py:28-39 (get_added_lines) — walks the raw git diff and raises ValueError(f"Deletions are not allowed in {filepath}. Only additions to the changelog are permitted. Found deleted line: {deleted_content}") on any line starting with - (not ---) whose stripped content is non-empty. This runs in the setup job.

Both jobs are gated on paths: [perf-changelog.yaml] in the workflow trigger, and the PR modifies that file, so both fire.

Step-by-step proof (reproduced locally)

1. origin/main currently contains the [AMD][AgentX] DeepSeek V4 multi-node Agentic framework and recipe / DeepSeek V4 多节点 Agentic 框架与配方 #2127 entry (commit 0bd3fa4 — merged before this revert commit 6927574, verified via git log).
2. HEAD (this revert) removes those 6 non-blank lines from perf-changelog.yaml (verified via the PR diff above).
3. Running python3 utils/validate_perf_changelog.py --changelog-file perf-changelog.yaml --base-ref 0bd3fa4 --head-ref HEAD produces:

   ValueError: Deletions are not allowed in perf-changelog.yaml. Only additions to the changelog are permitted. Found deleted line: - config-keys:
   

   validate_perf_changelog.py invokes process_changelog.py internally, so the first --prefixed content line raises before it even reaches the length check.
4. check-changelog fails at its very first step → sweep never runs → PR cannot merge without an admin override.

Why existing code does not prevent this

The append-only rule is intentional and enforced by design: validate_perf_changelog.py has extensive raw-byte and ordering checks in validate_raw_change so historical entries reflect state at merge time and cannot be rewritten. There is no escape hatch for reverts.

Impact

• check-changelog job fails immediately on this PR.
• setup job would also fail before dispatching any matrix.
• The entire sweep pipeline is blocked, which in turn blocks merge (and the label-gated benchmarks/evals cannot run).

How to fix

Instead of deleting the #2127 changelog entry, append a new entry at the end of perf-changelog.yaml describing the revert, with pr-link pointing at PR #2164, e.g.:

- config-keys:
    - dsv4-fp4-mi355x-sglang-disagg-agentic-hicache
  description:
    - "Revert #2127: recipe contained inference-engine patches to SGLang's disaggregation/prefill.py, which #2155 forbids without a docs/waiver/ entry. Removes the dsv4-fp4-mi355x-sglang-disagg-agentic-hicache config-key added in #2127."\n  pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2164\n```\n\nRestore the original #2127 lines (currently at 4720-4725) and add the revert entry as a new append at the bottom. The historical entry stays intact; the new entry documents the revert.

</details>

[claude]

🟡 The revert restores a known-broken conc-list format at run-sweep.yml:539/554 for the sweep-multi-node-agentic job that #2127 had explicitly fixed. matrix.config.conc is a list for multi_node agentic entries (utils/matrix_logic/generate_sweep_configs.py:670 assigns Fields.CONC.value: conc_batch), so conc-list: '[${{ matrix.config.conc }}]' produces malformed JSON that fails downstream fromJson in benchmark-multinode-tmpl.yml:154 — the sister workflow e2e-tests.yml:227/242 already uses the correct toJson(matrix.config.conc) + matrix.config.conc[0] pattern. Currently dormant (no perf-changelog entry references a multi_node agentic config), so this doesn't block the revert, but consider preserving the two-line workflow fix out of the revert scope since it's orthogonal to the #2155 engine-patch policy motivating the revert.

Extended reasoning...

What the bug is

PR #2127 (commit 0bd3fa4) bundled a workflow fix alongside the AMD agentic recipe: it changed .github/workflows/run-sweep.yml:539 from conc-list: '[${{ matrix.config.conc }}]' → conc-list: ${{ toJson(matrix.config.conc) }} and removed the paired conc: ${{ matrix.config.conc }} at line 554. This revert restores those two broken lines verbatim.

Why the restored pattern is broken

1. matrix.config.conc is a list for multi_node agentic entries. In utils/matrix_logic/generate_sweep_configs.py:670, Fields.CONC.value is set to conc_batch, which comes from chunk_multinode_agentic_concurrencies() (lines 128-131) — this function slices conc_values into sublists and returns list[list[int]]. So every matrix element gets .conc as a JSON array like [32] or [4096].

2. The downstream template requires a valid JSON array string. .github/workflows/benchmark-multinode-tmpl.yml:154 does CONC_LIST: ${{ join(fromJson(inputs.conc-list), ' ') }} — this needs inputs.conc-list to be parsable JSON of a flat array of scalars. Interpolating a list expression inside literal brackets ('[${{ [32] }}]') either produces nested '[[32]]' or GitHub Actions' Array stringification — neither works with fromJson.

3. The sister workflow already has the correct pattern. .github/workflows/e2e-tests.yml:227 uses conc-list: ${{ toJson(matrix.config.conc) }} and line 242 uses conc: ${{ matrix.config.conc[0] }} (extracting the scalar via [0]) for the identical input shape. Post-revert, run-sweep.yml and e2e-tests.yml are inconsistent for the same matrix shape.

Step-by-step proof of the failure mode

Take dsv4-fp4-gb300-dynamo-vllm-agentic (defined in configs/nvidia-master.yaml:12520 with multinode: true and agentic-coding scenario) and imagine a future changelog entry referencing it:

1. generate_sweep_configs.py calls chunk_multinode_agentic_concurrencies([32, 64]) → [[32], [64]].
2. Each matrix element ends up with config.conc = [32] or [64] (a list).
3. sweep-multi-node-agentic job evaluates '[${{ matrix.config.conc }}]' — GHA stringifies the array, yielding malformed content (nested '[[32]]' or the literal token Array).
4. benchmark-multinode-tmpl.yml:154 runs fromJson(inputs.conc-list) on the malformed string — parse error, job fails immediately.

Why this is dormant on this PR

The revert also removes dsv4-fp4-mi355x-sglang-disagg-agentic-hicache from perf-changelog.yaml and configs/amd-master.yaml, which was the only recent changelog entry exercising the sweep-multi-node-agentic job. grep confirms no other perf-changelog entry currently references any multi_node agentic config, so the broken code path is not triggered by this PR's own CI.

However, configs/nvidia-master.yaml still defines multi_node agentic configs (dsv4-fp4-gb300-dynamo-vllm-agentic line 12520, dsv4-fp4-gb200-dynamo-vllm-agentic-3p2d-tep8-tp8 line 14119). The next PR that adds any of these to perf-changelog.yaml will silently hit the broken fromJson path.

Fix

Preserve the two-line workflow fix out of the revert scope. The full delta is trivial and orthogonal to the inference-engine-patch policy (#2155) motivating this revert:

• Line 539: conc-list: '[${{ matrix.config.conc }}]' → conc-list: ${{ toJson(matrix.config.conc) }}
• Line 554: delete the conc: ${{ matrix.config.conc }} line (the multi-node template doesn't consume a scalar conc input for agentic runs when conc-list is provided).

Severity: nit

Dormant on this PR — nothing currently drives the broken code path. But the regression undoes a validated bug fix that was bundled into #2127, and the fix is a 2-line diff completely unrelated to the engine-patch policy scope of this revert. Worth surfacing so the author can decide whether to preserve the workflow fix or file a fast follow-up.

[claude]

🟣 This revert removes the assignment flat["server_external_cache_hit_rate"] = flat["server_cpu_cache_hit_rate"] from utils/agentic/aggregation/backends/sglang.py, but the nested-output read of that flat key at line ~115 remains, so the sglang agentic agg JSON silently emits server_metrics.cache.external_cache_hit_rate: null even when HiCache host hits are non-zero. Pre-existing — the read existed before #2127 (which briefly added the assignment), and this clean revert simply restores that state; leaving as a nit/pre-existing follow-up so the strict revert stays strict.

Extended reasoning...

What the code does now

After the revert, SglangBackend.populate() in utils/agentic/aggregation/backends/sglang.py computes GPU/CPU/overall cache hit rates from sglang:cached_tokens{cache_source=...}, but never assigns flat["server_external_cache_hit_rate"]. A few lines later, in the same method, it still copies that key into nested["cache"]["external_cache_hit_rate"]:

nested["cache"].update({
    ...
    "external_cache_hit_rate": flat["server_external_cache_hit_rate"],
    ...
})

Because base.py:12 pre-initializes SERVER_CACHE_FLAT_FIELDS["server_external_cache_hit_rate"] = None, there is no KeyError — the read just resolves to None and the emitted agg JSON silently records external_cache_hit_rate: null. Sister backend vllm.py (vllm.py:67,86) does set this field from external-prefix metrics, so the two backends disagree on the field's semantics.

Step-by-step proof (qwen3.5-fp8-mi355x-sglang-agentic-hicache, kv-offloading: dram + kv-offload-backend: hicache)

1. HiCache promotes host hits, so sglang exports non-zero sglang:cached_tokens{cache_source="host"}.
2. SglangBackend.populate() reads cached_by_source["host"] and sets flat["server_cpu_cache_hit_rate"] = rate(host_hits, prompt_total) — non-null.
3. Post-revert, no line sets flat["server_external_cache_hit_rate"], so it retains the None sentinel from SERVER_CACHE_FLAT_FIELDS (base.py:12).
4. The nested-output update copies that None into nested["cache"]["external_cache_hit_rate"].
5. round_floats(...) returns None unchanged; json.dump writes "external_cache_hit_rate": null.
6. vLLM in the same conditions would have written a real hit rate at that key.

Why this is pre-existing (addressing refutations)

The dead read is not new. git show ed15750:utils/agentic/aggregation/backends/sglang.py (the sglang backend's introduction in #2058, Jul 4, 2026) already had the identical unassigned read of flat["server_external_cache_hit_rate"] in the nested update. #2127 added the two-line assignment as a fix. This revert removes that fix as part of a full, byte-for-byte revert of squash commit 0bd3fa41 — the state it restores is exactly what shipped on main for ~6 days between #2127's merge and this revert.

The PR's motivation is the #2155 policy prohibiting in-tree inference-engine patches without a docs/waiver/ entry, which #2127 violated by live-patching SGLang's disaggregation/prefill.py at setup time. A clean revert must not carry unrelated fixes, or it defeats the purpose of the policy revert. The two possible follow-up fixes (restore the assignment, or drop the dead nested-output key) both introduce diffs outside the revert set and are more appropriate as a separate PR.

Impact

• No crash, no data corruption — silent null in one nested field of the aggregated agentic JSON.
• The equivalent numeric value is still surfaced via server_metrics.cache.cpu_cache_hit_rate, so downstream consumers can compute host-hit rate from that field.
• The still-active qwen3.5-fp8-mi355x-sglang-agentic-hicache recipe in configs/amd-master.yaml exercises this path (its search space includes a kv-offloading: dram + kv-offload-backend: hicache row), so this is not fully dormant despite the removed dsv4-fp4-mi355x-sglang-disagg-agentic-hicache recipe. The test test_process_agentic_result.py::test_processor_normalizes_sglang_server_metrics correctly drops its external_cache_hit_rate assertion in this PR, matching the reverted behavior.

Suggested fix (follow-up PR, not this one)

Either re-add the two-line assignment in a standalone cleanup PR (keeps HiCache host hits mirrored as external, aligning with vllm), or drop the dead external_cache_hit_rate key from the sglang nested output. Both are trivial and unblocked by this revert.