From beb9c7d8871a0cf0bf376f0472cd62a73d2e2779 Mon Sep 17 00:00:00 2001 From: xiami762 Date: Sat, 8 Aug 2026 10:28:26 +0800 Subject: [PATCH 1/2] feat(config): unify model settings and reasoning defaults --- .flocks/flocks.json.example | 5 + flocks/config/config_writer.py | 226 +++++++++++++++++-- flocks/provider/model_manager.py | 11 + flocks/provider/options.py | 17 +- tests/config/test_config_writer.py | 192 +++++++++++++++- tests/provider/test_model_management_p2p3.py | 5 +- tests/provider/test_provider_options.py | 28 ++- tests/provider/test_thinking_params.py | 4 +- webui/src/pages/Model/index.tsx | 52 ++--- 9 files changed, 473 insertions(+), 67 deletions(-) diff --git a/.flocks/flocks.json.example b/.flocks/flocks.json.example index 1f8b273c7..dc9ffd359 100644 --- a/.flocks/flocks.json.example +++ b/.flocks/flocks.json.example @@ -1,5 +1,10 @@ { "provider": {}, + "default_models": { + "default_parameters": { + "reasoning_effort": "high" + } + }, "mcp": {}, "channels": {}, "plugin": [], diff --git a/flocks/config/config_writer.py b/flocks/config/config_writer.py index 2b8d6a900..62ddfe221 100644 --- a/flocks/config/config_writer.py +++ b/flocks/config/config_writer.py @@ -21,7 +21,13 @@ _FALLBACK_CONFIG_TEMPLATES: Dict[str, Dict[str, Any]] = { - "flocks.json": {}, + "flocks.json": { + "default_models": { + "default_parameters": { + "reasoning_effort": "high", + }, + }, + }, ".secret.json": {}, "mcp_list.json": { "version": "1.0.0", @@ -30,6 +36,59 @@ }, } +_MODEL_SETTING_FIELDS = ("enabled", "credential_id", "default_parameters") + + +def _extract_model_setting(model_config: Any) -> Dict[str, Any]: + """Extract user-setting fields from a provider model entry.""" + if not isinstance(model_config, dict): + return {} + return { + field: model_config[field] + for field in _MODEL_SETTING_FIELDS + if field in model_config + } + + +def _merge_model_settings( + legacy: Optional[Dict[str, Any]], + current: Optional[Dict[str, Any]], +) -> Dict[str, Any]: + """Merge legacy and provider-scoped settings with current values winning.""" + result = dict(legacy or {}) + current = current or {} + legacy_parameters = result.get("default_parameters") + current_parameters = current.get("default_parameters") + if isinstance(legacy_parameters, dict) or isinstance(current_parameters, dict): + merged_parameters = dict( + legacy_parameters if isinstance(legacy_parameters, dict) else {} + ) + if isinstance(current_parameters, dict): + merged_parameters.update(current_parameters) + result["default_parameters"] = merged_parameters + for field in ("enabled", "credential_id"): + if field in current: + result[field] = current[field] + return result + + +def _get_model_setting_from_data( + data: Dict[str, Any], + provider_id: str, + model_id: str, +) -> Dict[str, Any]: + """Resolve legacy and provider-scoped settings from already-read data.""" + key = f"{provider_id}/{model_id}" + legacy_settings = data.get("model_settings") + legacy = legacy_settings.get(key) if isinstance(legacy_settings, dict) else None + + providers = data.get("provider") + provider = providers.get(provider_id) if isinstance(providers, dict) else None + models = provider.get("models") if isinstance(provider, dict) else None + model = models.get(model_id) if isinstance(models, dict) else None + return _merge_model_settings(legacy, _extract_model_setting(model)) + + def _get_example_config_dir() -> Path: """Return the bundled example directory used for first-run initialization.""" return Path(__file__).resolve().parents[2] / ".flocks" @@ -367,7 +426,12 @@ def add_model( if "models" not in pconfig: pconfig["models"] = {} - pconfig["models"][model_id] = model_config + existing_model = pconfig["models"].get(model_id, {}) + preserved_settings = _extract_model_setting(existing_model) + merged_model = dict(model_config) + for field, value in preserved_settings.items(): + merged_model.setdefault(field, value) + pconfig["models"][model_id] = merged_model data["provider"][provider_id] = pconfig cls._write_raw(data) log.info("config_writer.model_added", { @@ -403,16 +467,15 @@ def remove_model(cls, provider_id: str, model_id: str) -> bool: return True # ------------------------------------------------------------------ - # Model settings (model_settings section) + # Model settings (provider-scoped with legacy read compatibility) # ------------------------------------------------------------------ @classmethod def get_model_setting(cls, provider_id: str, model_id: str) -> Optional[Dict[str, Any]]: - """Get setting for a specific model from flocks.json model_settings section.""" + """Get model settings, preferring the provider-scoped model entry.""" data = cls._read_raw() - settings = data.get("model_settings", {}) - key = f"{provider_id}/{model_id}" - return settings.get(key) + merged = _get_model_setting_from_data(data, provider_id, model_id) + return merged or None @classmethod def set_model_setting( @@ -421,14 +484,55 @@ def set_model_setting( model_id: str, setting: Dict[str, Any], ) -> None: - """Set or update model setting in flocks.json model_settings section.""" + """Set model settings inside provider..models..""" data = cls._read_raw() - if "model_settings" not in data: - data["model_settings"] = {} - key = f"{provider_id}/{model_id}" - existing = data["model_settings"].get(key, {}) - existing.update(setting) - data["model_settings"][key] = existing + providers = data.get("provider") + if not isinstance(providers, dict): + providers = {} + provider = providers.get(provider_id) + if not isinstance(provider, dict): + provider = {} + models = provider.get("models") + if not isinstance(models, dict): + models = {} + model = models.get(model_id) + if not isinstance(model, dict): + model = {} + + # A write to a legacy-only setting migrates its complete effective value + # into the provider model. Untouched legacy entries remain readable. + existing_setting = _get_model_setting_from_data(data, provider_id, model_id) + for field, value in existing_setting.items(): + if field in _MODEL_SETTING_FIELDS: + model[field] = value + + for field, value in setting.items(): + if field not in _MODEL_SETTING_FIELDS: + continue + if field == "default_parameters" and isinstance(value, dict): + existing_parameters = model.get("default_parameters") + merged_parameters = dict( + existing_parameters + if isinstance(existing_parameters, dict) + else {} + ) + merged_parameters.update(value) + model[field] = merged_parameters + else: + model[field] = value + models[model_id] = model + provider["models"] = models + providers[provider_id] = provider + data["provider"] = providers + + legacy_settings = data.get("model_settings") + legacy_key = f"{provider_id}/{model_id}" + if isinstance(legacy_settings, dict) and legacy_key in legacy_settings: + legacy_settings.pop(legacy_key) + if legacy_settings: + data["model_settings"] = legacy_settings + else: + data.pop("model_settings", None) cls._write_raw(data) log.info("config_writer.model_setting_updated", { "provider_id": provider_id, @@ -437,14 +541,36 @@ def set_model_setting( @classmethod def remove_model_setting(cls, provider_id: str, model_id: str) -> bool: - """Remove a model setting from flocks.json.""" + """Remove provider-scoped and legacy settings for a model.""" data = cls._read_raw() - settings = data.get("model_settings", {}) + removed = False + providers = data.get("provider") + provider = providers.get(provider_id, {}) if isinstance(providers, dict) else {} + models = provider.get("models") if isinstance(provider, dict) else None + model = models.get(model_id) if isinstance(models, dict) else None + if isinstance(model, dict): + for field in _MODEL_SETTING_FIELDS: + if field in model: + del model[field] + removed = True + if removed and not model: + models.pop(model_id, None) + if not models: + provider.pop("models", None) + if not provider: + if isinstance(providers, dict): + providers.pop(provider_id, None) + settings = data.get("model_settings") key = f"{provider_id}/{model_id}" - if key not in settings: + if isinstance(settings, dict) and key in settings: + del settings[key] + removed = True + if settings: + data["model_settings"] = settings + else: + data.pop("model_settings", None) + if not removed: return False - del settings[key] - data["model_settings"] = settings cls._write_raw(data) return True @@ -452,7 +578,54 @@ def remove_model_setting(cls, provider_id: str, model_id: str) -> bool: def get_all_model_settings(cls) -> Dict[str, Dict[str, Any]]: """Get all model settings. Returns dict keyed by 'provider_id/model_id'.""" data = cls._read_raw() - return data.get("model_settings", {}) + legacy_settings = data.get("model_settings") + result = { + key: dict(value) + for key, value in ( + legacy_settings.items() + if isinstance(legacy_settings, dict) + else () + ) + if isinstance(value, dict) + } + providers = data.get("provider") + provider_items = providers.items() if isinstance(providers, dict) else () + for provider_id, provider in provider_items: + if not isinstance(provider, dict): + continue + models = provider.get("models") + model_items = models.items() if isinstance(models, dict) else () + for model_id, model in model_items: + current = _extract_model_setting(model) + if not current: + continue + key = f"{provider_id}/{model_id}" + result[key] = _merge_model_settings(result.get(key), current) + return result + + @classmethod + def get_effective_model_default_parameters( + cls, + provider_id: str, + model_id: str, + ) -> Dict[str, Any]: + """Resolve global, legacy, and provider-model parameter defaults.""" + data = cls._read_raw() + result: Dict[str, Any] = {} + default_models = data.get("default_models") + global_parameters = ( + default_models.get("default_parameters", {}) + if isinstance(default_models, dict) + else {} + ) + if isinstance(global_parameters, dict): + result.update(global_parameters) + + setting = _get_model_setting_from_data(data, provider_id, model_id) + model_parameters = setting.get("default_parameters", {}) + if isinstance(model_parameters, dict): + result.update(model_parameters) + return result # ------------------------------------------------------------------ # Default models (default_models section) @@ -503,7 +676,14 @@ def delete_default_model(cls, model_type: str) -> bool: def get_all_default_models(cls) -> Dict[str, Dict[str, Any]]: """Get all default model configs.""" data = cls._read_raw() - return data.get("default_models", {}) + defaults = data.get("default_models") + if not isinstance(defaults, dict): + return {} + return { + model_type: config + for model_type, config in defaults.items() + if model_type != "default_parameters" and isinstance(config, dict) + } # ------------------------------------------------------------------ # Runtime model fallbacks (fallback_providers section) @@ -712,8 +892,8 @@ def remove_api_service(cls, service_id: str) -> bool: # ------------------------------------------------------------------ # # User-level overlay for per-tool settings (currently: ``enabled``). - # The section mirrors ``model_settings`` for naming consistency — - # both are flat maps keyed by the entity's unique id. + # This remains a flat map keyed by tool name; model settings now live + # directly under their provider model entries. # # Why this exists: YAML plugin tool files under # ``/.flocks/plugins/tools/`` are tracked by git and may be diff --git a/flocks/provider/model_manager.py b/flocks/provider/model_manager.py index bb4e813cb..cd897d955 100644 --- a/flocks/provider/model_manager.py +++ b/flocks/provider/model_manager.py @@ -139,6 +139,17 @@ def update_setting( model_id=model_id, ) + def get_effective_default_parameters( + self, + provider_id: str, + model_id: str, + ) -> Dict[str, Any]: + """Get inherited default parameters for a provider model.""" + return ConfigWriter.get_effective_model_default_parameters( + provider_id, + model_id, + ) + # ==================== Default Models ==================== def get_default_model( diff --git a/flocks/provider/options.py b/flocks/provider/options.py index 733093df5..bea626cb2 100644 --- a/flocks/provider/options.py +++ b/flocks/provider/options.py @@ -29,7 +29,8 @@ # --------------------------------------------------------------------------- DEFAULT_THINKING_BUDGET = 16000 DEFAULT_OUTPUT_BUFFER = 8192 -DEFAULT_KIMI_K3_REASONING_EFFORT = "max" +DEFAULT_REASONING_EFFORT = "high" +DEFAULT_KIMI_K3_REASONING_EFFORT = DEFAULT_REASONING_EFFORT KIMI_K3_REASONING_EFFORTS = frozenset({"low", "high", "max"}) _GENERIC_CHAT_REASONING_EXTRA_BODY_KEYS = { @@ -76,15 +77,15 @@ def _resolve_reasoning_enabled(provider_id: str, model_id: str) -> Optional[bool def _resolve_reasoning_effort(provider_id: str, model_id: str) -> Optional[str]: - """Read a model-level reasoning effort from flocks.json.""" + """Read the effective reasoning effort from flocks.json.""" try: from flocks.provider.model_manager import get_model_manager - setting = get_model_manager().get_setting(provider_id, model_id) - if not setting: - return None - - value = (setting.default_parameters or {}).get("reasoning_effort") + default_parameters = get_model_manager().get_effective_default_parameters( + provider_id, + model_id, + ) + value = default_parameters.get("reasoning_effort") return value.strip().lower() if isinstance(value, str) else None except Exception as exc: log.debug("options.reasoning_effort_setting_lookup_failed", { @@ -370,7 +371,7 @@ def build_provider_options( # -- OpenAI reasoning (o1 / o3 / gpt-5) -------------------------------- elif provider_id == "openai": if reasoning_enabled is not False and any(tag in model_lower for tag in ("o1", "o3", "gpt-5")): - options["reasoningEffort"] = "medium" + options["reasoningEffort"] = reasoning_effort or DEFAULT_REASONING_EFFORT # -- Google Gemini thinking --------------------------------------------- elif provider_id == "google": diff --git a/tests/config/test_config_writer.py b/tests/config/test_config_writer.py index 73c0b1af6..4355508b1 100644 --- a/tests/config/test_config_writer.py +++ b/tests/config/test_config_writer.py @@ -256,7 +256,7 @@ def test_no_config_file(self, tmp_path, monkeypatch): class TestConfigWriterModelSettings: - """Test model_settings section CRUD.""" + """Test provider-scoped model settings and legacy compatibility.""" def test_get_model_setting_empty(self, temp_project): from flocks.config.config_writer import ConfigWriter @@ -273,6 +273,12 @@ def test_set_and_get_model_setting(self, temp_project): assert setting is not None assert setting["enabled"] is False assert setting["default_parameters"]["temperature"] == 0.5 + data = ConfigWriter._read_raw() + assert "model_settings" not in data + assert data["provider"]["openai"]["models"]["gpt-4o"] == { + "enabled": False, + "default_parameters": {"temperature": 0.5}, + } def test_update_model_setting_merges(self, temp_project): from flocks.config.config_writer import ConfigWriter @@ -294,6 +300,24 @@ def test_remove_model_setting(self, temp_project): ConfigWriter.set_model_setting("openai", "gpt-4o", {"enabled": True}) assert ConfigWriter.remove_model_setting("openai", "gpt-4o") is True assert ConfigWriter.get_model_setting("openai", "gpt-4o") is None + assert "openai" not in ConfigWriter._read_raw()["provider"] + + def test_remove_model_setting_preserves_model_definition(self, temp_project): + from flocks.config.config_writer import ConfigWriter + + ConfigWriter.set_model_setting( + "anthropic", + "claude-sonnet-4-5", + {"enabled": False}, + ) + + assert ConfigWriter.remove_model_setting( + "anthropic", + "claude-sonnet-4-5", + ) is True + assert ConfigWriter.get_provider_raw("anthropic")["models"] == { + "claude-sonnet-4-5": {"name": "Claude Sonnet 4.5"} + } def test_remove_nonexistent_model_setting(self, temp_project): from flocks.config.config_writer import ConfigWriter @@ -309,7 +333,151 @@ def test_get_all_model_settings(self, temp_project): assert "anthropic/claude-sonnet" in all_settings assert len(all_settings) == 2 - def test_model_settings_preserve_other_sections(self, temp_project): + def test_legacy_model_setting_remains_readable(self, temp_project): + from flocks.config.config_writer import ConfigWriter + + data = ConfigWriter._read_raw() + data["model_settings"] = { + "anthropic/claude-sonnet-4-5": { + "enabled": False, + "default_parameters": {"reasoning_effort": "high"}, + } + } + ConfigWriter._write_raw(data) + + setting = ConfigWriter.get_model_setting( + "anthropic", + "claude-sonnet-4-5", + ) + + assert setting == { + "enabled": False, + "default_parameters": {"reasoning_effort": "high"}, + } + + def test_provider_model_setting_overrides_legacy_values(self, temp_project): + from flocks.config.config_writer import ConfigWriter + + data = ConfigWriter._read_raw() + data["model_settings"] = { + "anthropic/claude-sonnet-4-5": { + "enabled": False, + "default_parameters": { + "reasoning_effort": "high", + "temperature": 0.4, + }, + } + } + data["provider"]["anthropic"]["models"]["claude-sonnet-4-5"].update( + { + "enabled": True, + "default_parameters": {"reasoning_effort": "max"}, + } + ) + ConfigWriter._write_raw(data) + + setting = ConfigWriter.get_model_setting( + "anthropic", + "claude-sonnet-4-5", + ) + + assert setting == { + "enabled": True, + "default_parameters": { + "reasoning_effort": "max", + "temperature": 0.4, + }, + } + + def test_updating_legacy_setting_migrates_it_to_provider_model(self, temp_project): + from flocks.config.config_writer import ConfigWriter + + data = ConfigWriter._read_raw() + data["model_settings"] = { + "anthropic/claude-sonnet-4-5": { + "enabled": False, + "default_parameters": { + "temperature": 0.4, + "reasoning_effort": "high", + }, + } + } + ConfigWriter._write_raw(data) + + ConfigWriter.set_model_setting( + "anthropic", + "claude-sonnet-4-5", + {"default_parameters": {"reasoning_effort": "max"}}, + ) + + data = ConfigWriter._read_raw() + assert data["provider"]["anthropic"]["models"]["claude-sonnet-4-5"] == { + "name": "Claude Sonnet 4.5", + "enabled": False, + "default_parameters": { + "temperature": 0.4, + "reasoning_effort": "max", + }, + } + assert "model_settings" not in data + + def test_effective_default_parameters_follow_scope_precedence(self, temp_project): + from flocks.config.config_writer import ConfigWriter + + data = ConfigWriter._read_raw() + data["default_models"] = { + "default_parameters": { + "reasoning_effort": "low", + "temperature": 0.1, + }, + } + data["model_settings"] = { + "anthropic/claude-sonnet-4-5": { + "default_parameters": { + "reasoning_effort": "high", + } + } + } + data["provider"]["anthropic"]["models"]["claude-sonnet-4-5"]["default_parameters"] = { + "reasoning_effort": "max", + } + ConfigWriter._write_raw(data) + + parameters = ConfigWriter.get_effective_model_default_parameters( + "anthropic", + "claude-sonnet-4-5", + ) + + assert parameters == { + "reasoning_effort": "max", + "temperature": 0.1, + } + + def test_add_model_preserves_provider_scoped_settings(self, temp_project): + from flocks.config.config_writer import ConfigWriter + + ConfigWriter.set_model_setting( + "anthropic", + "claude-sonnet-4-5", + { + "enabled": False, + "default_parameters": {"reasoning_effort": "high"}, + }, + ) + ConfigWriter.add_model( + "anthropic", + "claude-sonnet-4-5", + {"name": "Updated Claude"}, + ) + + model = ConfigWriter.get_provider_raw("anthropic")["models"]["claude-sonnet-4-5"] + assert model == { + "name": "Updated Claude", + "enabled": False, + "default_parameters": {"reasoning_effort": "high"}, + } + + def test_provider_model_settings_preserve_other_sections(self, temp_project): from flocks.config.config_writer import ConfigWriter ConfigWriter.set_model_setting("openai", "gpt-4o", {"enabled": True}) @@ -439,6 +607,26 @@ def test_get_all_default_models(self, temp_project): assert "text-embedding" in all_defaults assert len(all_defaults) == 2 + def test_get_all_default_models_excludes_default_parameters(self, temp_project): + from flocks.config.config_writer import ConfigWriter + + data = ConfigWriter._read_raw() + data["default_models"] = { + "default_parameters": {"reasoning_effort": "medium"}, + "llm": { + "provider_id": "anthropic", + "model_id": "claude-sonnet", + }, + } + ConfigWriter._write_raw(data) + + assert ConfigWriter.get_all_default_models() == { + "llm": { + "provider_id": "anthropic", + "model_id": "claude-sonnet", + } + } + def test_default_models_preserve_other_sections(self, temp_project): from flocks.config.config_writer import ConfigWriter ConfigWriter.set_default_model("llm", "anthropic", "claude") diff --git a/tests/provider/test_model_management_p2p3.py b/tests/provider/test_model_management_p2p3.py index e4ff4ea61..a86e26ce8 100644 --- a/tests/provider/test_model_management_p2p3.py +++ b/tests/provider/test_model_management_p2p3.py @@ -335,9 +335,8 @@ def test_settings_persisted_in_flocks_json(self, temp_project): # Verify it's in flocks.json data = ConfigWriter._read_raw() - assert "model_settings" in data - assert "openai/gpt-4o" in data["model_settings"] - assert data["model_settings"]["openai/gpt-4o"]["enabled"] is False + assert "model_settings" not in data + assert data["provider"]["openai"]["models"]["gpt-4o"]["enabled"] is False def test_default_model_persisted_in_flocks_json(self, temp_project): """Verify that default models are persisted in flocks.json.""" diff --git a/tests/provider/test_provider_options.py b/tests/provider/test_provider_options.py index 45f2af82b..7ae8d17fd 100644 --- a/tests/provider/test_provider_options.py +++ b/tests/provider/test_provider_options.py @@ -86,7 +86,7 @@ def test_moonshot_kimi_k3_uses_default_reasoning_effort(self): resolve_max_tokens=False, ) - assert options["extra_body"] == {"reasoning_effort": "max"} + assert options["extra_body"] == {"reasoning_effort": "high"} def test_kimi_k27_forces_thinking_even_when_toggle_is_disabled(self): options = provider_options.build_provider_options( @@ -127,7 +127,7 @@ def test_kimi_k3_uses_reasoning_effort_instead_of_thinking(self): resolve_max_tokens=False, ) - assert options["extra_body"] == {"reasoning_effort": "max"} + assert options["extra_body"] == {"reasoning_effort": "high"} assert "thinking" not in options["extra_body"] def test_kimi_k3_respects_supported_reasoning_effort(self): @@ -443,3 +443,27 @@ def test_openai_reasoning_can_be_disabled(self): ) assert "reasoningEffort" not in options + + def test_openai_reasoning_defaults_to_high(self): + options = provider_options.build_provider_options( + "openai", + "gpt-5.4", + resolve_max_tokens=False, + ) + + assert options["reasoningEffort"] == "high" + + def test_openai_uses_configured_reasoning_effort(self, monkeypatch): + monkeypatch.setattr( + provider_options, + "_resolve_reasoning_effort", + lambda *_args: "low", + ) + + options = provider_options.build_provider_options( + "openai", + "gpt-5.4", + resolve_max_tokens=False, + ) + + assert options["reasoningEffort"] == "low" diff --git a/tests/provider/test_thinking_params.py b/tests/provider/test_thinking_params.py index d9c638214..046564d37 100644 --- a/tests/provider/test_thinking_params.py +++ b/tests/provider/test_thinking_params.py @@ -100,7 +100,7 @@ def _expected_generic_chat_extra_body( if "mimo" in model_lower: return MIMO_THINKING_EXTRA_BODY if is_kimi_k3_model(model_id): - return {"reasoning_effort": "max"} + return {"reasoning_effort": "high"} if is_kimi_k27_code_model(model_id): return KIMI_THINKING_EXTRA_BODY if "kimi" in model_lower: @@ -444,7 +444,7 @@ def test_anthropic_transport_still_uses_thinking_field( ("kimi-k2.6-uncatalogued", KIMI_THINKING_EXTRA_BODY), ("kimi-k2.7-code", KIMI_THINKING_EXTRA_BODY), ("kimi-k2.7-code-highspeed", KIMI_THINKING_EXTRA_BODY), - ("kimi-k3", {"reasoning_effort": "max"}), + ("kimi-k3", {"reasoning_effort": "high"}), ("mimo-v2.5-pro-uncatalogued", MIMO_THINKING_EXTRA_BODY), ("minimax-m4-uncatalogued", {"reasoning_split": True}), ("step-3.5-flash-uncatalogued", {"enable_thinking": True}), diff --git a/webui/src/pages/Model/index.tsx b/webui/src/pages/Model/index.tsx index 959ac8ede..ba81c6c05 100644 --- a/webui/src/pages/Model/index.tsx +++ b/webui/src/pages/Model/index.tsx @@ -2841,33 +2841,31 @@ function ModelDetailSheet({ const handleSave = async () => { setLoading(true); try { - await Promise.all([ - modelV2API.createDefinition(provider.id, { - model_id: model.id, - name: name.trim() || model.id, - context_window: parseInt(contextWindow) || undefined, - max_output_tokens: parseInt(maxOutput) || undefined, - supports_vision: supportsVision, - supports_tools: supportsTools, - supports_streaming: supportsStreaming, - supports_reasoning: modelSupportsReasoning ? modelSupportsReasoning : supportsReasoning, - input_price: parseFloat(inputPrice) || 0, - output_price: parseFloat(outputPrice) || 0, - cache_read_price: cacheReadPrice.trim() === '' - ? null - : parseFloat(cacheReadPrice) || 0, - currency, - }), - modelSettingsAPI.update(provider.id, model.id, { - enabled, - default_parameters: modelSupportsReasoning - ? { - ...defaultParameters, - enable_thinking: supportsReasoning, - } - : undefined, - }), - ]); + await modelV2API.createDefinition(provider.id, { + model_id: model.id, + name: name.trim() || model.id, + context_window: parseInt(contextWindow) || undefined, + max_output_tokens: parseInt(maxOutput) || undefined, + supports_vision: supportsVision, + supports_tools: supportsTools, + supports_streaming: supportsStreaming, + supports_reasoning: modelSupportsReasoning ? modelSupportsReasoning : supportsReasoning, + input_price: parseFloat(inputPrice) || 0, + output_price: parseFloat(outputPrice) || 0, + cache_read_price: cacheReadPrice.trim() === '' + ? null + : parseFloat(cacheReadPrice) || 0, + currency, + }); + await modelSettingsAPI.update(provider.id, model.id, { + enabled, + default_parameters: modelSupportsReasoning + ? { + ...defaultParameters, + enable_thinking: supportsReasoning, + } + : undefined, + }); toast.success(t('credentialsSaved')); onSaved(); } catch (e: any) { From 96e58a6f98a444349f4075f35567312d880c4747 Mon Sep 17 00:00:00 2001 From: xiami762 Date: Mon, 10 Aug 2026 15:04:17 +0800 Subject: [PATCH 2/2] refactor(config): limit changes to reasoning effort --- flocks/config/config_writer.py | 190 +++---------------- tests/config/test_config_writer.py | 145 +------------- tests/provider/test_model_management_p2p3.py | 5 +- webui/src/pages/Model/index.tsx | 52 ++--- 4 files changed, 57 insertions(+), 335 deletions(-) diff --git a/flocks/config/config_writer.py b/flocks/config/config_writer.py index 62ddfe221..51129ba11 100644 --- a/flocks/config/config_writer.py +++ b/flocks/config/config_writer.py @@ -36,59 +36,6 @@ }, } -_MODEL_SETTING_FIELDS = ("enabled", "credential_id", "default_parameters") - - -def _extract_model_setting(model_config: Any) -> Dict[str, Any]: - """Extract user-setting fields from a provider model entry.""" - if not isinstance(model_config, dict): - return {} - return { - field: model_config[field] - for field in _MODEL_SETTING_FIELDS - if field in model_config - } - - -def _merge_model_settings( - legacy: Optional[Dict[str, Any]], - current: Optional[Dict[str, Any]], -) -> Dict[str, Any]: - """Merge legacy and provider-scoped settings with current values winning.""" - result = dict(legacy or {}) - current = current or {} - legacy_parameters = result.get("default_parameters") - current_parameters = current.get("default_parameters") - if isinstance(legacy_parameters, dict) or isinstance(current_parameters, dict): - merged_parameters = dict( - legacy_parameters if isinstance(legacy_parameters, dict) else {} - ) - if isinstance(current_parameters, dict): - merged_parameters.update(current_parameters) - result["default_parameters"] = merged_parameters - for field in ("enabled", "credential_id"): - if field in current: - result[field] = current[field] - return result - - -def _get_model_setting_from_data( - data: Dict[str, Any], - provider_id: str, - model_id: str, -) -> Dict[str, Any]: - """Resolve legacy and provider-scoped settings from already-read data.""" - key = f"{provider_id}/{model_id}" - legacy_settings = data.get("model_settings") - legacy = legacy_settings.get(key) if isinstance(legacy_settings, dict) else None - - providers = data.get("provider") - provider = providers.get(provider_id) if isinstance(providers, dict) else None - models = provider.get("models") if isinstance(provider, dict) else None - model = models.get(model_id) if isinstance(models, dict) else None - return _merge_model_settings(legacy, _extract_model_setting(model)) - - def _get_example_config_dir() -> Path: """Return the bundled example directory used for first-run initialization.""" return Path(__file__).resolve().parents[2] / ".flocks" @@ -426,12 +373,7 @@ def add_model( if "models" not in pconfig: pconfig["models"] = {} - existing_model = pconfig["models"].get(model_id, {}) - preserved_settings = _extract_model_setting(existing_model) - merged_model = dict(model_config) - for field, value in preserved_settings.items(): - merged_model.setdefault(field, value) - pconfig["models"][model_id] = merged_model + pconfig["models"][model_id] = model_config data["provider"][provider_id] = pconfig cls._write_raw(data) log.info("config_writer.model_added", { @@ -467,15 +409,16 @@ def remove_model(cls, provider_id: str, model_id: str) -> bool: return True # ------------------------------------------------------------------ - # Model settings (provider-scoped with legacy read compatibility) + # Model settings (model_settings section) # ------------------------------------------------------------------ @classmethod def get_model_setting(cls, provider_id: str, model_id: str) -> Optional[Dict[str, Any]]: - """Get model settings, preferring the provider-scoped model entry.""" + """Get setting for a specific model from flocks.json model_settings section.""" data = cls._read_raw() - merged = _get_model_setting_from_data(data, provider_id, model_id) - return merged or None + settings = data.get("model_settings", {}) + key = f"{provider_id}/{model_id}" + return settings.get(key) @classmethod def set_model_setting( @@ -484,55 +427,14 @@ def set_model_setting( model_id: str, setting: Dict[str, Any], ) -> None: - """Set model settings inside provider..models..""" + """Set or update model setting in flocks.json model_settings section.""" data = cls._read_raw() - providers = data.get("provider") - if not isinstance(providers, dict): - providers = {} - provider = providers.get(provider_id) - if not isinstance(provider, dict): - provider = {} - models = provider.get("models") - if not isinstance(models, dict): - models = {} - model = models.get(model_id) - if not isinstance(model, dict): - model = {} - - # A write to a legacy-only setting migrates its complete effective value - # into the provider model. Untouched legacy entries remain readable. - existing_setting = _get_model_setting_from_data(data, provider_id, model_id) - for field, value in existing_setting.items(): - if field in _MODEL_SETTING_FIELDS: - model[field] = value - - for field, value in setting.items(): - if field not in _MODEL_SETTING_FIELDS: - continue - if field == "default_parameters" and isinstance(value, dict): - existing_parameters = model.get("default_parameters") - merged_parameters = dict( - existing_parameters - if isinstance(existing_parameters, dict) - else {} - ) - merged_parameters.update(value) - model[field] = merged_parameters - else: - model[field] = value - models[model_id] = model - provider["models"] = models - providers[provider_id] = provider - data["provider"] = providers - - legacy_settings = data.get("model_settings") - legacy_key = f"{provider_id}/{model_id}" - if isinstance(legacy_settings, dict) and legacy_key in legacy_settings: - legacy_settings.pop(legacy_key) - if legacy_settings: - data["model_settings"] = legacy_settings - else: - data.pop("model_settings", None) + if "model_settings" not in data: + data["model_settings"] = {} + key = f"{provider_id}/{model_id}" + existing = data["model_settings"].get(key, {}) + existing.update(setting) + data["model_settings"][key] = existing cls._write_raw(data) log.info("config_writer.model_setting_updated", { "provider_id": provider_id, @@ -541,36 +443,14 @@ def set_model_setting( @classmethod def remove_model_setting(cls, provider_id: str, model_id: str) -> bool: - """Remove provider-scoped and legacy settings for a model.""" + """Remove a model setting from flocks.json.""" data = cls._read_raw() - removed = False - providers = data.get("provider") - provider = providers.get(provider_id, {}) if isinstance(providers, dict) else {} - models = provider.get("models") if isinstance(provider, dict) else None - model = models.get(model_id) if isinstance(models, dict) else None - if isinstance(model, dict): - for field in _MODEL_SETTING_FIELDS: - if field in model: - del model[field] - removed = True - if removed and not model: - models.pop(model_id, None) - if not models: - provider.pop("models", None) - if not provider: - if isinstance(providers, dict): - providers.pop(provider_id, None) - settings = data.get("model_settings") + settings = data.get("model_settings", {}) key = f"{provider_id}/{model_id}" - if isinstance(settings, dict) and key in settings: - del settings[key] - removed = True - if settings: - data["model_settings"] = settings - else: - data.pop("model_settings", None) - if not removed: + if key not in settings: return False + del settings[key] + data["model_settings"] = settings cls._write_raw(data) return True @@ -578,30 +458,7 @@ def remove_model_setting(cls, provider_id: str, model_id: str) -> bool: def get_all_model_settings(cls) -> Dict[str, Dict[str, Any]]: """Get all model settings. Returns dict keyed by 'provider_id/model_id'.""" data = cls._read_raw() - legacy_settings = data.get("model_settings") - result = { - key: dict(value) - for key, value in ( - legacy_settings.items() - if isinstance(legacy_settings, dict) - else () - ) - if isinstance(value, dict) - } - providers = data.get("provider") - provider_items = providers.items() if isinstance(providers, dict) else () - for provider_id, provider in provider_items: - if not isinstance(provider, dict): - continue - models = provider.get("models") - model_items = models.items() if isinstance(models, dict) else () - for model_id, model in model_items: - current = _extract_model_setting(model) - if not current: - continue - key = f"{provider_id}/{model_id}" - result[key] = _merge_model_settings(result.get(key), current) - return result + return data.get("model_settings", {}) @classmethod def get_effective_model_default_parameters( @@ -609,7 +466,7 @@ def get_effective_model_default_parameters( provider_id: str, model_id: str, ) -> Dict[str, Any]: - """Resolve global, legacy, and provider-model parameter defaults.""" + """Resolve global defaults with model-specific overrides.""" data = cls._read_raw() result: Dict[str, Any] = {} default_models = data.get("default_models") @@ -621,7 +478,8 @@ def get_effective_model_default_parameters( if isinstance(global_parameters, dict): result.update(global_parameters) - setting = _get_model_setting_from_data(data, provider_id, model_id) + settings = data.get("model_settings", {}) + setting = settings.get(f"{provider_id}/{model_id}", {}) model_parameters = setting.get("default_parameters", {}) if isinstance(model_parameters, dict): result.update(model_parameters) @@ -892,8 +750,8 @@ def remove_api_service(cls, service_id: str) -> bool: # ------------------------------------------------------------------ # # User-level overlay for per-tool settings (currently: ``enabled``). - # This remains a flat map keyed by tool name; model settings now live - # directly under their provider model entries. + # The section mirrors ``model_settings`` for naming consistency — + # both are flat maps keyed by the entity's unique id. # # Why this exists: YAML plugin tool files under # ``/.flocks/plugins/tools/`` are tracked by git and may be diff --git a/tests/config/test_config_writer.py b/tests/config/test_config_writer.py index 4355508b1..b735f0a06 100644 --- a/tests/config/test_config_writer.py +++ b/tests/config/test_config_writer.py @@ -256,7 +256,7 @@ def test_no_config_file(self, tmp_path, monkeypatch): class TestConfigWriterModelSettings: - """Test provider-scoped model settings and legacy compatibility.""" + """Test model_settings section CRUD.""" def test_get_model_setting_empty(self, temp_project): from flocks.config.config_writer import ConfigWriter @@ -273,12 +273,6 @@ def test_set_and_get_model_setting(self, temp_project): assert setting is not None assert setting["enabled"] is False assert setting["default_parameters"]["temperature"] == 0.5 - data = ConfigWriter._read_raw() - assert "model_settings" not in data - assert data["provider"]["openai"]["models"]["gpt-4o"] == { - "enabled": False, - "default_parameters": {"temperature": 0.5}, - } def test_update_model_setting_merges(self, temp_project): from flocks.config.config_writer import ConfigWriter @@ -300,24 +294,6 @@ def test_remove_model_setting(self, temp_project): ConfigWriter.set_model_setting("openai", "gpt-4o", {"enabled": True}) assert ConfigWriter.remove_model_setting("openai", "gpt-4o") is True assert ConfigWriter.get_model_setting("openai", "gpt-4o") is None - assert "openai" not in ConfigWriter._read_raw()["provider"] - - def test_remove_model_setting_preserves_model_definition(self, temp_project): - from flocks.config.config_writer import ConfigWriter - - ConfigWriter.set_model_setting( - "anthropic", - "claude-sonnet-4-5", - {"enabled": False}, - ) - - assert ConfigWriter.remove_model_setting( - "anthropic", - "claude-sonnet-4-5", - ) is True - assert ConfigWriter.get_provider_raw("anthropic")["models"] == { - "claude-sonnet-4-5": {"name": "Claude Sonnet 4.5"} - } def test_remove_nonexistent_model_setting(self, temp_project): from flocks.config.config_writer import ConfigWriter @@ -333,94 +309,6 @@ def test_get_all_model_settings(self, temp_project): assert "anthropic/claude-sonnet" in all_settings assert len(all_settings) == 2 - def test_legacy_model_setting_remains_readable(self, temp_project): - from flocks.config.config_writer import ConfigWriter - - data = ConfigWriter._read_raw() - data["model_settings"] = { - "anthropic/claude-sonnet-4-5": { - "enabled": False, - "default_parameters": {"reasoning_effort": "high"}, - } - } - ConfigWriter._write_raw(data) - - setting = ConfigWriter.get_model_setting( - "anthropic", - "claude-sonnet-4-5", - ) - - assert setting == { - "enabled": False, - "default_parameters": {"reasoning_effort": "high"}, - } - - def test_provider_model_setting_overrides_legacy_values(self, temp_project): - from flocks.config.config_writer import ConfigWriter - - data = ConfigWriter._read_raw() - data["model_settings"] = { - "anthropic/claude-sonnet-4-5": { - "enabled": False, - "default_parameters": { - "reasoning_effort": "high", - "temperature": 0.4, - }, - } - } - data["provider"]["anthropic"]["models"]["claude-sonnet-4-5"].update( - { - "enabled": True, - "default_parameters": {"reasoning_effort": "max"}, - } - ) - ConfigWriter._write_raw(data) - - setting = ConfigWriter.get_model_setting( - "anthropic", - "claude-sonnet-4-5", - ) - - assert setting == { - "enabled": True, - "default_parameters": { - "reasoning_effort": "max", - "temperature": 0.4, - }, - } - - def test_updating_legacy_setting_migrates_it_to_provider_model(self, temp_project): - from flocks.config.config_writer import ConfigWriter - - data = ConfigWriter._read_raw() - data["model_settings"] = { - "anthropic/claude-sonnet-4-5": { - "enabled": False, - "default_parameters": { - "temperature": 0.4, - "reasoning_effort": "high", - }, - } - } - ConfigWriter._write_raw(data) - - ConfigWriter.set_model_setting( - "anthropic", - "claude-sonnet-4-5", - {"default_parameters": {"reasoning_effort": "max"}}, - ) - - data = ConfigWriter._read_raw() - assert data["provider"]["anthropic"]["models"]["claude-sonnet-4-5"] == { - "name": "Claude Sonnet 4.5", - "enabled": False, - "default_parameters": { - "temperature": 0.4, - "reasoning_effort": "max", - }, - } - assert "model_settings" not in data - def test_effective_default_parameters_follow_scope_precedence(self, temp_project): from flocks.config.config_writer import ConfigWriter @@ -438,9 +326,6 @@ def test_effective_default_parameters_follow_scope_precedence(self, temp_project } } } - data["provider"]["anthropic"]["models"]["claude-sonnet-4-5"]["default_parameters"] = { - "reasoning_effort": "max", - } ConfigWriter._write_raw(data) parameters = ConfigWriter.get_effective_model_default_parameters( @@ -449,35 +334,11 @@ def test_effective_default_parameters_follow_scope_precedence(self, temp_project ) assert parameters == { - "reasoning_effort": "max", + "reasoning_effort": "high", "temperature": 0.1, } - def test_add_model_preserves_provider_scoped_settings(self, temp_project): - from flocks.config.config_writer import ConfigWriter - - ConfigWriter.set_model_setting( - "anthropic", - "claude-sonnet-4-5", - { - "enabled": False, - "default_parameters": {"reasoning_effort": "high"}, - }, - ) - ConfigWriter.add_model( - "anthropic", - "claude-sonnet-4-5", - {"name": "Updated Claude"}, - ) - - model = ConfigWriter.get_provider_raw("anthropic")["models"]["claude-sonnet-4-5"] - assert model == { - "name": "Updated Claude", - "enabled": False, - "default_parameters": {"reasoning_effort": "high"}, - } - - def test_provider_model_settings_preserve_other_sections(self, temp_project): + def test_model_settings_preserve_other_sections(self, temp_project): from flocks.config.config_writer import ConfigWriter ConfigWriter.set_model_setting("openai", "gpt-4o", {"enabled": True}) diff --git a/tests/provider/test_model_management_p2p3.py b/tests/provider/test_model_management_p2p3.py index a86e26ce8..e4ff4ea61 100644 --- a/tests/provider/test_model_management_p2p3.py +++ b/tests/provider/test_model_management_p2p3.py @@ -335,8 +335,9 @@ def test_settings_persisted_in_flocks_json(self, temp_project): # Verify it's in flocks.json data = ConfigWriter._read_raw() - assert "model_settings" not in data - assert data["provider"]["openai"]["models"]["gpt-4o"]["enabled"] is False + assert "model_settings" in data + assert "openai/gpt-4o" in data["model_settings"] + assert data["model_settings"]["openai/gpt-4o"]["enabled"] is False def test_default_model_persisted_in_flocks_json(self, temp_project): """Verify that default models are persisted in flocks.json.""" diff --git a/webui/src/pages/Model/index.tsx b/webui/src/pages/Model/index.tsx index ba81c6c05..959ac8ede 100644 --- a/webui/src/pages/Model/index.tsx +++ b/webui/src/pages/Model/index.tsx @@ -2841,31 +2841,33 @@ function ModelDetailSheet({ const handleSave = async () => { setLoading(true); try { - await modelV2API.createDefinition(provider.id, { - model_id: model.id, - name: name.trim() || model.id, - context_window: parseInt(contextWindow) || undefined, - max_output_tokens: parseInt(maxOutput) || undefined, - supports_vision: supportsVision, - supports_tools: supportsTools, - supports_streaming: supportsStreaming, - supports_reasoning: modelSupportsReasoning ? modelSupportsReasoning : supportsReasoning, - input_price: parseFloat(inputPrice) || 0, - output_price: parseFloat(outputPrice) || 0, - cache_read_price: cacheReadPrice.trim() === '' - ? null - : parseFloat(cacheReadPrice) || 0, - currency, - }); - await modelSettingsAPI.update(provider.id, model.id, { - enabled, - default_parameters: modelSupportsReasoning - ? { - ...defaultParameters, - enable_thinking: supportsReasoning, - } - : undefined, - }); + await Promise.all([ + modelV2API.createDefinition(provider.id, { + model_id: model.id, + name: name.trim() || model.id, + context_window: parseInt(contextWindow) || undefined, + max_output_tokens: parseInt(maxOutput) || undefined, + supports_vision: supportsVision, + supports_tools: supportsTools, + supports_streaming: supportsStreaming, + supports_reasoning: modelSupportsReasoning ? modelSupportsReasoning : supportsReasoning, + input_price: parseFloat(inputPrice) || 0, + output_price: parseFloat(outputPrice) || 0, + cache_read_price: cacheReadPrice.trim() === '' + ? null + : parseFloat(cacheReadPrice) || 0, + currency, + }), + modelSettingsAPI.update(provider.id, model.id, { + enabled, + default_parameters: modelSupportsReasoning + ? { + ...defaultParameters, + enable_thinking: supportsReasoning, + } + : undefined, + }), + ]); toast.success(t('credentialsSaved')); onSaved(); } catch (e: any) {