mirror of
https://github.com/HKUDS/nanobot.git
synced 2026-09-02 17:22:06 +03:00
fix(webui): keep schema defaults out of model presets
This commit is contained in:
@@ -280,7 +280,10 @@ def migrate_model_configurations(
|
||||
config_path: Path | None = None,
|
||||
) -> dict[str, Any]:
|
||||
config = _load_settings_config(config_path)
|
||||
if models.migrate_model_configurations(config):
|
||||
if models.migrate_model_configurations(
|
||||
config,
|
||||
oauth_status=_oauth_provider_status,
|
||||
):
|
||||
_save_settings_config(config, config_path)
|
||||
return settings_payload(config_path=config_path)
|
||||
|
||||
|
||||
@@ -81,6 +81,7 @@ class ModelSettingsPayload(TypedDict):
|
||||
model_presets: list[dict[str, Any]]
|
||||
model_call_order: list[str]
|
||||
model_call_order_editable: bool
|
||||
model_configuration_migratable: bool
|
||||
providers: list[dict[str, Any]]
|
||||
|
||||
|
||||
@@ -925,6 +926,48 @@ def _model_call_order_state(config: Config) -> tuple[list[str], bool]:
|
||||
return order, True
|
||||
|
||||
|
||||
def _legacy_model_configuration_migratable(
|
||||
config: Config,
|
||||
oauth_status: OAuthStatusReader,
|
||||
) -> bool:
|
||||
"""Return whether the implicit default represents usable legacy configuration.
|
||||
|
||||
A pristine config still carries schema defaults for backwards compatibility.
|
||||
Those defaults are not user configuration and must not be materialized as a
|
||||
preset. Inline fallbacks, or a default whose matching provider is configured,
|
||||
are evidence that there is real legacy state to preserve.
|
||||
"""
|
||||
_, editable = _model_call_order_state(config)
|
||||
if editable:
|
||||
return False
|
||||
|
||||
defaults = config.agents.defaults
|
||||
if defaults.fallback_models:
|
||||
return True
|
||||
|
||||
provider_name = defaults.provider
|
||||
if provider_name == "auto":
|
||||
model_prefix = defaults.model.split("/", 1)[0] if "/" in defaults.model else ""
|
||||
if model_prefix and resolve_settings_provider(config, model_prefix) is not None:
|
||||
provider_name = model_prefix
|
||||
else:
|
||||
provider_name = (
|
||||
config.get_provider_name(
|
||||
defaults.model,
|
||||
preset=config.resolve_default_preset(),
|
||||
)
|
||||
or ""
|
||||
)
|
||||
if not provider_name or provider_name == "auto":
|
||||
return False
|
||||
|
||||
resolved_provider = resolve_settings_provider(config, provider_name)
|
||||
if resolved_provider is None:
|
||||
return False
|
||||
spec, _, provider_config = resolved_provider
|
||||
return provider_configured_for_settings(spec, provider_config, oauth_status)
|
||||
|
||||
|
||||
def _validate_configured_provider(
|
||||
config: Config,
|
||||
provider: str,
|
||||
@@ -1073,6 +1116,10 @@ def model_settings_payload(
|
||||
"model_presets": model_presets,
|
||||
"model_call_order": model_call_order,
|
||||
"model_call_order_editable": model_call_order_editable,
|
||||
"model_configuration_migratable": _legacy_model_configuration_migratable(
|
||||
config,
|
||||
oauth_status,
|
||||
),
|
||||
"providers": providers,
|
||||
}
|
||||
|
||||
@@ -1153,6 +1200,10 @@ def create_model_configuration(
|
||||
raise WebUISettingsError("configuration already exists", status=409)
|
||||
_validate_configured_provider(config, provider, oauth_status)
|
||||
|
||||
activate_as_primary = not config.model_presets and not _legacy_model_configuration_migratable(
|
||||
config, oauth_status
|
||||
)
|
||||
|
||||
base = config.resolve_preset()
|
||||
max_tokens = _parse_positive_int(
|
||||
query_first_alias(query, "max_tokens", "maxTokens"),
|
||||
@@ -1180,6 +1231,9 @@ def create_model_configuration(
|
||||
temperature=temperature if temperature is not None else base.temperature,
|
||||
reasoning_effort=reasoning_effort,
|
||||
)
|
||||
if activate_as_primary:
|
||||
config.agents.defaults.model_preset = name
|
||||
config.agents.defaults.fallback_models = []
|
||||
return name
|
||||
|
||||
|
||||
@@ -1299,8 +1353,18 @@ def update_model_call_order(config: Config, query: QueryParams) -> bool:
|
||||
return changed
|
||||
|
||||
|
||||
def migrate_model_configurations(config: Config) -> bool:
|
||||
def migrate_model_configurations(
|
||||
config: Config,
|
||||
*,
|
||||
oauth_status: OAuthStatusReader,
|
||||
) -> bool:
|
||||
"""Materialize legacy primary/inline model settings as named presets."""
|
||||
_, editable = _model_call_order_state(config)
|
||||
if editable:
|
||||
return False
|
||||
if not _legacy_model_configuration_migratable(config, oauth_status):
|
||||
raise WebUISettingsError("there is no legacy model configuration to convert", status=409)
|
||||
|
||||
defaults = config.agents.defaults
|
||||
primary = config.resolve_preset()
|
||||
created: list[str] = []
|
||||
|
||||
@@ -287,6 +287,33 @@ def test_create_model_configuration_accepts_legacy_label_without_changing_call_o
|
||||
assert duplicate.value.status == 409
|
||||
|
||||
|
||||
def test_first_model_configuration_replaces_unused_schema_default(
|
||||
tmp_path,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
config_path = tmp_path / "config.json"
|
||||
config = Config()
|
||||
config.providers.openai.api_key = "sk-test"
|
||||
save_config(config, config_path)
|
||||
monkeypatch.setattr("nanobot.config.loader._current_config_path", config_path)
|
||||
|
||||
payload = create_model_configuration(
|
||||
{
|
||||
"name": ["openai"],
|
||||
"provider": ["openai"],
|
||||
"model": ["openai/gpt-4.1"],
|
||||
}
|
||||
)
|
||||
|
||||
assert payload["model_call_order"] == ["openai"]
|
||||
assert payload["model_call_order_editable"] is True
|
||||
assert payload["agent"]["model_preset"] == "openai"
|
||||
saved = load_config(config_path)
|
||||
assert saved.agents.defaults.model_preset == "openai"
|
||||
assert saved.agents.defaults.fallback_models == []
|
||||
assert saved.model_presets["openai"].model == "openai/gpt-4.1"
|
||||
|
||||
|
||||
def test_create_model_configuration_preserves_canonical_name(
|
||||
tmp_path,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
@@ -337,7 +364,7 @@ def test_create_model_configuration_accepts_dynamic_custom_provider(
|
||||
}
|
||||
)
|
||||
|
||||
assert payload["agent"]["model_preset"] == "default"
|
||||
assert payload["agent"]["model_preset"] == "tenant-model"
|
||||
assert payload["created_model_preset"] == "tenant-model"
|
||||
saved = load_config(config_path)
|
||||
assert saved.model_presets["tenant-model"].provider == DYNAMIC_PROVIDER_NAME
|
||||
@@ -604,6 +631,7 @@ def test_migrate_model_configurations_preserves_legacy_chain(
|
||||
legacy_payload = settings_payload()
|
||||
assert legacy_payload["model_call_order"] == []
|
||||
assert legacy_payload["model_call_order_editable"] is False
|
||||
assert legacy_payload["model_configuration_migratable"] is True
|
||||
|
||||
payload = migrate_model_configurations()
|
||||
|
||||
@@ -621,6 +649,27 @@ def test_migrate_model_configurations_preserves_legacy_chain(
|
||||
assert set(load_config(config_path).model_presets) == {"gpt-4o", "claude-sonnet-4"}
|
||||
|
||||
|
||||
def test_schema_default_is_not_exposed_or_materialized_as_legacy_configuration(
|
||||
tmp_path,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
config_path = tmp_path / "config.json"
|
||||
save_config(Config(), config_path)
|
||||
monkeypatch.setattr("nanobot.config.loader._current_config_path", config_path)
|
||||
|
||||
payload = settings_payload()
|
||||
|
||||
assert payload["model_configuration_migratable"] is False
|
||||
assert payload["model_call_order_editable"] is False
|
||||
with pytest.raises(WebUISettingsError) as error:
|
||||
migrate_model_configurations()
|
||||
|
||||
assert error.value.status == 409
|
||||
saved = load_config(config_path)
|
||||
assert saved.agents.defaults.model_preset is None
|
||||
assert saved.model_presets == {}
|
||||
|
||||
|
||||
def test_model_configuration_advanced_options_round_trip(
|
||||
tmp_path,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
@@ -2288,7 +2337,7 @@ def test_create_model_configuration_accepts_configured_oauth_provider(
|
||||
}
|
||||
)
|
||||
|
||||
assert payload["agent"]["model_preset"] == "default"
|
||||
assert payload["agent"]["model_preset"] == "codex"
|
||||
assert payload["created_model_preset"] == "codex"
|
||||
saved = load_config(config_path)
|
||||
assert saved.model_presets["codex"].provider == "openai_codex"
|
||||
@@ -2376,7 +2425,7 @@ def test_create_model_configuration_accepts_azure_openai_aad_mode(
|
||||
}
|
||||
)
|
||||
|
||||
assert payload["agent"]["model_preset"] == "default"
|
||||
assert payload["agent"]["model_preset"] == "azure-aad"
|
||||
assert payload["created_model_preset"] == "azure-aad"
|
||||
saved = load_config(config_path)
|
||||
assert saved.model_presets["azure-aad"].provider == "azure_openai"
|
||||
|
||||
@@ -53,6 +53,7 @@ def test_model_domain_owns_dto_and_config_updates() -> None:
|
||||
"model_presets",
|
||||
"model_call_order",
|
||||
"model_call_order_editable",
|
||||
"model_configuration_migratable",
|
||||
"providers",
|
||||
}
|
||||
assert payload["agent"]["model"] == "openai/gpt-5.4"
|
||||
|
||||
@@ -560,7 +560,8 @@ export function ModelsSettings({
|
||||
{tx("settings.models.presets", "Model presets")}
|
||||
</SettingsSectionTitle>
|
||||
<SettingsGroup>
|
||||
{!settings.model_call_order_editable ? (
|
||||
{!settings.model_call_order_editable &&
|
||||
settings.model_configuration_migratable !== false ? (
|
||||
<div className="flex flex-col gap-4 px-4 py-4 sm:flex-row sm:items-center sm:justify-between sm:px-5">
|
||||
<div className="flex min-w-0 items-start gap-3">
|
||||
<span className="grid h-9 w-9 shrink-0 place-items-center rounded-control bg-muted text-muted-foreground">
|
||||
|
||||
@@ -595,6 +595,8 @@ export interface SettingsPayload {
|
||||
}>;
|
||||
model_call_order: string[];
|
||||
model_call_order_editable: boolean;
|
||||
/** Whether an actual legacy model configuration is available to convert. */
|
||||
model_configuration_migratable?: boolean;
|
||||
created_model_preset?: string;
|
||||
created_provider?: string;
|
||||
providers: Array<{
|
||||
|
||||
@@ -690,6 +690,7 @@ describe("Settings models", () => {
|
||||
model_presets: [defaultPreset],
|
||||
model_call_order: [],
|
||||
model_call_order_editable: false,
|
||||
model_configuration_migratable: true,
|
||||
};
|
||||
const fetchMock = vi.fn(async (input: RequestInfo | URL) => {
|
||||
const url = String(input);
|
||||
@@ -725,6 +726,48 @@ describe("Settings models", () => {
|
||||
expect(screen.queryByText("Default")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("starts fresh users with an empty preset list instead of legacy conversion", async () => {
|
||||
const base = settingsPayload();
|
||||
const freshPayload: SettingsPayload = {
|
||||
...base,
|
||||
agent: {
|
||||
...base.agent,
|
||||
model: "anthropic/claude-opus-4-5",
|
||||
provider: "auto",
|
||||
resolved_provider: null,
|
||||
has_api_key: false,
|
||||
model_preset: "default",
|
||||
},
|
||||
model_presets: [
|
||||
{
|
||||
...base.model_presets[0],
|
||||
name: "default",
|
||||
label: "Default",
|
||||
active: true,
|
||||
is_default: true,
|
||||
model: "anthropic/claude-opus-4-5",
|
||||
provider: "auto",
|
||||
resolved_provider: null,
|
||||
},
|
||||
],
|
||||
model_call_order: [],
|
||||
model_call_order_editable: false,
|
||||
model_configuration_migratable: false,
|
||||
providers: [],
|
||||
};
|
||||
vi.stubGlobal("fetch", vi.fn(() => new Promise<Response>(() => {})));
|
||||
|
||||
renderSettingsView({ initialSection: "models", initialSettings: freshPayload });
|
||||
|
||||
expect(
|
||||
await screen.findByRole("button", { name: "New model preset" }),
|
||||
).toBeInTheDocument();
|
||||
expect(
|
||||
screen.queryByRole("button", { name: "Convert to presets" }),
|
||||
).not.toBeInTheDocument();
|
||||
expect(screen.queryByText("claude-opus-4-5")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("does not expose the synthetic default configuration as a WebUI preset", async () => {
|
||||
const base = settingsPayload();
|
||||
const payload: SettingsPayload = {
|
||||
|
||||
Reference in New Issue
Block a user