-
Notifications
You must be signed in to change notification settings - Fork 11.7k
Webui dynamic config #13429
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Webui dynamic config #13429
Conversation
ServeurpersoCom
commented
May 10, 2025
•
edited
Loading
edited
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this doesn't work as-is, basically this introduces a weak point that may cause the whole app to crash in the future.
Also I already planned to work on this via #11717 , so I'll implement it using a proper approach
// Note: in order not to introduce breaking changes, please keep the same data type (number, string, etc) if you want to change the default value. Do not use null or undefined for default value. | ||
// Do not use nested objects, keep it single level. Prefix the key if you need to group them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should not delete existing comments
showTokensPerSecond: false, | ||
showThoughtInProgress: false, | ||
excludeThoughtOnReq: true, | ||
// make sure these default values are in sync with `common.h` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
custom: '', // custom json-stringified object | ||
// experimental features |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
// config keys having numeric value (i.e. temperature, top_k, top_p, etc) | ||
export const CONFIG_NUMERIC_KEYS = Object.entries(CONFIG_DEFAULT) | ||
.filter((e) => isNumeric(e[1])) | ||
.map((e) => e[0]); | ||
// list of themes supported by daisyui | ||
export const THEMES = ['light', 'dark'] | ||
// make sure light & dark are always at the beginning | ||
.concat( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
getConfig(): AppConfig { | ||
return JSON.parse(localStorage.getItem('config') || '{}') as AppConfig; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be a weak typing. Basically the getConfig()
is guaranteed to return AppConfig
, but now there is a chance it will return empty object {}
, which may cause undefined behavior or even crash the whole app if some code tries to read keys from that empty object.
For me the main problem with the WebUI defaults is that those are always being sent. This seems to be trying to sync WebUI with the llama.cpp/tools/server/webui/src/utils/app.context.tsx Lines 208 to 233 in 8c83449
llama.cpp/tools/server/server.cpp Lines 250 to 274 in 9a390c4
I think it makes sense sync frontend and backend, but because it's not always possible to keep them in sync, allow NOT sending the defaults, to fully rely on the backend values. This would also better align with #13367 and similar, where WebUI might be sending requests to different models with different settings, and keeping multiple defaults in WebUI might become hard to maintain. |
You're absolutely right — this is the core issue I ran into as well. The current behavior of always sending the full WebUI config overrides any server-side defaults, even when values haven’t been changed by the user. This breaks the ability to tune the server per model, which is especially important when exposing a shared WebUI for others to use. Ideally, the WebUI should only send parameters that have been explicitly changed by the user. That way:
To keep the frontend simple, when resetting a field, we could just clear it or show something like "default from server", allowing the backend to handle the fallback logic. That way, the WebUI doesn’t have to duplicate config logic — it only overrides selectively, when explicitly told to. For example, if I deploy a well-tuned MoE model on the server, I want a friend using the WebUI to benefit from the correct sampling settings and get optimal performance out-of-the-box — not be stuck with unrelated or legacy settings hardcoded from the webui. This would make the UI much easier to maintain, while preserving clean server-driven behavior. |