feat: ask for the model on the first screen of the endpoint flow
The model field was one screen too late. Reported directly: "this is where I would have expected to see the model dropdown", on the AI Task init step that showed only Name and the recommended checkbox. - CONF_CHAT_MODEL moves from async_step_additional to async_step_init for conversation and ai_task_data, so all four endpoint types now ask on the first screen. stt and tts already did. - UNSUPPORTED_MODELS is validated there too, since the recommended path creates the subentry from that step and would otherwise skip the check. - model_list.default_model() picks what to preselect: RECOMMENDED_CHAT_MODEL only when the provider actually serves it (or discovery returned nothing), otherwise the first advertised model. Offering a stranger's gpt-4o-mini to a LiteLLM user preselects a model that fails on first use. This is the trade I got wrong last commit. I kept the field on the second step to avoid touching 71 ported tests, having judged them unfixable mechanically. They were not: the 71 were ~13 functions under parametrize, and test_subentry_switching's data is already a list of per-step dicts, so moving a model between steps is moving a key between two dicts. Two scripted passes moved 14 of them; 5 needed reasoning about (an error that now surfaces on init rather than the next step, and four cases whose expected data gains the now-always-recorded model). async_fetch_model_ids now also catches non-OpenAIError exceptions. Found via a ported test whose client is a MagicMock: the TypeError from awaiting it escaped and broke the whole form. A real proxy returning a payload the SDK cannot parse would do the same, and this runs inside a dialog with a working free-text fallback, so nothing it raises justifies taking the form down. Also fixes a string that was simply false on a third-party provider, again reported from the UI: "Store requests and responses in OpenAI" / "stored by OpenAI and visible in your OpenAI dashboard logs". It now describes the Responses API store flag and says support varies. Swept the other 43 OpenAI-mentioning strings while there -- the rest are [%key:...%] self-references or this fork's own action names, and no other option text makes a claim about who stores the data. Local: 170 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NxJYWgGJtCw13qUHZtG8ki
This commit is contained in:
1 parent
030822c845
commit
6a599e092a
7 files changed
+128
-91
No files matched your search
+36
-36
@@ -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,
|
||||
|
||||
@@ -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"])
|
||||
|
||||
+11
-24
@@ -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
|
||||
|
||||
Reference in new issue
Block a user