[Klaud Cold] minimaxm3-fp4-b300-vllm-agentic-mtp: long-prefill-token-threshold 512 / 设置 long-prefill-token-threshold 512 - #2539
Conversation
|
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 For PR verification, add the 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 模板，包括保留英文语句 如需进行 PR 验证，请为此 PR 添加 PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动（flake），重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档 |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — trivial one-line addition of a vLLM server flag, consistent with existing usage of the same flag in dsv4_fp4_b300_vllm_mtp.sh.
What was reviewed: the single line addition of --long-prefill-token-threshold 512 to the VLLM_CMD array, and its placement/effect within the array. The only finding is a cosmetic stray trailing backslash (harmless — bash array elements are still newline/whitespace delimited) noted inline.
Extended reasoning...
Overview
This PR adds a single vLLM server flag, --long-prefill-token-threshold 512, to the VLLM_CMD array in benchmarks/single_node/agentic/minimaxm3_fp4_b300_mtp.sh. It is a one-line diff touching only this benchmark launch script; no other files are changed.
Security risks
None. This is a benchmark configuration script that shells out to vllm serve with a fixed set of CLI flags. There is no user input, auth, or data handling involved in this change.
Level of scrutiny
Low scrutiny is appropriate here. This is a config-only change to a single-node benchmark recipe, following an established pattern already used for the same flag in dsv4_fp4_b300_vllm_mtp.sh. The change is mechanical and self-contained.
Other factors
The only issue surfaced by the bug hunting system is a stray trailing backslash on the new line, which is cosmetic/harmless since bash array elements are whitespace/newline-delimited regardless (confirmed by multiple independent verifiers). This does not affect the actual server command and is not blocking. No outstanding reviewer comments exist on this PR, and no prior review from this account is present in the timeline.
| --max-cudagraph-capture-size 512 | ||
| --max-num-batched-tokens 16384 | ||
| --stream-interval 20 | ||
| --long-prefill-token-threshold 512 \ |
There was a problem hiding this comment.
🟡 Line 105 has a stray trailing backslash inside the VLLM_CMD array literal after --long-prefill-token-threshold 512. It's harmless since bash array elements are already newline-separated (the surviving whitespace still splits 512 and --trust-remote-code into separate words), but it's an unneeded copy-paste artifact inconsistent with the rest of the array — remove it for consistency.
Extended reasoning...
What the bug is
Line 105 of minimaxm3_fp4_b300_mtp.sh adds a new array element to VLLM_CMD:
--long-prefill-token-threshold 512 \
--trust-remote-codeThe trailing backslash at the end of the new line is a line-continuation character. It has no functional purpose here because VLLM_CMD is a bash array literal — array elements are already delimited by newlines/whitespace, not by continuation. None of the other ~20 elements in this array (lines 89-108) use a trailing backslash.
Why it appears harmless (and the proof)
Backslash-newline in bash is spliced out, effectively joining line 105 and 106 into one logical line before word-splitting occurs. But the space before the backslash and the leading whitespace of line 106 both survive the splice, so the tokenizer still sees two distinct words:
$ arr=( a 20 b 512 \
c )
$ declare -p arr
declare -a arr=([0]="a" [1]="20" [2]="b" [3]="512" [4]="c")512 and --trust-remote-code/c remain separate array elements either way. So the actual vllm serve invocation (verified via the script's own printf '%q ' "${VLLM_CMD[@]}" dump) is unaffected — this is not a functional bug.
Why it looks like a mistake anyway
The file does legitimately use backslash-continuation elsewhere — e.g. the printf calls at lines 42, 62, and 66-67 span multiple lines for readability. The stray backslash on line 105 reads as if it were copy-pasted from one of those printf forms into the array context, where it serves no purpose and is simply confusing to a future reader who might assume it changes parsing behavior.
Fix
Drop the trailing backslash so line 105 reads:
--long-prefill-token-threshold 512matching the style of every other element in the VLLM_CMD array.
Verifier notes
All three independent verifiers reproduced the harmlessness claim with the same arr=(...) experiment and confirmed no other array element uses continuation. No refutations were raised. This is purely a cosmetic/consistency nit — it does not affect the server command, does not block merge, and should be graded nit per the [quality] classification.
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31297582527 |
|
/stage-results 31297582527 |
|
@xinli-sw staged run 31297582527: https://inferencemax-app-git-staging-semianalysisai.vercel.app/inference?i_dates=2026-08-09~r31297582527 This run remains available across future @xinli-sw 已将运行 31297582527 发布到预发布环境：https://inferencemax-app-git-staging-semianalysisai.vercel.app/inference?i_dates=2026-08-09~r31297582527 后续的 |
1 participant
Add
--long-prefill-token-threshold 512tominimaxm3_fp4_b300_mtp.sh.中文说明
在
minimaxm3_fp4_b300_mtp.sh中添加--long-prefill-token-threshold 512。