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,
|
config_path: Path | None = None,
|
||||||
) -> dict[str, Any]:
|
) -> dict[str, Any]:
|
||||||
config = _load_settings_config(config_path)
|
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)
|
_save_settings_config(config, config_path)
|
||||||
return settings_payload(config_path=config_path)
|
return settings_payload(config_path=config_path)
|
||||||
|
|
||||||
|
|||||||
@@ -81,6 +81,7 @@ class ModelSettingsPayload(TypedDict):
|
|||||||
model_presets: list[dict[str, Any]]
|
model_presets: list[dict[str, Any]]
|
||||||
model_call_order: list[str]
|
model_call_order: list[str]
|
||||||
model_call_order_editable: bool
|
model_call_order_editable: bool
|
||||||
|
model_configuration_migratable: bool
|
||||||
providers: list[dict[str, Any]]
|
providers: list[dict[str, Any]]
|
||||||
|
|
||||||
|
|
||||||
@@ -925,6 +926,48 @@ def _model_call_order_state(config: Config) -> tuple[list[str], bool]:
|
|||||||
return order, True
|
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(
|
def _validate_configured_provider(
|
||||||
config: Config,
|
config: Config,
|
||||||
provider: str,
|
provider: str,
|
||||||
@@ -1073,6 +1116,10 @@ def model_settings_payload(
|
|||||||
"model_presets": model_presets,
|
"model_presets": model_presets,
|
||||||
"model_call_order": model_call_order,
|
"model_call_order": model_call_order,
|
||||||
"model_call_order_editable": model_call_order_editable,
|
"model_call_order_editable": model_call_order_editable,
|
||||||
|
"model_configuration_migratable": _legacy_model_configuration_migratable(
|
||||||
|
config,
|
||||||
|
oauth_status,
|
||||||
|
),
|
||||||
"providers": providers,
|
"providers": providers,
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1153,6 +1200,10 @@ def create_model_configuration(
|
|||||||
raise WebUISettingsError("configuration already exists", status=409)
|
raise WebUISettingsError("configuration already exists", status=409)
|
||||||
_validate_configured_provider(config, provider, oauth_status)
|
_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()
|
base = config.resolve_preset()
|
||||||
max_tokens = _parse_positive_int(
|
max_tokens = _parse_positive_int(
|
||||||
query_first_alias(query, "max_tokens", "maxTokens"),
|
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,
|
temperature=temperature if temperature is not None else base.temperature,
|
||||||
reasoning_effort=reasoning_effort,
|
reasoning_effort=reasoning_effort,
|
||||||
)
|
)
|
||||||
|
if activate_as_primary:
|
||||||
|
config.agents.defaults.model_preset = name
|
||||||
|
config.agents.defaults.fallback_models = []
|
||||||
return name
|
return name
|
||||||
|
|
||||||
|
|
||||||
@@ -1299,8 +1353,18 @@ def update_model_call_order(config: Config, query: QueryParams) -> bool:
|
|||||||
return changed
|
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."""
|
"""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
|
defaults = config.agents.defaults
|
||||||
primary = config.resolve_preset()
|
primary = config.resolve_preset()
|
||||||
created: list[str] = []
|
created: list[str] = []
|
||||||
|
|||||||
@@ -287,6 +287,33 @@ def test_create_model_configuration_accepts_legacy_label_without_changing_call_o
|
|||||||
assert duplicate.value.status == 409
|
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(
|
def test_create_model_configuration_preserves_canonical_name(
|
||||||
tmp_path,
|
tmp_path,
|
||||||
monkeypatch: pytest.MonkeyPatch,
|
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"
|
assert payload["created_model_preset"] == "tenant-model"
|
||||||
saved = load_config(config_path)
|
saved = load_config(config_path)
|
||||||
assert saved.model_presets["tenant-model"].provider == DYNAMIC_PROVIDER_NAME
|
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()
|
legacy_payload = settings_payload()
|
||||||
assert legacy_payload["model_call_order"] == []
|
assert legacy_payload["model_call_order"] == []
|
||||||
assert legacy_payload["model_call_order_editable"] is False
|
assert legacy_payload["model_call_order_editable"] is False
|
||||||
|
assert legacy_payload["model_configuration_migratable"] is True
|
||||||
|
|
||||||
payload = migrate_model_configurations()
|
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"}
|
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(
|
def test_model_configuration_advanced_options_round_trip(
|
||||||
tmp_path,
|
tmp_path,
|
||||||
monkeypatch: pytest.MonkeyPatch,
|
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"
|
assert payload["created_model_preset"] == "codex"
|
||||||
saved = load_config(config_path)
|
saved = load_config(config_path)
|
||||||
assert saved.model_presets["codex"].provider == "openai_codex"
|
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"
|
assert payload["created_model_preset"] == "azure-aad"
|
||||||
saved = load_config(config_path)
|
saved = load_config(config_path)
|
||||||
assert saved.model_presets["azure-aad"].provider == "azure_openai"
|
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_presets",
|
||||||
"model_call_order",
|
"model_call_order",
|
||||||
"model_call_order_editable",
|
"model_call_order_editable",
|
||||||
|
"model_configuration_migratable",
|
||||||
"providers",
|
"providers",
|
||||||
}
|
}
|
||||||
assert payload["agent"]["model"] == "openai/gpt-5.4"
|
assert payload["agent"]["model"] == "openai/gpt-5.4"
|
||||||
|
|||||||
@@ -560,7 +560,8 @@ export function ModelsSettings({
|
|||||||
{tx("settings.models.presets", "Model presets")}
|
{tx("settings.models.presets", "Model presets")}
|
||||||
</SettingsSectionTitle>
|
</SettingsSectionTitle>
|
||||||
<SettingsGroup>
|
<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 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">
|
<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">
|
<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: string[];
|
||||||
model_call_order_editable: boolean;
|
model_call_order_editable: boolean;
|
||||||
|
/** Whether an actual legacy model configuration is available to convert. */
|
||||||
|
model_configuration_migratable?: boolean;
|
||||||
created_model_preset?: string;
|
created_model_preset?: string;
|
||||||
created_provider?: string;
|
created_provider?: string;
|
||||||
providers: Array<{
|
providers: Array<{
|
||||||
|
|||||||
@@ -690,6 +690,7 @@ describe("Settings models", () => {
|
|||||||
model_presets: [defaultPreset],
|
model_presets: [defaultPreset],
|
||||||
model_call_order: [],
|
model_call_order: [],
|
||||||
model_call_order_editable: false,
|
model_call_order_editable: false,
|
||||||
|
model_configuration_migratable: true,
|
||||||
};
|
};
|
||||||
const fetchMock = vi.fn(async (input: RequestInfo | URL) => {
|
const fetchMock = vi.fn(async (input: RequestInfo | URL) => {
|
||||||
const url = String(input);
|
const url = String(input);
|
||||||
@@ -725,6 +726,48 @@ describe("Settings models", () => {
|
|||||||
expect(screen.queryByText("Default")).not.toBeInTheDocument();
|
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 () => {
|
it("does not expose the synthetic default configuration as a WebUI preset", async () => {
|
||||||
const base = settingsPayload();
|
const base = settingsPayload();
|
||||||
const payload: SettingsPayload = {
|
const payload: SettingsPayload = {
|
||||||
|
|||||||
Reference in New Issue
Block a user