From d1ae73a8a88cb36125196d3dae64db68110335d4 Mon Sep 17 00:00:00 2001 From: axelray-dev <110029405+axelray-dev@users.noreply.github.com> Date: Thu, 25 Jun 2026 01:00:24 +0800 Subject: [PATCH] fix: add clear error message for invalid thinking_style values Widen thinking_style from Literal to str | None and add a @field_validator that produces a helpful error message listing valid options when an invalid value is provided. Addresses the review feedback on #4482. --- nanobot/config/schema.py | 28 +++++++++++++++++-- tests/providers/test_custom_thinking_style.py | 15 ++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/nanobot/config/schema.py b/nanobot/config/schema.py index 9ad60144..22f2acfa 100644 --- a/nanobot/config/schema.py +++ b/nanobot/config/schema.py @@ -2,9 +2,9 @@ from __future__ import annotations from pathlib import Path -from typing import TYPE_CHECKING, Any, Literal +from typing import TYPE_CHECKING, Any, ClassVar, Literal -from pydantic import AliasChoices, ConfigDict, Field, model_validator +from pydantic import AliasChoices, ConfigDict, Field, field_validator, model_validator from pydantic_settings import BaseSettings from nanobot.config_base import Base @@ -182,7 +182,29 @@ class ProviderConfig(Base): extra_headers: dict[str, str] | None = None # Custom headers (e.g. APP-Code for AiHubMix) extra_body: dict[str, Any] | None = None # Extra provider request fields; shape depends on provider/API surface extra_query: dict[str, str] | None = None # Extra query params (e.g. api-version for Azure-style gateways) - thinking_style: Literal["thinking_type", "enable_thinking", "reasoning_split"] | None = None # Thinking/reasoning style for custom providers + thinking_style: str | None = None # Thinking/reasoning style for custom providers + + # Valid values mirror the keys of _THINKING_STYLE_MAP in + # nanobot/providers/openai_compat_provider.py. Kept duplicated here to + # avoid an import cycle (schema.py must not import from providers/). + _VALID_THINKING_STYLES: ClassVar[tuple[str, ...]] = ( + "thinking_type", + "enable_thinking", + "reasoning_split", + ) + + @field_validator("thinking_style") + @classmethod + def _validate_thinking_style(cls, v: str | None) -> str | None: + if not v: # None or "" -> no injection, valid (backwards compatible) + return v + if v not in cls._VALID_THINKING_STYLES: + raise ValueError( + f"Invalid thinking_style {v!r}. " + f"Must be one of: {', '.join(repr(s) for s in cls._VALID_THINKING_STYLES)} " + f"(or empty/omitted)." + ) + return v class BedrockProviderConfig(ProviderConfig): diff --git a/tests/providers/test_custom_thinking_style.py b/tests/providers/test_custom_thinking_style.py index 55b6a05d..44cdf17b 100644 --- a/tests/providers/test_custom_thinking_style.py +++ b/tests/providers/test_custom_thinking_style.py @@ -45,3 +45,18 @@ class TestCustomProviderThinkingStyle: } pc = ProvidersConfig.model_validate(data) assert pc.custom.thinking_style == "enable_thinking" + + def test_invalid_thinking_style_raises_with_clear_message(self) -> None: + """An invalid thinking_style must raise a ValidationError whose message + lists the valid options (not just Pydantic's generic Literal error).""" + import pytest + from pydantic import ValidationError + + with pytest.raises(ValidationError) as exc_info: + ProviderConfig.model_validate({"thinkingStyle": "thinking_typ"}) + + message = str(exc_info.value) + assert "Invalid thinking_style" in message + assert "thinking_type" in message + assert "enable_thinking" in message + assert "reasoning_split" in message