Repository navigation
fix(setup): repair the setup.sh -> switch.sh journey for gemma-4-26b-a4b - #1312
Merged
Merged
Conversation
… tier
`bash scripts/setup.sh gemma-4-26b-a4b` downloaded 16 GB that no shipped slug
can serve, and skipped the 17 GB that both of them need.
The profile carried two defaults that disagreed: `default_weight_variant: awq`
(what the slug side resolves) against `setup: primary: autoround-int4-mixed`
(what setup.sh downloads). `setup.primary` is a deliberate override — weights.py
resolves `setup.get("primary") or default_weight_variant` — but this particular
override pointed at a variant whose composes had been moved to
`vllm/compose/_archive/`. No entry in registry.yaml references it; both
registered slugs (vllm/gemma-26ba4b-single, vllm/gemma-26ba4b-dual) pin
`weights_variant: awq`. So the obvious path — setup, then launch the model's own
slug — could not work, and failed 16 GB later.
Drop the override so setup falls back to the profile default (awq). The
autoround-int4-mixed weights entry itself stays registered: kv-calc and the
arch_patches / profile_runtime evidence rows still name it.
Gate the general form in test-setup-primary-slug-backed.sh: every model's
resolved `setup.primary` must name a variant some registry.yaml entry actually
serves. It is deliberately NOT "primary == default_weight_variant" — two models
override it correctly (qwen3.8-27b -> fp8, deepseek-v4-flash-0731 ->
unsloth-iq2-xxs) and equality would red both. It is also status-agnostic: a
deprecated entry still pins its variant, and deprecated != deleted here. What it
rejects is a variant that survives only under _archive/, which is exactly how
this drifted.
Verified: the gate fails on the unfixed tree naming gemma-4-26b-a4b and nothing
else, and passes after. `SETUP_DUMP_KEYS=1` now resolves
`primary=gemma-4-26b-a4b:awq`. Not boot-tested — no weights for this model here.
Fixes #1303
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vllm/gemma-26ba4b-single defaults to SPEC_N=4 and needs an external MTP drafter that setup.sh fetches only under WITH_ASSISTANT_DRAFT=1 — a flag that existed in the model profile, in setup.sh's implementation and in a test, and in no user-facing surface at all, while its siblings WITH_DFLASH_DRAFT and WITH_VISION were at least in the file's header comment. A user who did not set it got a bare missing-config.json out of vLLM naming neither the cause nor either fix, and reasonably concluded the download should have been automatic. There was already a working escape hatch — SPEC_N=0, standardised across every vLLM compose by #1048 — and nothing in the failure path mentioned it either. Three surfaces, working outward from the failure: - The compose refuses to boot when SPEC_N>0 and the drafter directory has no config.json, and its message names BOTH routes: re-run setup with the flag, or boot drafter-less with SPEC_N=0. Same fail-loud contract the deepseek moecache composes carry for their external GGUF (#1054). The header's Drafter line now says how to obtain the file, which it previously described without sourcing. - `setup.sh --help` grows an "Optional extra downloads" section. The header comment was never the right surface — reading it means already having opened the script. WITH_PRISM_EAGLE3 was undocumented in both places and is added too; the new gate found it. - The header comment gains the two flags it was missing. test-setup-optin-flags-documented asserts the general form: every WITH_* flag setup.sh reads in code must be named in --help. Comment-only mentions are excluded, so a flag another script owns cannot satisfy it. test-compose-drafter-preflight executes the entrypoint with vllm stubbed and asserts three things: a missing drafter refuses and names both routes; the check is inert when the drafter is present; and SPEC_N=0 still boots with nothing on disk — the escape hatch must not require the download it exists to avoid. test-spec-toggle-contract needed one change to be able to see any of this: it runs entrypoints in a sandbox, and a fail-loud check on a path embedded in the entrypoint (rather than in `command:`, which it already stubs) made the compose exit before emitting argv. It now redirects the container HF cache into the sandbox and materialises the model dirs the body names. Verified: both new gates fail against the pre-fix tree — the flags gate names all four flags as undocumented, and the preflight gate shows --speculative-config reaching vLLM pointing at a directory that does not exist. The spec-toggle change is load-bearing, not cosmetic: the old gate reds against the new compose. Not boot-tested — no weights for this model here, and the entrypoint was exercised with a stub rather than a container. Fixes #1304 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`_hf_download_repo` called the hub CLI exactly once. Under `set -euo pipefail` a single transient stall — seen in the wild as `httpx.ConnectTimeout: _ssl.c:1015: The handshake operation timed out`, 1.1 GB into a 17 GB download, against a hub that was otherwise answering in 0.65s — aborted the entire setup run with a ~40-line unhandled Python traceback. The recovery was trivial and entirely undiscoverable: re-running the identical command resumed from the `.incomplete` markers the hub client had already written. Nothing in the output said so, so a first-time user reads a traceback as "this is broken" rather than "run it again". Wrap the call in a bounded retry (4 attempts, 8s doubling to 120s, both env-tunable), and on final failure print an instruction rather than leaving the traceback as the last word: the transfer is resumable, re-running continues it, and here is where the partial files are. The env hardening was already correct and is untouched — HF_HUB_ENABLE_HF_TRANSFER=1 and HF_HUB_DISABLE_XET=1 still go on every attempt. No external wrapper is introduced: the hardened wrapper this repo ships for ComfyUI is a different call path, and a shipped setup.sh cannot depend on anything outside the repo, so the retry lives here. NOT implemented: the stall watchdog the issue raises as a third option. A transfer that sits idle without erroring is a different failure than a non-zero exit, and a plain retry does not catch it. Left for a follow-up rather than half-built. test-setup-download-retry extracts the function and drives it against a stubbed CLI: a clean download still takes exactly one attempt, two transient failures are survived, a permanent failure is bounded and non-zero, and the give-up message says the transfer is resumable and that re-running continues it. Verified: the gate fails against the unfixed tree on case 2 with exactly the reported shape — one attempt, a traceback, rc=1. Real network behaviour is NOT verified; every attempt here was a stubbed CLI, so what is proven is the retry loop and the message, not that a real stalled 17 GB transfer resumes (the issue verified that half independently). Fixes #1305 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three defects found by reproducing a Discord user's failure on a clean install. They are separate bugs but sit on one user journey —
setup.shthenswitch.shfor a gemma slug — and a new user hits all three in order. One commit each.Fixes #1303
Fixes #1304
Fixes #1305
1.
setup.sh gemma-4-26b-a4bdownloaded 16 GB no slug can serve (#1303)profiles/models/gemma-4-26b-a4b.ymlcarrieddefault_weight_variant: awqagainstsetup: primary: autoround-int4-mixed. That tier's composes had been moved to_archive/and no registry entry references the variant, but the setup block was never repointed — so the obvious path downloaded the wrong 16 GB, skipped the 17 GB both slugs need, and failed atswitch.shtime.Dropped the override so setup falls back to the profile default. The weights entry itself stays registered (kv-calc and the
arch_patches/profile_runtimeevidence rows still name it).setup.primaryis a deliberate override mechanism, so the new gate is not "primary == default_weight_variant" — that would red the two models that override it correctly (qwen3.8-27b→ fp8,deepseek-v4-flash-0731→ unsloth-iq2-xxs). It asserts the weaker invariant true of all three: whatever setup fetches, some registry entry must serve it. It is also status-agnostic — a deprecated entry still pins its variant, and three models' primaries are referenced only by deprecated slugs today, which is a different question.2. The MTP drafter was opt-in behind a flag with no user-facing surface (#1304)
WITH_ASSISTANT_DRAFT=1existed in the model profile, insetup.sh's implementation and in a test — and nowhere a user would look, while its siblingsWITH_DFLASH_DRAFTandWITH_VISIONwere at least in the header comment.vllm/gemma-26ba4b-singledefaults toSPEC_N=4, so a user who did not set it got a bare missing-config.jsonnaming neither the cause nor either fix. The working escape hatch (SPEC_N=0, standardised across every vLLM compose by #1048) was not surfaced in the failure path either.Three surfaces, working outward from the failure:
SPEC_N>0and the drafter has noconfig.json, naming both routes — same fail-loud contract the deepseek moecache composes carry for their external GGUF ([bench] 2x A6000 - llamacpp-club3090/deepseek-flash-dual-q8-moecache #1054). Its header'sDrafter:line now says how to obtain the file.setup.sh --helpgrows an "Optional extra downloads" section. The header comment was never the right surface: reading it means already having opened the script.WITH_PRISM_EAGLE3turned out to be undocumented in both places too — the new gate found it, so it is documented here as well.test-spec-toggle-contractneeded one change to be able to see any of this: it runs entrypoints in a sandbox, and a fail-loud check on a path embedded in the entrypoint (rather than incommand:, which it already stubs) made the compose exit before emitting argv. It now redirects the container HF cache into the sandbox. Confirmed load-bearing, not cosmetic: the old gate reds against the new compose.3. One transient stall aborted a 17 GB download with a traceback (#1305)
_hf_download_repocalled the hub CLI once. Underset -euo pipefaila singlehttpx.ConnectTimeoutmid-transfer aborted the whole run with a ~40-line unhandled traceback. Re-running resumed cleanly — nothing said so.Bounded retry (4 attempts, 8s doubling to 120s, both env-tunable), and a give-up message that is an instruction rather than a traceback. The env hardening was already correct and is untouched; no external wrapper is introduced, since a shipped
setup.shcannot depend on anything outside the repo.Not implemented: the stall watchdog the issue raises as a third option. An idle-but-not-erroring transfer is a different failure than a non-zero exit and a plain retry will not catch it — left for a follow-up rather than half-built.
Validation
for t in scripts/tests/*.sh; do bash "$t"; done— 172 pass / 0 fail (master baseline was 168/0; four new test files).Every new assertion was negative-controlled against the unfixed state first:
test-setup-primary-slug-backedgemma-4-26b-a4band nothing else — the two legitimate overrides are not flaggedtest-setup-optin-flags-documentedWITH_*flags as undocumented in--helptest-compose-drafter-preflight--speculative-configreaching vLLM pointed at a directory that does not existtest-setup-download-retrytest-spec-toggle-contract(edit)Each also has an in-test positive control, so a scanner that quietly matched nothing cannot pass.
Not verified: nothing was boot-tested. There are no
gemma-4-26b-a4bweights on the rig that produced this, and no container was started. The compose entrypoint was exercised with a stubbedvllm, not in an image — so what is proven is the shell logic and the messages, not engine behaviour. Likewise the retry was driven against a stubbed CLI: the loop and the give-up message are tested, but not that a real stalled 17 GB transfer resumes (#1305 verified that half independently, 1.1 GB → 6.4 GB with three.incompletemarkers).One hardening found while testing rather than from the issues: a mistyped
HF_DOWNLOAD_RETRIESused to die withfoo: unbound variable, because(( attempt >= max ))resolves a non-numeric value as a variable name — a crash inside the code that handles crashes. It now errors clearly, and case 5 covers it.🤖 Generated with Claude Code