From 41135fc894ac6d5d254c2446bd3fbde3f43d8257 Mon Sep 17 00:00:00 2001 From: arc53-machine <232052973+arc53-machine@users.noreply.github.com> Date: Thu, 17 Sep 2026 11:51:28 +0100 Subject: [PATCH] refactor(settings): apply the unset rule to optional string Literals too Review follow-up. EMBEDDINGS_POOLING became Optional[Literal["cls", "mean"]], but the group-base rule that maps "", "None" and whitespace to None only matched Optional[str], so EMBEDDINGS_POOLING= in a .env file would have failed validation. The rule now also covers an Optional Literal whose choices are all strings, which lets the AUTH_TYPE validator drop its own copy of that handling. --- docsgpt/core/settings/_shared.py | 13 +++++++++++-- docsgpt/core/settings/auth.py | 6 +++--- tests/core/test_settings.py | 9 ++++++--- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/docsgpt/core/settings/_shared.py b/docsgpt/core/settings/_shared.py index 59723bbf..5b8bfec4 100644 --- a/docsgpt/core/settings/_shared.py +++ b/docsgpt/core/settings/_shared.py @@ -18,15 +18,24 @@ from pydantic_settings import BaseSettings, SettingsConfigDict def _is_optional_str(annotation: Any) -> bool: + """``Optional[str]`` or ``Optional[Literal[...]]`` whose choices are all strings.""" if typing.get_origin(annotation) not in (typing.Union, types.UnionType): return False - return set(typing.get_args(annotation)) == {str, type(None)} + members = set(typing.get_args(annotation)) + if type(None) not in members or len(members) != 2: + return False + (member,) = members - {type(None)} + if member is str: + return True + return typing.get_origin(member) is typing.Literal and all( + isinstance(choice, str) for choice in typing.get_args(member) + ) class SettingsGroup(BaseSettings): """Base for one domain's settings; groups are composed into ``Settings``. - Every ``Optional[str]`` field treats the spellings an unset value has in a + Every ``Optional[str]`` field (and optional string ``Literal``) treats the spellings an unset value has in a ``.env`` file (``KEY=``, ``KEY=None``, whitespace) as ``None``, so a check like ``if settings.OIDC_ISSUER`` or a fallback like ``settings.X or default`` sees "unset" rather than a truthy placeholder string. Real values are diff --git a/docsgpt/core/settings/auth.py b/docsgpt/core/settings/auth.py index c9309775..5069d6cf 100644 --- a/docsgpt/core/settings/auth.py +++ b/docsgpt/core/settings/auth.py @@ -6,7 +6,7 @@ from typing import Literal, Optional from pydantic import Field, field_validator, model_validator -from docsgpt.core.settings._shared import SettingsGroup, normalize_choice, normalize_secret +from docsgpt.core.settings._shared import SettingsGroup, normalize_choice #: Settings an OIDC deployment cannot run without; checked when AUTH_TYPE=oidc. @@ -88,8 +88,8 @@ class AuthSettings(SettingsGroup): @field_validator("AUTH_TYPE", mode="before") @classmethod def _normalize_auth_type(cls, v): - # ``AUTH_TYPE=None`` and ``AUTH_TYPE=`` in .env both mean "no authentication". - return normalize_choice(normalize_secret(v)) + # Unset spellings ("None", "") became None on the group base; this only case-folds a value. + return normalize_choice(v) @model_validator(mode="after") def _require_oidc_settings(self): diff --git a/tests/core/test_settings.py b/tests/core/test_settings.py index db78a45b..ff53fba4 100644 --- a/tests/core/test_settings.py +++ b/tests/core/test_settings.py @@ -92,9 +92,11 @@ class TestValidators: name for name, field in Settings.model_fields.items() if typing.get_origin(field.annotation) in (typing.Union, types.UnionType) - and set(typing.get_args(field.annotation)) == {str, type(None)} + and type(None) in typing.get_args(field.annotation) + and all(a is str or typing.get_origin(a) is typing.Literal for a in typing.get_args(field.annotation) if a is not type(None)) ] assert len(names) > 60 + assert {"EMBEDDINGS_POOLING", "AUTH_TYPE", "OIDC_ISSUER"} <= set(names) loaded = Settings.model_validate({name: " None " for name in names}) with warnings.catch_warnings(): warnings.simplefilter("ignore", DeprecationWarning) @@ -171,9 +173,10 @@ class TestCrossFieldRules: class TestClosedChoices: """Enum-like settings are Literal types: a typo fails at startup instead of falling through.""" + @pytest.mark.parametrize("name", ["AUTH_TYPE", "EMBEDDINGS_POOLING"]) @pytest.mark.parametrize("raw", ["None", "none", "", " "]) - def test_auth_type_unset_spellings(self, raw): - assert Settings.model_validate({"AUTH_TYPE": raw}).AUTH_TYPE is None + def test_optional_choice_unset_spellings(self, name, raw): + assert getattr(Settings.model_validate({name: raw}), name) is None @pytest.mark.parametrize( ("name", "raw", "expected"),