Skip to content

chore: sync text generation launcher with vLLM 0.26 - #524

Merged
cswaney merged 1 commit into
mainfrom
515-cleanup-text-generation-launcher
Aug 28, 2026
Merged

chore: sync text generation launcher with vLLM 0.26#524
cswaney merged 1 commit into
mainfrom
515-cleanup-text-generation-launcher

Conversation

@cswaney

@cswaney cswaney commented Aug 28, 2026

Copy link
Copy Markdown
Member

Summary

Two narrow fixes surfaced by the vLLM v0.26 audit.

Migrate `--model` to a positional argument. v0.26 emits a deprecation warning on every service start:

With `vllm serve`, you should provide the model as a positional argument or in a config file instead of via the `--model` option. The `--model` option will be removed in a future version.

Migrating now avoids a hard break on the next vLLM bump. Applies to `text_generation_slurm.sh` and both branches of `text_generation_local.sh` (docker + apptainer).

Remove the Disable Custom Kernels toggle. It has been a phantom option for a while: the toggle sets container state and appears in the request body, but neither the active `text_generation_*.sh` templates nor the backend translate it into a vLLM flag — only the `templates/deprecated/` copies mention `--disable-custom-kernels`. The toggle told users they were changing behavior; they were not. Cleans up UI, initial state, propTypes, tests, and dev fixtures.

Test plan

  • `uv run just lint`
  • `uv run just test` (969 pass)
  • `npm run lint`
  • `npm test` (558 pass)
  • Manual: launch a text-generation service on Della, confirm the vLLM deprecation warning is gone from the container log.

Notes

Closes #515

Two narrow fixes surfaced by the vLLM v0.26 audit (#515):

Pass the model reference as a positional argument to `vllm serve` rather
than `--model`. v0.26 warns on every service start: "With `vllm serve`,
you should provide the model as a positional argument or in a config file
instead of via the `--model` option. The `--model` option will be removed
in a future version." Migrating now avoids a hard break on the next
vLLM bump. Applies to `text_generation_slurm.sh` and both branches of
`text_generation_local.sh`.

Remove the Disable Custom Kernels toggle. It has been a phantom option
for a while: the toggle sets container state and passes through the API
payload, but neither the active `text_generation_*.sh` templates nor
the backend translate it into a vLLM flag (only the deprecated templates
mention `--disable-custom-kernels`). The toggle told users they were
changing behavior; they were not. Cleans up UI, initial state, propTypes,
tests, and dev fixtures.

Closes #515
@claude

claude Bot commented Aug 28, 2026

Copy link
Copy Markdown

Review

Small, well-scoped PR — both changes do exactly what the description says, and I verified there's no leftover state:

  • No remaining disable_custom_kernels references outside templates/deprecated/ (checked Python backend services/base.py/asgi.py, and the frontend requests.js) — the field was truly dead everywhere except the deprecated templates, matching the PR's claim.
  • buildContainerConfig in web/src/lib/requests.js is a generic passthrough, so dropping the field from the two default-options objects (text-generation.jsx, SidebarContainer propTypes) fully removes it from the request body — no orphaned reference.
  • Positional-arg migration is applied consistently across all three templates (text_generation_local.sh docker + apptainer branches, text_generation_slurm.sh), with the positional model path placed before the flags in each, which matches vLLM's documented vllm serve MODEL [options] form.

Minor/non-blocking observations:

  • --revision {{ container_config.revision }} is still passed as a flag alongside the positional /data/snapshots/{{ revision }} path — that duplication predates this PR, but since you're already touching every line around it, it might be worth a follow-up to confirm it's still needed now that the model path itself is revision-scoped.
  • The "Manual: launch on Della" checkbox in the test plan is unchecked — worth confirming before merge that the deprecation warning is actually gone in a real container log, since that's the entire motivation for the --model change and templates aren't covered by the Python/JS test suites.
  • lib/tests/dev/test_text_generation.py isn't part of the automated suite (manual requests-based smoke script), so the 969 pass / 558 pass counts don't actually exercise these template changes — the manual Della check is the only real verification here, reinforcing the point above.

No security, performance, or test-coverage concerns beyond the manual-verification gap noted above. Good cleanup.

@cswaney
cswaney merged commit e352059 into main Aug 28, 2026
10 checks passed
@cswaney
cswaney deleted the 515-cleanup-text-generation-launcher branch August 28, 2026 09:09
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.

chore(web): sync text generation launcher params with the bumped vLLM version

1 participant