Repository navigation
Studio: save sampling defaults per model and use them for API requests - #14515
danielhanchen wants to merge 13 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
The account paying for this security review has reached its Codex usage limits. The payer can check the Codex usage dashboard. For personal accounts, using credits requires enabling “Use credits for security reviews” in Code review settings. If you do not manage the paying account, contact this repository's admins. |
[skip ci]
|
@codex review |
1 similar comment
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d44cf4f793
ℹ️ 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".
| try { | ||
| // Fresh, so another tab's save is what the fields start from. | ||
| const all = await fetchModelSamplingDefaults(); | ||
| setStored({ modelId, saved: all[modelId] ?? {} }); |
There was a problem hiding this comment.
Resolve stored keys using the backend's case-folding rules
When a stored model key differs from modelId only in case, this exact lookup opens an empty editor even though saved_sampling_defaults deliberately case-folds unambiguous repository IDs and API requests are still using the stored row. Saving from this empty view creates a second differently-cased key, after which requests using another casing become ambiguous and receive no saved defaults. The read path should return or resolve the effective unambiguous entry using the same rules as the backend rather than indexing the raw map exactly.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Every write and read here goes through the same configId from the run settings page, so the stored key and the lookup key are the same string; a differently cased duplicate only arises from a hand-made PUT.
| all of them null forgets the model.""" | ||
|
|
||
| model_id: str = Field(..., min_length = 1, max_length = MAX_MODEL_OVERRIDE_KEY_LEN) | ||
| sampling: Optional[dict[str, Optional[float]]] = None |
There was a problem hiding this comment.
Reject booleans before Pydantic converts them to floats
When a caller sends a sampling value such as {"temperature": true}, this non-strict float annotation converts the boolean to 1.0 before normalize_sampling_defaults runs. _clean_sampling_value therefore no longer sees a boolean to reject and silently persists it for temperature, top-p, top-k, min-p, and presence penalty, contrary to the normalizer's validation contract. Add a mode="before" validator or use strict numeric fields so malformed boolean values cannot become real defaults.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The UI only ever sends numbers; a client posting a JSON boolean as a temperature is malformed input, and what gets stored is still range checked.
… means for Chat [skip ci]
[skip ci]
|
@codex review |
1 similar comment
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0e236c7f1f
ℹ️ 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".
| const fallback = | ||
| cleared.length > 0 | ||
| ? await fetchEffectiveSampling(modelId).catch(() => ({})) | ||
| : {}; |
There was a problem hiding this comment.
Do not silently keep stale Chat values when fallback loading fails
If the PUT succeeds but this follow-up request fails, the error is converted to {} and the save still reports success. For fields just cleared, chatPatchFrom then has no replacement value, while setRememberedParamsForModel only merges supplied keys, so Chat retains the old saved sampling values even though API requests now use the model defaults. Surface the partial failure or provide a way to remove those remembered keys instead of silently continuing with an empty fallback.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This needs the PUT to succeed and the follow-up GET on the same server to fail in between. The server copy, which is what the editor shows and what requests use, is already correct then, and the next save writes Chat's memory again.
|
@codex review |
1 similar comment
|
@codex review |
… id, let the row label wrap
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 124d06e0df
ℹ️ 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".
| <SamplingDefaultsRow | ||
| modelId={configId} | ||
| chatModelId={target.id} |
There was a problem hiding this comment.
Apply saved defaults to managed-engine completions
When a non-GGUF model is served through vLLM or SGLang and a caller uses /v1/completions, this row allows defaults to be saved and the sampling-defaults endpoint reports them, but openai_completions returns through managed_engine_completions at routes/inference.py:39938-39941 before _fill_recommended_sampling_completions runs, and that function forwards the unchanged body at routes/managed_engine_chat.py:491. Consequently omitted sampling fields ignore the saved values for this supported backend/endpoint combination; resolve and merge them before forwarding the managed-engine request.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
That path forwards the body to vLLM or SGLang before any per-model fill runs, so it has never applied the model's recommended sampling either; it is an existing gap of /v1/completions on managed engines, not something this PR changes. /v1/chat/completions on those engines does get the saved values.
|
Codex Review: Didn't find any major issues. Hooray! 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". |
|
Codex Review: Didn't find any major issues. What shall we delve into next? 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". |
…get by the row [skip ci]
|
Codex Review: Didn't find any major issues. Keep it up! 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". |
# Conflicts: # studio/frontend/scripts/check-bundle-budget.ts
|
@codex review |

Fixes #10631
Problem
Studio already saves each model's run settings (context length, KV cache, speculative decoding, GPU placement), and the OpenAI-compatible auto-switch applies them when an API request loads that model (#7473, #11904, #10216). Sampling is the gap:
temperature,top_pand so on unset always gets the model's recommended values. Nothing the user set in Studio reaches it.What this adds
A Sampling Defaults row on the model's run settings page, which is where the Hub's Run button and the picker's gear already go. It opens a small editor for Temperature, Top P, Top K, Min P, Repetition Penalty and Presence Penalty.
Saving writes the values to two places:
model_sampling_defaults).resolve_effective_samplingreads it, so/v1/chat/completions,/v1/completions,/v1/responsesand/v1/messagesfill unset fields from it, including the request that switches the model in. The order is: operatorUNSLOTH_SAMPLING_*pin, then the request's own value, then the saved default, then the model's recommendation, then the schema default. The/sampling-defaultsroute behind the API page's example request uses the same function.setRememberedParamsForModelpath the connected-model settings dialog uses. They are written under both ids a model can have in Chat (repo id and cache snapshot path). A field that was saved and is then cleared goes back to the model's recommended value. A field that was never saved keeps Chat's last-used value, as today, and the editor says so.How defaults are keyed:
_fold_case_insensitive_pathrule. POSIX paths match exactly.Not changed
/v1/completionson a vLLM or SGLang engine still forwards the body without any per-model fill, as before.End to end
Real Studio installs at the merge base and at this branch, CPU only,
unsloth/gemma-3-270m-it-GGUFQ8_0, auto-switch on. Values are what llama-server reports in/slotsafter the request:/v1/chat/completionsthat switches the model in, no sampling fieldstemperature: 0.9Cost
resolve_effective_samplingruns once per request, never per token. The store read reuses the 2 s cached setting the auto-switch already uses, and an index is rebuilt only when that cache refreshes. Per-call timing on this host (median of 5 x 20k calls, base and head alternating, two rounds):Tests
studio/backend/tests/test_model_sampling_defaults.py(16 tests) covers:/v1/completionspassthroughrepeat_penaltyalias;/sampling-defaultsreporting the saved value.Reverting only the resolution hook makes 5 of them fail. Backend tests touching the settings routes fail the same 27 pre-existing cases on the merge base and on this branch, and nothing new. Frontend:
tscclean,vite buildOK,npm test13242 passed.Screenshots
Run settings panel for the loaded model, merge base vs this PR:
The editor with three values set:
After Save, Chat's own Sampling section for the loaded model picks up the saved values (right), against the merge base (left):