Studio: launch a DFlash speculative drafter automatically - #8338
Conversation
Studio has recognised dflash-*.gguf since #7811, but only to hide it from the quant picker. Nothing ever launched it, so a model that ships a DFlash sidecar fell through to no speculative decoding at all. Add DFlash as the third launchable drafter kind beside MTP and DSpark: a _is_dflash_drafter_path predicate, local and Hub discovery, a supports_dflash capability parsed from llama-server --help, and the --model-draft / --spec-type draft-dflash emission. Unlike DSpark it is on under Auto, since the published sidecar is 1.52 GiB and ships in the model's own GGUF repo rather than being an ~11 GB opt-in fetch. DSpark keeps first refusal when a repo somehow ships both, matching llama.cpp's own downloader. Discovery confirms general.architecture = dflash in the header rather than pairing on the filename: the published sidecar is dflash-kquant.gguf, which names no model family, so the DSpark pairing rule would reject the one file this exists to find. The dflash/ directory is still not a drafter marker, and DFlash is still excluded from companion reclaim, both because the name doubles as a family a publisher puts on real weights.
for more information, see https://pre-commit.ci
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Four fixes from review of the auto-launch path. Strip user-supplied DFlash args on the drafterless retry. The gate that enters the retry counts a DFlash request, but the cleanup only recognised MTP and DSpark. llama.cpp accumulates speculative types, so prepending --spec-default while the DFlash group survived relaunched the drafter that had just failed, and a main model that loads fine without it was lost instead of recovered. Skip a DFlash sidecar that names another weight in the same folder. _drafter_matches_weight is False both for a sidecar naming no family and for one naming a different family, so ranking put them in one bucket and precision could float the foreign one to the top: loading model B beside dflash-model-A-Q8_0.gguf and dflash-kquant.gguf launched model A's drafter. Both files carry a real dflash header, so the architecture check behind the ranking cannot catch it. The decision is made against the weights actually present in the folder rather than by guessing which stems are precision tokens, which keeps the published unpaired sidecar eligible. Stand the Auto DFlash fetch down once DSpark has resolved. DSpark takes first refusal in the promotion, so for a repo shipping both kinds the DFlash sidecar could never launch and the fetch spent bandwidth and cache on a file that would not be used. An explicit dflash request still fetches. Keep Auto deduplicated after a failed DFlash drafter. _speculative_type is reset to "default" by a successful drafterless retry while the launch still records the resolved sidecar, so the next Apply compared the intent's empty MTP path against it and reloaded a healthy server. _spec_drafter_kind survives the fallback and now decides the comparison. test_mtp_drafter_companion.py, test_native_gguf_companion.py, test_llama_cpp_mtp_detection.py and test_resolve_quant_gguf.py: 489 passed, including two new tests for the foreign-sidecar case and for the paired sidecar still winning.
for more information, see https://pre-commit.ci
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Discovery now accepts the dflash- prefix only. The shared companion predicates recognise DFlash by that prefix, so a <model>-dflash.gguf accepted by discovery was also a selectable Q8_0 main model in the quant picker, and choosing that variant handed llama-server the drafter as the target. Teaching the predicate the suffix instead would hide a real model whose name merely ends in DFlash, which is the case #7811 exists to protect, so detection gives the form up rather than the picker giving up a model. No published sidecar uses it; the shipped one is dflash-kquant.gguf. The same mismatch exists for MTP on main and is left alone here. The training VRAM guard now sizes a drafter named through llama_extra_args. Discovery never fills gguf_dflash_file for a file outside the model directory, but load_model still passes that path to llama-server, so a load could be admitted beside a training run while nothing was charged for the sidecar it makes resident. The Speculative Decoding hint said Auto picks DSpark or else MTP / ngram and that everything but DSpark leaves output unchanged. Auto now picks DFlash too, and like DSpark it is not bit-identical on quantized targets. The Draft Tokens hint gained the DFlash default, which shares the MTP branch at 2 on GPU and 3 on CPU/Mac. 514 passed across the drafter, companion, detection, quant-resolution and picker suites, including two new tests pinning the suffix form out of discovery and the prefix form still in.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
detect_dflash_file already refuses a sidecar named after a NEIGHBOURING weight, so a folder holding two families cannot attach a foreign drafter locally. The download picker and the offline cache reuse still ranked every dflash-*.gguf by precision and name alone, never comparing a candidate against the weight being loaded, so in a repo hosting more than one family dflash-model-A-Q8_0.gguf outranked the generic dflash-kquant.gguf and model B downloaded and launched model A's drafter. The pairing rule now lives in one place, dflash_repo_preference_key, built on the same _drafter_names_other_weight predicate the local scan uses: a sidecar naming this weight's family first (most specific stem first, as detect_mtp_file does), then one naming no weight present here, then one naming a neighbour. The last is demoted rather than dropped, so a repo whose only sidecar looks foreign still has a fallback. Deciding against the weights actually present is what keeps the published unpaired sidecar eligible: dflash-kquant.gguf has a precision token for a stem, not a family name, so "the stem is non-empty" cannot stand in for "this names another model". Nothing changes for a repo with one sidecar, and with no weight in hand the order is precision only, as before. Tests cover a multi-family repo picking the generic sidecar, the same repo picking the specific one for its own weight, the shipped Muse-Glimmer layout still resolving, and the cached path agreeing with the download path.
for more information, see https://pre-commit.ci
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Three fixes found in review of the DFlash drafter work. detect_dflash_file read a candidate's GGUF header before asking the caller's accept callback about it, so a dflash-*.gguf symlink in a directory reached through a native grant had its out-of-lease target opened before the grant check ran, and no later rejection takes a read back. The loop now resolves the launch path, runs accept, and only then parses the header and applies the architecture check. accept still receives the resolved launch path, and callers that pass no accept see the same candidates in the same order as before. The training admission guard charged the llama_extra_args --model-draft sidecar on top of the local one discovery had already found, so a 1.5 GiB drafter was billed as 3 GiB and the guard could refuse an inference load that fits. The effective draft path is now sized exactly once, with identity taken from the resolved path so a symlink or another spelling of the same file dedupes too. That same charge also satisfied the local-weights early return on its own. Loading a remote GGUF repo has no local main weight, so a local --model-draft made the guard return the drafter alone and skip the listing that prices the target model, which could admit a load that then exhausts VRAM next to a running training job. The local branch now fires only when a local weight is actually present, and the drafter is added to whichever branch produces the estimate, including the remote one. Regression tests for all three.
for more information, see https://pre-commit.ci
# Conflicts: # studio/backend/core/inference/llama_cpp.py
There was a problem hiding this comment.
💡 Codex Review
unsloth/studio/backend/hub/utils/gguf.py
Line 88 in 9ac0469
Keep root dflash-prefixed model weights selectable
Adding dflash to this basename-prefix classifier makes every root-level dflash-*.gguf look like a companion, even when it is the actual model. The repository itself notes that whole repos such as Lucebox/Qwen3.6-27B-DFlash-GGUF publish ordinary weights with this prefix; these files are now filtered by build_gguf_variant_plans, inventory scans, and the mirrored local-model detector, leaving such repositories without variants and rejecting direct local loads. DFlash sidecars need a discriminator that does not blanket-hide all weights sharing the prefix.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…rd for PR #8338 Six review items from the third round, each reproduced first. Five are places the previous round's rules had not reached. dflash_plan_files now filters candidate families before ranking rather than after, so a half-published split set or an oversized ordinary weight at the top of the order steps aside for a usable sidecar behind it instead of taking the plan down with it. It also applies the split-completeness rule the runtime got last round, since planning a set the listing only half carries reports the download complete and then loses DFlash. The runtime size bound compared the picked shard rather than its whole set, so a split ordinary weight whose halves each sit under the target still downloaded in full. It sums the family now, through a shared helper. The training coexistence guard took the maximum over every root candidate with no size bound at all, charging gigabytes for files the fetch itself refuses. dflash_budget_bytes takes the target size and drops them. The incomplete-split rejection added last round lands after outcome["listed"] is set, so DSpark read a settled answer as retryable and relaunched a healthy server on every Apply. It records absence explicitly. SpeculativeType omitted dflash, so Typer rejected --speculative-type dflash before any of the new loading code ran and the mode was reachable only through Auto. Each fix has a regression test that fails without it.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Four review items from the fourth round, two of them under-charges that could admit a load beside a running training job and then exhaust VRAM. The guard priced a remote DSpark sidecar as the single file the ranking picked, while llama-server maps every shard of a split set, so a two-shard sidecar was budgeted at roughly half its resident weight. DSpark candidates are grouped into shard families now and the selected family's total is charged, matching what DFlash already did. Auto granted DSpark first refusal on the strength of the listing alone. Since the fetch now refuses an incomplete split set, the load falls through to DFlash, which can be the larger of the two, and the guard had already returned the DSpark figure. Only a complete set settles it. The runtime DFlash picker filtered incomplete families after ranking rather than before, so a half-published set at the top of the order returned a shard, _download_companion_gguf refused it, and the loop ended instead of reaching the complete sidecar behind it. Extras owning --spec-type with their own --model-draft charged the discovered sidecar as well, though _build_speculative_flags returns before that one is emitted. Only the drafter that launches is charged now. Extras without --spec-type still charge both, since Studio emits its own and which lands is genuinely unknown. The listing completeness rule was about to have three copies, so it moved into utils.models.drafters as split_listing_is_complete and the plan, the fetch and the guard all call it. Each fix has a regression test that fails without it.
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Comments and docstrings only, no code change: verified with comment_tools.py check and the prepush gate's comment-only mode.
|
@codex review |
1 similar comment
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 744603bb43
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| and (_auto_dspark or not (dspark_requested or dflash_requested)) | ||
| ), | ||
| include_dspark = (_dspark_capable and (_auto_dspark or dspark_requested)), | ||
| include_dflash = (_dflash_capable and (_auto_dflash or dflash_requested)), |
There was a problem hiding this comment.
When a remote load supplies llama_extra_args containing --spec-type draft-dflash and a local --model-draft, _extra_args_own_spec is true and load_model() skips the repository DFlash download, but _forced_dflash still makes this argument true. The training coexistence estimate consequently charges both the repository sidecar and _extras_bytes, potentially returning a spurious 409 for a load that would only make the explicitly named drafter resident; suppress include_dflash when the extras own the spec type.
Useful? React with 👍 / 👎.
| # over whole shard sets is the answer a listing can give. Bounded by the | ||
| # target too, so the guard stops charging for the oversized candidates the | ||
| # fetch itself now refuses. | ||
| dflash_bytes = dflash_budget_bytes(dflash_sizes, _gguf_extra_shards, weight_bytes) |
There was a problem hiding this comment.
When the Hub listing contains only part of a split DFlash sidecar, _download_companion_gguf() refuses that family because its encoded shard count is incomplete, but this call still budgets the listed partial files. Unlike the DSpark path immediately above, dflash_sizes is never filtered with split_listing_is_complete, so the active-training guard can overestimate VRAM and reject an otherwise fitting load during a partial publication; pass only complete DFlash families to the budget calculation.
Useful? React with 👍 / 👎.
| # No companion subdirectory: dflash/ is a family name a user picks for real | ||
| # weights, so detect_dflash_file only ever offers a root-level sidecar and | ||
| # nothing outside the model's own directory is in bounds. | ||
| "dflash": ("DFlash drafter", None), |
There was a problem hiding this comment.
For a native directory selection whose chosen weight is under a quant subdirectory (for example repo/Q4_K_M/model.gguf), DFlash discovery intentionally searches repo/ for the root-level sidecar, but this rule supplies no allowed location beyond the weight's immediate parent. native_gguf_companion_parent_allowed() therefore rejects repo/dflash-kquant.gguf even when the signed grant covers the whole repo/ directory, so Auto silently loses DFlash for this common layout; thread the actual granted root into validation and admit the repository root only when that directory is covered.
Useful? React with 👍 / 👎.
| # DFlash asks through _dflash_retry_needed below, which is set only for | ||
| # retryable failures. A permanent listing error records no answer, so | ||
| # _dflash_sidecar_absent stays False and this arm relaunched forever. | ||
| and self._spec_drafter_kind != "dflash" |
There was a problem hiding this comment.
When the main GGUF is already cached but DFlash listing or download fails with a classified permanent Hub error such as GatedRepoError, no transient callback sets _dflash_retry_needed; this condition also excludes DFlash from the existing drafter_not_found retry path. Because matches_load_source() and _runtime_matches_intent() do not compare hf_token, applying the same model with a newly valid token deduplicates against the drafterless server and never retries the sidecar, requiring an unrelated reload or unload; retain enough failure or credential state to retry when access changes.
Useful? React with 👍 / 👎.
1 participant
Problem
Studio has recognised
dflash-*.ggufsince #7811, but only as an exclusion: the name sits in_DRAFTER_KINDSso the file is hidden from the GGUF quant picker and never mistaken for the main model. Nothing launches it.So a model shipping a DFlash sidecar gets no speculative decoding at all. It is not an MTP GGUF, so that branch does not fire. It has no DSpark sidecar, so that branch does not fire. At 30B it is above the sub-3B ngram fallback, so Auto lands on
--spec-defaultand the drafter sitting next to the weights is never opened.Fix
Add
dflashas the third launchable drafter kind, following the DSpark work in #7968 and the detection hardening in #8037.Auto-on rather than opt-in, and the size is why. The DSpark sidecar is around 11 GiB and fetched on demand, which is what made that one a deliberate choice. The published DFlash sidecar is 1.5 GiB and already ships in the model's own GGUF repo, so Auto fetches it the way it already fetches the MTP drafter.
core/inference/llama_cpp.pysupports_dflashinprobe_server_capabilities,_download_dflashand its cache reuse,_emit_dflash, thedflashcanonical mode with itsdraft-dflashlegacy alias,GgufLoadIntent.dflash_draft_path, and the Auto promotion with its reuse and fit-budget bookkeepingutils/models/drafters/routes/inference.pyfrontendThe drafters package
The rules that decide which sidecar a load ends up on are reached from four places: the local scan, the download, the snapshot reuse and the offline cache. Keeping four copies in step by hand is what most of the review on this PR was about, so they now live in one place.
model_configre-exports every moved name, so existing imports keep working. The package does not importmodel_configat module import time; the GGUF split and quant naming helpers stay where they are, since non-drafter code shares them.Behaviour worth calling out
acceptboundary before it opens anything, so a sidecar symlinked out of a native grant is never read.quants/dflash-model-Q8_0.ggufis an ordinary weight, and going by basename downloaded it in full before the header could reject it.dflashfalls through to the next candidate rather than giving up.Results
Muse Glimmer 30B,
UD-Q4_K_XLwith the publisheddflash-kquant.gguf:n_max=2n_maxis decisive and the default is deliberate. At 2 the acceptance rate runs 0.593 to 0.766; at 15 it falls to 0.259, and at 4 the run is slower than no speculation at all. Auto uses 2 on GPU and 3 on CPU and Mac, matching MTP.Testing
1109 tests across the drafter, load and training coexistence suites. Every behaviour above has a regression test that fails without its fix.