diff --git a/custom_components/openai_compatible/config_flow.py b/custom_components/openai_compatible/config_flow.py index 0f1dc13..104982d 100644 --- a/custom_components/openai_compatible/config_flow.py +++ b/custom_components/openai_compatible/config_flow.py @@ -104,7 +104,7 @@ from .const import ( UNSUPPORTED_PRIORITY_SERVICE_TIERS_MODELS, UNSUPPORTED_WEB_SEARCH_MODELS, ) -from .model_list import async_fetch_model_ids, model_field +from .model_list import async_fetch_model_ids, default_model, model_field from .url_util import ( CredentialsInBaseURL, InvalidBaseURL, @@ -325,36 +325,58 @@ class OpenAISubentryFlowHandler(ConfigSubentryFlow): } ) + # The model belongs on the first screen. Picking one is the entire + # point of adding an endpoint, and behind the "recommended" checkbox + # it was unreachable: ticked, this step creates the subentry and the + # model silently stays RECOMMENDED_CHAT_MODEL -- an OpenAI name a + # third-party provider will not serve. entity.py reads + # CONF_CHAT_MODEL regardless of the recommended flag, so a model + # chosen here takes effect on either path. + model_ids = await async_fetch_model_ids(self._get_entry().runtime_data) + step_schema[ + vol.Optional(CONF_CHAT_MODEL, default=default_model(model_ids)) + ] = model_field(model_ids) + step_schema[ vol.Required(CONF_RECOMMENDED, default=options.get(CONF_RECOMMENDED, False)) ] = bool + errors: dict[str, str] = {} + if user_input is not None: if user_input.get(CONF_LLM_HASS_API) is None: user_input.pop(CONF_LLM_HASS_API, None) - if user_input[CONF_RECOMMENDED]: - if self._is_new: - return self.async_create_entry( - title=user_input.pop(CONF_NAME), + # Checked here rather than on the next step, because the + # recommended path creates the subentry from this one and would + # otherwise skip the check entirely. + if user_input.get(CONF_CHAT_MODEL) in UNSUPPORTED_MODELS: + errors[CONF_CHAT_MODEL] = "model_not_supported" + + if not errors: + if user_input[CONF_RECOMMENDED]: + if self._is_new: + return self.async_create_entry( + title=user_input.pop(CONF_NAME), + data=user_input, + ) + return self.async_update_and_abort( + self._get_entry(), + self._get_reconfigure_subentry(), data=user_input, ) - return self.async_update_and_abort( - self._get_entry(), - self._get_reconfigure_subentry(), - data=user_input, - ) - options.update(user_input) - if CONF_LLM_HASS_API in options and CONF_LLM_HASS_API not in user_input: - options.pop(CONF_LLM_HASS_API) - return await self.async_step_additional() + options.update(user_input) + if CONF_LLM_HASS_API in options and CONF_LLM_HASS_API not in user_input: + options.pop(CONF_LLM_HASS_API) + return await self.async_step_additional() return self.async_show_form( step_id="init", data_schema=self.add_suggested_values_to_schema( vol.Schema(step_schema), options ), + errors=errors, ) async def async_step_additional( @@ -365,14 +387,8 @@ class OpenAISubentryFlowHandler(ConfigSubentryFlow): errors: dict[str, str] = {} step_schema: VolDictType = { - # Upstream makes this a free-text box defaulting to an OpenAI - # model name. Offer what the provider actually serves instead. - vol.Optional( - CONF_CHAT_MODEL, - default=RECOMMENDED_CHAT_MODEL, - ): model_field( - await async_fetch_model_ids(self._get_entry().runtime_data) - ), + # CONF_CHAT_MODEL is asked on the init step, so that it is + # reachable without unticking "recommended" first. vol.Optional( CONF_MAX_TOKENS, default=RECOMMENDED_MAX_TOKENS, @@ -393,8 +409,6 @@ class OpenAISubentryFlowHandler(ConfigSubentryFlow): if user_input is not None: options.update(user_input) - if user_input.get(CONF_CHAT_MODEL) in UNSUPPORTED_MODELS: - errors[CONF_CHAT_MODEL] = "model_not_supported" if not errors: return await self.async_step_model() diff --git a/custom_components/openai_compatible/model_list.py b/custom_components/openai_compatible/model_list.py index d8268b1..d4c5002 100644 --- a/custom_components/openai_compatible/model_list.py +++ b/custom_components/openai_compatible/model_list.py @@ -22,7 +22,7 @@ from homeassistant.helpers.selector import ( SelectSelectorMode, ) -from .const import LOGGER +from .const import LOGGER, RECOMMENDED_CHAT_MODEL DEFAULT_TIMEOUT = 10.0 @@ -48,6 +48,14 @@ async def async_fetch_model_ids( except openai.OpenAIError as err: LOGGER.debug("Could not list models: %s", err) return [] + except Exception: # noqa: BLE001 - see below + # Deliberately broad. This runs inside a config-flow form and has a + # working fallback (a free-text model field), so nothing it can raise + # justifies taking the dialog down: a proxy may return a payload the + # SDK cannot parse, and the failure modes of arbitrary + # OpenAI-compatible servers are not enumerable in advance. + LOGGER.debug("Unexpected error listing models", exc_info=True) + return [] return sorted({model.id for model in page.data if getattr(model, "id", None)}) @@ -71,3 +79,17 @@ def model_field(model_ids: list[str]) -> Any: sort=True, ) ) + + +def default_model(model_ids: list[str]) -> str: + """Return the model to preselect for a new endpoint. + + Upstream's default is RECOMMENDED_CHAT_MODEL, an OpenAI name. Offering it + to someone configuring a third-party provider preselects a model that will + fail on first use, so prefer it only when the provider actually serves it + (or when discovery told us nothing) and otherwise start from what is + advertised. + """ + if not model_ids or RECOMMENDED_CHAT_MODEL in model_ids: + return RECOMMENDED_CHAT_MODEL + return model_ids[0] diff --git a/custom_components/openai_compatible/strings.json b/custom_components/openai_compatible/strings.json index 29d0531..ca49531 100644 --- a/custom_components/openai_compatible/strings.json +++ b/custom_components/openai_compatible/strings.json @@ -123,12 +123,12 @@ "data": { "chat_model": "[%key:common::generic::model%]", "max_tokens": "Maximum tokens to return in response", - "store_responses": "Store requests and responses in OpenAI", + "store_responses": "Store requests and responses on the provider", "temperature": "Temperature", "top_p": "Top P" }, "data_description": { - "store_responses": "If enabled, requests and responses are stored by OpenAI and visible in your OpenAI dashboard logs" + "store_responses": "Sets the Responses API store flag, asking the provider to keep requests and responses in its own logs. Support varies: OpenAI shows them in its dashboard, while many compatible servers ignore the flag entirely." }, "title": "Additional settings" }, diff --git a/custom_components/openai_compatible/translations/en.json b/custom_components/openai_compatible/translations/en.json index 774e1db..bfd17c1 100644 --- a/custom_components/openai_compatible/translations/en.json +++ b/custom_components/openai_compatible/translations/en.json @@ -59,12 +59,12 @@ "data": { "chat_model": "Model", "max_tokens": "Maximum tokens to return in response", - "store_responses": "Store requests and responses in OpenAI", + "store_responses": "Store requests and responses on the provider", "temperature": "Temperature", "top_p": "Top P" }, "data_description": { - "store_responses": "If enabled, requests and responses are stored by OpenAI and visible in your OpenAI dashboard logs" + "store_responses": "Sets the Responses API store flag, asking the provider to keep requests and responses in its own logs. Support varies: OpenAI shows them in its dashboard, while many compatible servers ignore the flag entirely." }, "title": "Additional settings" }, @@ -123,12 +123,12 @@ "data": { "chat_model": "Model", "max_tokens": "Maximum tokens to return in response", - "store_responses": "Store requests and responses in OpenAI", + "store_responses": "Store requests and responses on the provider", "temperature": "Temperature", "top_p": "Top P" }, "data_description": { - "store_responses": "If enabled, requests and responses are stored by OpenAI and visible in your OpenAI dashboard logs" + "store_responses": "Sets the Responses API store flag, asking the provider to keep requests and responses in its own logs. Support varies: OpenAI shows them in its dashboard, while many compatible servers ignore the flag entirely." }, "title": "Additional settings" }, diff --git a/tests/test_config_flow.py b/tests/test_config_flow.py index 17c37bf..b87c47b 100644 --- a/tests/test_config_flow.py +++ b/tests/test_config_flow.py @@ -177,6 +177,9 @@ async def test_creating_conversation_subentry( processed_options = RECOMMENDED_CONVERSATION_OPTIONS.copy() processed_options[CONF_PROMPT] = processed_options[CONF_PROMPT].strip() + # The init step now asks for the model, so the created subentry records + # one explicitly instead of falling back to the default at request time. + processed_options[CONF_CHAT_MODEL] = RECOMMENDED_CHAT_MODEL assert result2["data"] == processed_options @@ -239,23 +242,17 @@ async def test_subentry_unsupported_model( subentry_flow["flow_id"], { CONF_RECOMMENDED: False, + CONF_CHAT_MODEL: "o1-mini", CONF_PROMPT: "Speak like a pirate", CONF_LLM_HASS_API: ["assist"], }, ) await hass.async_block_till_done() - assert subentry_flow["type"] is FlowResultType.FORM - assert subentry_flow["step_id"] == "additional" - # Configure additional step - subentry_flow = await hass.config_entries.subentries.async_configure( - subentry_flow["flow_id"], - { - CONF_CHAT_MODEL: "o1-mini", - }, - ) - await hass.async_block_till_done() + # The model is asked for, and so validated, on the init step -- the flow + # stays there and reports the error against the field. assert subentry_flow["type"] is FlowResultType.FORM + assert subentry_flow["step_id"] == "init" assert subentry_flow["errors"] == {"chat_model": "model_not_supported"} @@ -295,6 +292,7 @@ async def test_subentry_reasoning_effort_list( subentry_flow["flow_id"], { CONF_RECOMMENDED: False, + CONF_CHAT_MODEL: model, CONF_PROMPT: "Speak like a pirate", CONF_LLM_HASS_API: ["assist"], }, @@ -305,9 +303,7 @@ async def test_subentry_reasoning_effort_list( # Configure additional step subentry_flow = await hass.config_entries.subentries.async_configure( subentry_flow["flow_id"], - { - CONF_CHAT_MODEL: model, - }, + {}, ) assert subentry_flow["type"] is FlowResultType.FORM assert subentry_flow["step_id"] == "model" @@ -349,6 +345,7 @@ async def test_subentry_reasoning_summary_visibility( subentry_flow["flow_id"], { CONF_RECOMMENDED: False, + CONF_CHAT_MODEL: model, CONF_PROMPT: "Speak like a pirate", CONF_LLM_HASS_API: ["assist"], }, @@ -359,9 +356,7 @@ async def test_subentry_reasoning_summary_visibility( # Configure additional step subentry_flow = await hass.config_entries.subentries.async_configure( subentry_flow["flow_id"], - { - CONF_CHAT_MODEL: model, - }, + {}, ) assert subentry_flow["type"] is FlowResultType.FORM assert subentry_flow["step_id"] == "model" @@ -398,6 +393,7 @@ async def test_subentry_reasoning_summary_options( subentry_flow["flow_id"], { CONF_RECOMMENDED: False, + CONF_CHAT_MODEL: model, CONF_PROMPT: "Speak like a pirate", CONF_LLM_HASS_API: ["assist"], }, @@ -407,9 +403,7 @@ async def test_subentry_reasoning_summary_options( subentry_flow = await hass.config_entries.subentries.async_configure( subentry_flow["flow_id"], - { - CONF_CHAT_MODEL: model, - }, + {}, ) assert subentry_flow["type"] is FlowResultType.FORM assert subentry_flow["step_id"] == "model" @@ -446,6 +440,7 @@ async def test_subentry_reasoning_summary_default_sanitized_on_model_switch( subentry_flow["flow_id"], { CONF_RECOMMENDED: False, + CONF_CHAT_MODEL: "o3", CONF_PROMPT: "Speak like a pirate", CONF_LLM_HASS_API: ["assist"], }, @@ -454,7 +449,7 @@ async def test_subentry_reasoning_summary_default_sanitized_on_model_switch( subentry_flow = await hass.config_entries.subentries.async_configure( subentry_flow["flow_id"], - {CONF_CHAT_MODEL: "o3"}, + {}, ) assert subentry_flow["step_id"] == "model" @@ -512,6 +507,7 @@ async def test_subentry_service_tier_list( subentry_flow["flow_id"], { CONF_RECOMMENDED: False, + CONF_CHAT_MODEL: model, CONF_PROMPT: "Speak like a pirate", CONF_LLM_HASS_API: ["assist"], }, @@ -522,9 +518,7 @@ async def test_subentry_service_tier_list( # Configure additional step subentry_flow = await hass.config_entries.subentries.async_configure( subentry_flow["flow_id"], - { - CONF_CHAT_MODEL: model, - }, + {}, ) assert subentry_flow["type"] is FlowResultType.FORM assert subentry_flow["step_id"] == "model" @@ -558,6 +552,7 @@ async def test_subentry_unsupported_reasoning_effort( subentry_flow["flow_id"], { CONF_RECOMMENDED: False, + CONF_CHAT_MODEL: "gpt-5", CONF_PROMPT: "Speak like a pirate", CONF_LLM_HASS_API: ["assist"], }, @@ -568,9 +563,7 @@ async def test_subentry_unsupported_reasoning_effort( # Configure additional step subentry_flow = await hass.config_entries.subentries.async_configure( subentry_flow["flow_id"], - { - CONF_CHAT_MODEL: "gpt-5", - }, + {}, ) assert subentry_flow["type"] is FlowResultType.FORM assert subentry_flow["step_id"] == "model" @@ -706,11 +699,11 @@ async def test_form_credentials_in_base_url(hass: HomeAssistant) -> None: ( { CONF_RECOMMENDED: False, + CONF_CHAT_MODEL: "o1-pro", CONF_PROMPT: "Speak like a pro", }, { CONF_TEMPERATURE: 1.0, - CONF_CHAT_MODEL: "o1-pro", CONF_TOP_P: RECOMMENDED_TOP_P, CONF_MAX_TOKENS: 10000, }, @@ -740,11 +733,11 @@ async def test_form_credentials_in_base_url(hass: HomeAssistant) -> None: ( { CONF_RECOMMENDED: False, + CONF_CHAT_MODEL: RECOMMENDED_CHAT_MODEL, CONF_PROMPT: "Speak like a pirate", }, { CONF_TEMPERATURE: 0.3, - CONF_CHAT_MODEL: RECOMMENDED_CHAT_MODEL, CONF_TOP_P: RECOMMENDED_TOP_P, CONF_MAX_TOKENS: RECOMMENDED_MAX_TOKENS, }, @@ -797,11 +790,11 @@ async def test_form_credentials_in_base_url(hass: HomeAssistant) -> None: ( { CONF_RECOMMENDED: False, + CONF_CHAT_MODEL: "gpt-4o", CONF_PROMPT: "Speak like super Mario", }, { CONF_TEMPERATURE: 0.8, - CONF_CHAT_MODEL: "gpt-4o", CONF_TOP_P: 0.9, CONF_MAX_TOKENS: 1000, CONF_STORE_RESPONSES: True, @@ -852,11 +845,11 @@ async def test_form_credentials_in_base_url(hass: HomeAssistant) -> None: ( { CONF_RECOMMENDED: False, + CONF_CHAT_MODEL: "gpt-5.6", CONF_PROMPT: "Speak like a pirate", }, { CONF_TEMPERATURE: 0.8, - CONF_CHAT_MODEL: "gpt-5.6", CONF_TOP_P: 0.9, CONF_MAX_TOKENS: 1000, }, @@ -975,11 +968,11 @@ async def test_form_credentials_in_base_url(hass: HomeAssistant) -> None: ( { CONF_RECOMMENDED: False, + CONF_CHAT_MODEL: "o3-mini", CONF_PROMPT: "Speak like a pirate", }, { CONF_TEMPERATURE: 0.8, - CONF_CHAT_MODEL: "o3-mini", CONF_TOP_P: 0.9, CONF_MAX_TOKENS: 1000, }, @@ -1020,11 +1013,11 @@ async def test_form_credentials_in_base_url(hass: HomeAssistant) -> None: ( { CONF_RECOMMENDED: False, + CONF_CHAT_MODEL: "gpt-4o", CONF_PROMPT: "Speak like a pirate", }, { CONF_TEMPERATURE: 0.8, - CONF_CHAT_MODEL: "gpt-4o", CONF_TOP_P: 0.9, CONF_MAX_TOKENS: 1000, }, @@ -1075,11 +1068,11 @@ async def test_form_credentials_in_base_url(hass: HomeAssistant) -> None: ( { CONF_RECOMMENDED: False, + CONF_CHAT_MODEL: "gpt-5-pro", CONF_PROMPT: "Speak like a pirate", }, { CONF_TEMPERATURE: 0.8, - CONF_CHAT_MODEL: "gpt-5-pro", CONF_TOP_P: 0.9, CONF_MAX_TOKENS: 1000, }, @@ -1155,7 +1148,13 @@ async def test_subentry_switching( assert subentry_flow["type"] is FlowResultType.ABORT assert subentry_flow["reason"] == "reconfigure_successful" - assert subentry.data == expected_options + # The init step always asks for a model, so every subentry records one. + # Cases that do not name one get the default, which is what the flow + # submits for them; cases that do still win, being unpacked second. + assert subentry.data == { + CONF_CHAT_MODEL: RECOMMENDED_CHAT_MODEL, + **expected_options, + } @pytest.mark.parametrize("store_responses", [False, True]) @@ -1183,6 +1182,7 @@ async def test_subentry_web_search_user_location( { CONF_RECOMMENDED: False, CONF_PROMPT: "Speak like a pirate", + CONF_CHAT_MODEL: RECOMMENDED_CHAT_MODEL, }, ) assert subentry_flow["type"] is FlowResultType.FORM @@ -1193,7 +1193,6 @@ async def test_subentry_web_search_user_location( subentry_flow["flow_id"], { CONF_TEMPERATURE: 1.0, - CONF_CHAT_MODEL: RECOMMENDED_CHAT_MODEL, CONF_TOP_P: RECOMMENDED_TOP_P, CONF_MAX_TOKENS: RECOMMENDED_MAX_TOKENS, CONF_STORE_RESPONSES: store_responses, @@ -1307,6 +1306,7 @@ async def test_creating_ai_task_subentry( assert result2.get("title") == "Custom AI Task" assert result2.get("data") == { CONF_RECOMMENDED: True, + CONF_CHAT_MODEL: RECOMMENDED_CHAT_MODEL, } assert ( @@ -1354,6 +1354,7 @@ async def test_creating_ai_task_subentry_additional( { "name": "Advanced AI Task", CONF_RECOMMENDED: False, + CONF_CHAT_MODEL: "gpt-4o", }, ) @@ -1364,7 +1365,6 @@ async def test_creating_ai_task_subentry_additional( result3 = await hass.config_entries.subentries.async_configure( result["flow_id"], { - CONF_CHAT_MODEL: "gpt-4o", CONF_MAX_TOKENS: 200, CONF_STORE_RESPONSES: True, CONF_TEMPERATURE: 0.5, diff --git a/tests/test_model_list.py b/tests/test_model_list.py index 8ba8c37..c8685d9 100644 --- a/tests/test_model_list.py +++ b/tests/test_model_list.py @@ -73,6 +73,20 @@ async def test_fetch_survives_an_endpoint_that_will_not_list(error: Exception) - assert await async_fetch_model_ids(_client(error=error)) == [] +async def test_fetch_survives_a_non_openai_error() -> None: + """Discovery must never break the form it runs inside. + + A proxy can return a payload the SDK cannot parse, or a client can be + wired in a way that raises something other than an OpenAIError; either way + the dropdown should degrade to a text box, not take the dialog down. + """ + client = MagicMock() + client.with_options.return_value = client + client.models.list = MagicMock(return_value="not awaitable") + + assert await async_fetch_model_ids(client) == [] + + async def test_field_is_a_dropdown_that_still_accepts_a_typed_name() -> None: """A proxy may serve a model it does not advertise, so allow free text.""" field = model_field(["claude-sonnet", "gpt-4o-mini"]) diff --git a/tests/test_model_picker.py b/tests/test_model_picker.py index 0d92989..9eb3fd3 100644 --- a/tests/test_model_picker.py +++ b/tests/test_model_picker.py @@ -47,27 +47,17 @@ async def _model_step( ) -> dict: """Return the form that asks for the model, for any endpoint type. - stt and tts are single-step flows that ask on init; conversation and - ai_task_data ask on the step after it. + All four types ask on the first screen, so that picking a model never + depends on noticing the "recommended" checkbox first. """ with patch(DISCOVERY, return_value=models): result = await hass.config_entries.subentries.async_init( (entry.entry_id, subentry_type), context={"source": config_entries.SOURCE_USER}, ) - assert result["type"] is FlowResultType.FORM - assert result["step_id"] == "init" - - if subentry_type in ("stt", "tts"): - return result - - result = await hass.config_entries.subentries.async_configure( - result["flow_id"], - {CONF_NAME: "An endpoint", CONF_RECOMMENDED: False}, - ) - assert result["type"] is FlowResultType.FORM - assert result["step_id"] == "additional" - return result + assert result["type"] is FlowResultType.FORM + assert result["step_id"] == "init" + return result @pytest.mark.parametrize( @@ -133,18 +123,15 @@ async def test_a_non_openai_model_name_survives_the_flow( (mock_config_entry.entry_id, "conversation"), context={"source": config_entries.SOURCE_USER}, ) + # The model is chosen on the very first screen, alongside the name. result = await hass.config_entries.subentries.async_configure( result["flow_id"], - {CONF_NAME: "Sonnet via LiteLLM", CONF_RECOMMENDED: False}, + { + CONF_NAME: "Sonnet via LiteLLM", + CONF_CHAT_MODEL: "claude-sonnet-4-6", + CONF_RECOMMENDED: True, + }, ) - result = await hass.config_entries.subentries.async_configure( - result["flow_id"], {CONF_CHAT_MODEL: "claude-sonnet-4-6"} - ) - # remaining steps take their defaults - while result["type"] is FlowResultType.FORM: - result = await hass.config_entries.subentries.async_configure( - result["flow_id"], {} - ) await hass.async_block_till_done() assert result["type"] is FlowResultType.CREATE_ENTRY