-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat(ai): per-provider max_tokens defaults with optional override #9703
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
Changes from all commits
4678db5
8f8126a
925418c
9613f17
a925628
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -400,6 +400,11 @@ def save_config( | |
| # Merge the current config with the new config | ||
| current_config = self._load_config() | ||
| merged = merge_config(current_config, config) | ||
| # None-as-delete: any key whose merged value is None (typically because | ||
| # the incoming config explicitly sent null) is removed from disk. Lets | ||
| # the UI clear optional scalars (e.g. ai.max_tokens) without a separate | ||
| # delete primitive. | ||
| _drop_none_values(cast(dict[str, Any], merged)) | ||
|
kirangadhave marked this conversation as resolved.
Comment on lines
+403
to
+407
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should note this for bug bash. We had issues in the past where config changes on the frontend didn't save, this whole function is a bit fragile.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll add it to notes for next release
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do you think we need more tests around this?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe.. The previous issues was if you add a custom model, then refresh, it wouldn't be saved. I would test ^ during bug bash |
||
|
|
||
| with open(config_path, "w", encoding="utf-8") as f: | ||
| tomlkit.dump(merged, f, sort_keys=True) | ||
|
|
@@ -459,3 +464,13 @@ def get_config(self, *, hide_secrets: bool = True) -> PartialMarimoConfig: | |
| if hide_secrets: | ||
| return mask_secrets_partial(self.override_config) | ||
| return self.override_config | ||
|
|
||
|
|
||
| def _drop_none_values(d: dict[str, Any]) -> None: | ||
| """Recursively remove keys whose value is None, in place.""" | ||
| for key in list(d): | ||
| v = d[key] | ||
| if v is None: | ||
| del d[key] | ||
| elif isinstance(v, dict): | ||
| _drop_none_values(v) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| # Copyright 2026 Marimo. All rights reserved. | ||
| from __future__ import annotations | ||
|
|
||
| DEFAULT_MAX_TOKENS = 4096 | ||
| ANTHROPIC_DEFAULT_MAX_TOKENS = 32768 | ||
|
Light2Dark marked this conversation as resolved.
|
||
| DEFAULT_MODEL = "openai/gpt-4o" | ||
Uh oh!
There was an error while loading. Please reload this page.