Fix CHAT_TEMPLATE_KWARGS_ON default quoting in dsv4_fp4_b300_vllm#1695
Conversation
…edbench
Bash ends `${VAR:-{...}}` parameter expansion at the first `}`, so the
trailing brace was appended as a literal — yielding `{...}}` when the
workflow exported a non-default value, breaking the thinking-on cells.
Move the JSON default into its own variable so the expansion contains
no brace literals.
|
Thanks for the contribution! For vLLM & SGLang, please ensure that your recipes is similar to the official vLLM recipes and/or the SGLang cookbook If it is not, please create a PR first before we can merge your single node PR into the master branch. Let's ensure that the documentation is first class such that the entire ML community can benefit from your hard work! Thank you 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. If re-running failed jobs is attempted, PR authors are responsible for ensuring it passes. See GitHub's docs on re-running failed jobs: https://docs.github.com/en/actions/how-tos/manage-workflow-runs/re-run-workflows-and-jobs#re-running-failed-jobs-in-a-workflow As a rule of thumb, generally, PR authors should request a review & get a PR approval from the respective companies' CODEOWNERS before requesting a review from core maintainers. If additional help is needed, PR authors can reach out to core maintainers over Slack. |
|
@functionstackx @cquil11 @Oseltamivir Here are two runs the collected cleanly: I think we can merge to main. |
|
Note on the workflow: by default just leave open-pr unchecked and grab the AL matrix from the run artifacts. If you want it to auto-open a PR updating the reference yaml, you can check open-pr button in workflow but that just requires the REPO_PAT secret to be from an account with write access to the repo. |
3 participants
The collector set
CHAT_TEMPLATE_KWARGS_ON="${VAR:-{...}}", but bash ends the parameter expansion at the first }, so the trailing } is appended as a literal. It worked locally because the var was unset (default path lands on a single brace), in CI the workflow passes the var in, so the value became {...}} invalid JSON. Fixed by moving the default into its own variable so there are no brace-literals in the expansion.Parent PR: #1685 and #1650
Note
Low Risk
Single-line bash default-value fix in a benchmark script; no production runtime or security impact.
Overview
Fixes how the SPEED-Bench AL collector sets the default thinking-on
chat_template_kwargsJSON indsv4_fp4_b300_vllm.sh.The default is now stored in
DEFAULT_CHAT_TEMPLATE_KWARGS_ONand assigned viaCHAT_TEMPLATE_KWARGS_ON="${CHAT_TEMPLATE_KWARGS_ON:-$DEFAULT_CHAT_TEMPLATE_KWARGS_ON}", avoiding a:-{...}expansion that bash truncates at the first}when CI sets the variable—previously producing invalid JSON with an extra trailing}for--chat-template-kwargs.Reviewed by Cursor Bugbot for commit 1ab04ad. Bugbot is set up for automated code reviews on this repo. Configure here.