[staging CI] unslothai/unsloth#8757 - #327
Closed
danielhanchen wants to merge 28 commits into
Closed
Conversation
Sampling params were a single global set, so switching models handed the
next model the previous one's temperature and system prompt. Presets
were the only way back, and they exist to switch use case within one
model, not between models.
Params are now remembered per checkpoint and replayed on switch, with a
Settings toggle (on by default). A model with nothing remembered keeps
whatever is on screen rather than snapping to defaults.
Storage is a per-model map the server deep merges per key, so tuning one
model never clobbers another's:
{
"rememberParamsPerModel": true,
"inferenceParamsByModel": {
"unsloth/Qwen3.5-9B-GGUF": { "temperature": 0.2 },
"unsloth/Llama-4-8B": { "temperature": 0.9 }
}
}
Replay runs in setParams as well as setCheckpoint. The interactive local
load, the switch people use most, calls setParams with the destination
checkpoint and the backend's recommended params and only reaches
setCheckpoint later, once params.checkpoint already matches, so replay
gated on setCheckpoint alone would never fire for it. Entries hold a
full snapshot rather than the edited keys, since replay overlays the
entry onto the outgoing model's params and any gap would keep that
model's value.
Rules live in features/chat/lib/per-model-params.ts, free of store and
network imports so they stay unit testable.
…e cap Five issues from review, all confirmed against the code. A model was only remembered once it was edited, so the settings it was actually used with were lost on the way out. That hit upgrades hardest: the checkpoint restored at startup predates the map, so switching away and back handed it the other model's settings. Both switch paths now snapshot the model being left. Hydration fenced the whole map behind one version counter. If any edit landed before the settings response, every other model's persisted entry was discarded for the session. The fence is per model id now, matching what inference params already do per key. The auto-load paths call setCheckpoint and then setParams with the load response, which carries the model's context length as maxTokens. The second call saw no checkpoint change, skipped replay, overwrote the remembered budget and then snapshotted the overwritten value, even for background loads passing persist: false. Those calls are marked, replay runs for them, and a non-persisting update no longer rewrites memory. The load-time trim did not bound anything: the server merge keeps every key in its original position, so a trimmed model's later edits were dropped again on every reload and it could never retain settings. Removed, since the bound was unenforceable rather than merely loose. Replaying a model left activePreset and activePresetSource describing the model being left. For Default the settings sheet reads provenance alone, so it reported no unsaved changes over custom values. A replay that changes params now marks provenance modified.
getReplayStatePatch was only wired into setCheckpoint, so the local load path, which replays through setParams, left preset provenance describing the model being left. That matters beyond the settings sheet. Every post-load hook that re-applies model defaults bails when the preset source is not builtin-default: mergeBackendRecommendedInference returns early, and both Qwen thinking-param sites check it before writing. Leaving provenance alone therefore let the status refresh and the Qwen defaults overwrite the values that had just been replayed, which the next switch then snapshotted back over the model's memory. Marking the replay closes all of them through the mechanism already in the codebase. Test drives the real mergeBackendRecommendedInference both ways: with builtin-default the replayed temperature is overwritten, with modified it survives.
A queued external message auto-loads with preserveVisibleSettings, then restores the visible model. Both switches carry trackQueuedSettings off, and restoreVisibleModelState puts params and the runtime fields back but not activePresetSource, so marking those replays left the visible model reading as modified after every background load. That both showed Default as unsaved and stood the backend and Qwen default hooks down, which is the opposite of preserving visible settings. Provenance is now restated only for a switch that is a visible settings change. Every ordinary switch passes no options and stays marked; only the background load and its restore pass the flag off.
A status response that lands before chat settings marked its checkpoint as locally edited, so hydration kept the recommendation and dropped the model's persisted settings; switching away then wrote the recommendation back. Per-model memory is now only recorded and fenced after hydration, and the model already selected when settings land has its memory replayed, since a resident or restored checkpoint never crosses a transition. Replace the replay provenance mark with a replay at the three sites that re-apply a model's defaults. Marking modified stood the defaults down for every model, including ones with nothing remembered, which is the behaviour this PR set out to fix.
A model's defaults are not settings it was used with. Recording them made the load response become memory, which the Qwen thinking-mode hook then replayed over itself: its minP and presence penalty were cancelled on every fresh load. Params staged for the model about to load were filed against the model still on screen, so its remembered context length became the other model's. Only an edit is recorded now; both cases are still picked up by the snapshot taken when the model is left, which clearCheckpoint now takes too, so an unload or eviction no longer forgets what the model was running with. A replayed output budget is clamped to the context the model just loaded with, and model defaults no longer fence hydration, so an edit made while the settings request is in flight outranks the replay.
Marking the staging call stopped it recording, but the switch that follows snapshots the params it left behind, so the outgoing model still ended up remembering the incoming model's context length. The snapshot now puts back what staging wrote, for those keys only and only while nothing has changed them since, so an edit made before the load still wins. A stored entry can be partial, and replay lays what it has over the params on screen, so the gaps came from whichever model was selected beforehand. Entries are completed from the saved global set as they hydrate.
A model's context is already kept per model by its load config, and that is the copy the load uses. Keeping a second one here is what let a load stage the next model's context into the model being left, and what let a replay leave the runtime advertising a context the backend never loaded. The memory no longer records or replays it, which also retires the staged-load bookkeeping the two earlier rounds added. A model loaded while the settings request is in flight has no entry to restore its defaults from, so the hydrated global set handed it the sampling of whichever model was used last. Those keys are kept, and still lose to the model's own entry when it has one.
… restores Last round's fix for a model loaded during hydration also covered the model already resident at startup, which is the one the saved global set describes, so an upgraded install had its temperature replaced by the recommendation. The exception now applies only when the defaults replaced another model. A restore after a hidden auto-load steps off the model that load put there, so its snapshot was recorded and persisted as that model's memory even though the load itself was told not to persist. A restore no longer records it; a visible switch still does.
Recording only the defaults that moved left a default equal to the outgoing model's value uncovered, so the saved global set overwrote it. It is still this model's default, and the global set is still the other model's, so the whole set is kept, which drops the key tracking with it. The replay at hydration takes the same cap the load and status replays do: a status that beat the settings response has already published the context the model loaded with, and a budget remembered from a larger one does not fit.
main landed the mirrored-settings cache, the deep-research external-model rule and its own copy of setInferenceParam in the same regions of the chat runtime store. Kept one copy of the helper, the shared one in lib/per-model-params, and took main's wording where both sides had added the same tsconfig include. The store now reads document at module scope under a `typeof window` guard, so the test kit's window fake grows a document to match.
…load An edit made while the settings GET is in flight is fenced out of the params the response carries, but the entry arriving for that model was written before the edit. The next update that re-applies model defaults replayed that entry and undid the edit. The edited keys are now laid over the model's hydrated entry as well, so the replay carries them. The remembered budget was capped only against a GGUF context. A safetensors model reloaded at a smaller sequence length replayed the older, larger budget and the next request asked for more than the load could hold. Those paths now cap at the sequence length they loaded with, the Qwen3 thinking-mode defaults use the same cap as the load they follow, and the status path caps whatever the backend reports, which it does for a safetensors load too.
The hydration replay clamped against ggufContextLength, which is null for everything that is not a GGUF, so a safetensors model whose status arrived first had its smaller context ignored and the older budget restored. The context a load or status publishes through the cap is now kept alongside the checkpoint it was reported for, and the replay uses it. Turning the memory off left the global set as whatever was last written to it. The settings on screen got there by replay, and a hidden load replays without persisting, so that could still be the previous model's set and the next launch restored it. Turning the memory off now promotes what is on screen to the one shared set, which is what the toggle says it does.
The hydration cap rode inside the replay, so an installation with only a global set and no per-model entries never got it and restored a budget the load could not hold. It applies to the hydrated params either way now. A partial entry was filled from the saved global set. That set is rewritten whenever any model is edited, so it describes whichever model was used last, and the filled entry is stored: the model's own entry would grow to hold another's settings for good. Filling from the shipped defaults trades that for overwriting the backend's recommendation for the model. Neither is an honest answer for a key the model never pinned, so the entry is kept as written and the gap takes whatever the load or status published, which is where it belongs. The first switch away writes a complete entry anyway. The server merges these per key, so a full snapshot rewrites every field of a model's entry. A second tab that had only read an entry rewrote it on any model switch, putting its stale copy over settings the first tab had since changed. A snapshot is written only for a model this browser has actually edited, or one with no entry yet, which is the case the snapshot exists for.
An edit sent the full snapshot for the model, so changing one slider from a copy hydrated before another tab changed the prompt put the old prompt back over it. The server merges per key, so only the keys the edit moved go out. That leaves the outgoing snapshot with one job, which is the one it was written for: seeding a model that has no entry at all, including the checkpoint restored at startup. Every later change is written by the edit that made it, so a snapshot for a model that already has an entry would restate keys this browser never touched. The set tracking locally edited models goes with it, having nothing left to gate.
…lone Writing an edit key by key made the patch merger's single level of nesting matter: two fields of one model changed inside the debounce window each send a one-field object, and the second replaced the first, so the earlier edit was gone on the next launch. The per-model map is merged a level further in. The hydration cap fell back to ggufContextLength for any checkpoint, but an external pick leaves whatever was loaded resident, so that value describes a local model with nothing to do with the provider's budget: a 32K budget was being clamped to an 8K resident context. The fallback is skipped for an external checkpoint, which is the only kind that can disagree with it.
A per-model entry was spread straight over the live params at both replay sites. The write side is careful about which keys go into an entry, and maxSeqLength is deliberately not one of them: the context a model loads with belongs to its load config, and per-model-params.ts says in its own comment that a second copy replayed over it would leave the runtime advertising a context the backend never loaded. The read side did not enforce that. It matters because an entry is not necessarily one this app wrote. ChatInferenceSettings accepts every persisted key inside a per-model dict, maxSeqLength included, and sanitizeInferenceParamsByModel keeps them, so an entry from an older client, a future key set or a hand-written payload arrives intact and gets replayed. The same spread also carried any key that is not an inference param at all into params, which flows on into request bodies and into a settings write whose payload is extra="forbid". Both replays now walk REMEMBERED_INFERENCE_PARAM_KEYS instead: the switch replay in getReplayedParams, and the hydration replay in the store, which is a second independent copy of the rules and was iterating the PERSISTED list. Tests are in the shape the fix has to survive: each was confirmed to fail against the previous commit. The hydration one is its own file rather than appended to per-model-params-hydration.test.ts, whose 980 lines share store and module state across tests. Also pins the id shapes a model key can take, since these are opaque keys and an id that did not round trip would mean that platform silently cannot remember settings: Windows drive and UNC paths, a WSL UNC path, a macOS path with spaces, a Linux cache path, a provider-qualified external id and a non-ASCII repo id.
Four items from the last two Codex rounds, each reproduced against the real store before touching anything, and each pinned by a test confirmed to fail at f04ad2f. Memory off no longer loses the shared set. keepModelDefaults exists so a model that loaded while the settings GET was in flight is not handed the previous model's globals. It did not consult the toggle, so with the memory off, where the global set IS the one set the user keeps for everything, a startup race stranded the model on whatever the load recommended and dropped the saved temperature and prompt for the session. Gated on the toggle as it reads after the response lands, under the same fence the scalar loop uses. A model that loaded mid-flight keeps its own context. The global maxSeqLength belongs to whichever model was used last, and an entry never carries one, so once the global loop wrote it the replay had nothing to put back: the UI advertised a context the backend never loaded. maxSeqLength is now held out of that loop for the model that just loaded, whatever the toggle says, since the context is a property of the load. A pre-hydration edit survives on an upgraded install. Settings written before this feature carry only inferenceParams, and the entry-seeding ran only when the response already had a map, so the edit was fenced out of the global set and then overwritten by the next status poll. It seeds from the fenced edits with no map too. This is the ordinary upgrade path. Compare can clamp a replayed budget. ensureModelLoaded reaches the replay through setCheckpoint, the one switch path with no way to pass the context it had just loaded, so a budget remembered from a larger context was restored above it and sent. setCheckpoint takes maxTokensCap like the rest.
# Conflicts: # studio/frontend/src/features/chat/stores/chat-runtime-store.ts # studio/frontend/src/features/settings/tabs/chat-tab.tsx
The cap describes the load, not the memory, so returning current early on a model with no entry, or with the memory off, sent a budget carried over from a larger model. Compare loading a fresh 8K model after a 32K one is the reachable case. Identity is still preserved when the budget already fits, so callers can tell a replay from an untouched switch.
On an upgraded install the global set is the only record of what the resident model was running with. Stepping off it before the settings GET returned dropped that link: nothing could be filed at the time, so the model ended up with no entry and switching back to it inherited whatever replaced it, which is the bleed this feature exists to stop. The model left before hydration is now remembered, and the global set is filed under it once the response makes it known.
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
1 participant
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.
Disposable CI run for unslothai/unsloth#8757. Do not merge; closed after CI.