ci: ban extra="allow" on new pydantic models, grandfather existing ones

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

.github/workflows/test-code-quality.yml

@@ -116,6 +116,9 @@ jobs:
       - name: ban_copy_deepcopy_kwargs
         run: uv run --no-sync python ./tests/code_coverage_tests/ban_copy_deepcopy_kwargs.py
 
+      - name: ban_pydantic_extra_allow
+        run: uv run --no-sync python ./tests/code_coverage_tests/ban_pydantic_extra_allow.py
+
       - name: check_fastuuid_usage
         run: uv run --no-sync python ./tests/code_coverage_tests/check_fastuuid_usage.py
 

CLAUDE.md

@@ -43,6 +43,8 @@ When you fix violations gated by `ruff-strict-budget.json`, `type-discipline-bud
 
 `make pre-commit` saves its complete output to a log file in .git (overwriting previous pre-commit logs) and prints that path as its first and last output lines. To inspect a run, read or grep that log instead of re-running the multi-minute checks just to see a different slice
 
+New Pydantic models must declare the fields they accept. `extra="allow"` is banned by `tests/code_coverage_tests/ban_pydantic_extra_allow.py`, which grandfathers the models that already had it, so don't add it to a new model and don't grow the grandfathered list without a real reason
+
 If you're trying to create a new function that relies on untyped stuff, instead of adding more Any's and pushing `reportAny` / `reportExplicitAny` closer to their basedpyright ceilings, just validate it in the caller with Pydantic (a model or `TypeAdapter` that returns the typed thing or raises will do) and then pass the now typed variable in
 
 If you get an LIT001 or LIT002 fail, refactor the code to follow functional programming best practices rather than introducing mutable data structures. For example, build values in one shot with comprehensions or generators wrapped in `tuple()` / `MappingProxyType()` / `frozenset()` instead of seeding an empty `list`/`dict`/`set` and mutating it over time. Ideally, `# mutable-ok` is never used; reach for it only as a genuine last resort when an immutable rewrite is truly impossible, and always pair it with a real reason

tests/code_coverage_tests/ban_pydantic_extra_allow.py

@@ -0,0 +1,208 @@
+"""Fail CI when a new Pydantic model opts into ``extra="allow"``.
+
+``extra="allow"`` silently accepts undeclared keys, so typos survive validation and
+real fields (pricing on ``ModelInfo``, for example) never get declared anywhere. The
+models listed in ``GRANDFATHERED`` predate this check and stay allowed; anything new
+must declare its fields.
+"""
+
+import ast
+import os
+import sys
+from typing import Final, Iterator, NamedTuple, Sequence
+
+SCAN_ROOT: Final = "litellm"
+
+GRANDFATHERED: Final = frozenset(
+    {
+        "litellm/llms/base_llm/ocr/transformation.py::OCRPage",
+        "litellm/llms/base_llm/ocr/transformation.py::OCRPageImage",
+        "litellm/llms/base_llm/ocr/transformation.py::OCRResponse",
+        "litellm/llms/base_llm/ocr/transformation.py::OCRUsageInfo",
+        "litellm/llms/base_llm/sandbox/transformation.py::CodeExecutionResult",
+        "litellm/llms/base_llm/sandbox/transformation.py::ContainerHandle",
+        "litellm/llms/base_llm/search/transformation.py::SearchResponse",
+        "litellm/llms/base_llm/search/transformation.py::SearchResult",
+        "litellm/proxy/_types.py::CoordinationRedisParams",
+        "litellm/proxy/_types.py::ModelInfo",
+        "litellm/proxy/_types.py::TeamDefaultSettings",
+        "litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py::UISettings",
+        "litellm/router_strategy/auto_router/litellm_encoder.py::CustomDenseEncoder",
+        "litellm/router_strategy/complexity_router/config.py::ComplexityRouterConfig",
+        "litellm/router_strategy/quality_router/config.py::QualityRouterConfig",
+        "litellm/router_strategy/quality_router/config.py::RoutingPreferences",
+        "litellm/types/agents.py::AgentCreateResponse",
+        "litellm/types/agents.py::AgentDeleteResult",
+        "litellm/types/agents.py::AgentListResponse",
+        "litellm/types/agents.py::AgentVersionsResponse",
+        "litellm/types/agents.py::LiteLLMSendMessageResponse",
+        "litellm/types/completion.py::CompletionRequest",
+        "litellm/types/embedding.py::EmbeddingRequest",
+        "litellm/types/fine_tuning.py::OpenAIFineTuningHyperparameters",
+        "litellm/types/guardrails.py::BaseLitellmParams",
+        "litellm/types/llms/anthropic.py::AnthropicResponseContentBlockToolUse",
+        "litellm/types/llms/anthropic.py::AnthropicResponseUsageBlock",
+        "litellm/types/llms/base.py::BaseLiteLLMOpenAIResponseObject",
+        "litellm/types/llms/base.py::HiddenParams",
+        "litellm/types/llms/openai.py::GenericEvent",
+        "litellm/types/llms/openai.py::Hyperparameters",
+        "litellm/types/llms/openai.py::InputTokensDetails",
+        "litellm/types/llms/openai.py::LiteLLMFineTuningJobCreate",
+        "litellm/types/llms/openai.py::OutputTokensDetails",
+        "litellm/types/llms/openai.py::ResponseAPIUsage",
+        "litellm/types/prompts/init_prompts.py::PromptInfo",
+        "litellm/types/prompts/init_prompts.py::PromptLiteLLMParams",
+        "litellm/types/proxy/guardrails/guardrail_hooks/cisco_ai_defense.py::CiscoAIDefenseGuardrailConfigModelOptionalParams",
+        "litellm/types/proxy/guardrails/guardrail_hooks/generic_guardrail_api.py::GuardrailToolParam",
+        "litellm/types/proxy/guardrails/guardrail_hooks/straiker.py::StraikerWebhookResponse",
+        "litellm/types/rag.py::RAGIngestRequest",
+        "litellm/types/rag.py::RAGQueryRequest",
+        "litellm/types/realtime.py::RealtimeSessionConfig",
+        "litellm/types/realtime.py::RealtimeTranscriptionSessionRequest",
+        "litellm/types/realtime.py::RealtimeTranscriptionSessionResponse",
+        "litellm/types/router.py::Deployment",
+        "litellm/types/router.py::GenericLiteLLMParams",
+        "litellm/types/router.py::LiteLLM_Params",
+        "litellm/types/router.py::ModelInfo",
+        "litellm/types/utils.py::ImageResponse",
+    }
+)
+
+
+class Violation(NamedTuple):
+    file: str
+    line: int
+    model: str
+
+    def identifier(self) -> str:
+        return f"{self.file}::{self.model}"
+
+
+def _is_extra_allow_keyword(keyword: ast.keyword) -> bool:
+    return keyword.arg == "extra" and isinstance(keyword.value, ast.Constant) and keyword.value.value == "allow"
+
+
+def _mapping_sets_extra_allow(node: ast.Dict) -> bool:
+    return any(
+        isinstance(key, ast.Constant)
+        and key.value == "extra"
+        and isinstance(value, ast.Constant)
+        and value.value == "allow"
+        for key, value in zip(node.keys, node.values)
+    )
+
+
+def _is_extra_allow_value(node: ast.expr) -> bool:
+    if isinstance(node, ast.Call):
+        return any(_is_extra_allow_keyword(keyword) for keyword in node.keywords)
+    if isinstance(node, ast.Dict):
+        return _mapping_sets_extra_allow(node)
+    return False
+
+
+def _assigns_extra_allow(statement: ast.stmt, target_names: Sequence[str]) -> bool:
+    if isinstance(statement, ast.Assign):
+        targets = statement.targets
+        value = statement.value
+    elif isinstance(statement, ast.AnnAssign) and statement.value is not None:
+        targets = [statement.target]
+        value = statement.value
+    else:
+        return False
+    names = {target.id for target in targets if isinstance(target, ast.Name)}
+    return bool(names & set(target_names)) and _is_extra_allow_value(value)
+
+
+def _is_allow_literal(node: ast.expr) -> bool:
+    if isinstance(node, ast.Constant):
+        return node.value == "allow"
+    return isinstance(node, ast.Attribute) and node.attr == "allow"
+
+
+def _legacy_config_sets_extra_allow(class_def: ast.ClassDef) -> bool:
+    return any(
+        isinstance(statement, ast.ClassDef)
+        and statement.name == "Config"
+        and any(
+            isinstance(inner, ast.Assign)
+            and any(isinstance(target, ast.Name) and target.id == "extra" for target in inner.targets)
+            and _is_allow_literal(inner.value)
+            for inner in statement.body
+        )
+        for statement in class_def.body
+    )
+
+
+def _class_allows_extra(class_def: ast.ClassDef) -> bool:
+    if any(_is_extra_allow_keyword(keyword) for keyword in class_def.keywords):
+        return True
+    if any(_assigns_extra_allow(statement, ["model_config"]) for statement in class_def.body):
+        return True
+    return _legacy_config_sets_extra_allow(class_def)
+
+
+def _iter_classes(body: Sequence[ast.stmt], prefix: str = "") -> Iterator[tuple[str, ast.ClassDef]]:
+    for statement in body:
+        if isinstance(statement, ast.ClassDef):
+            qualified = f"{prefix}{statement.name}"
+            yield qualified, statement
+            yield from _iter_classes(statement.body, f"{qualified}.")
+
+
+def find_violations_in_source(source: str, relative_path: str) -> tuple[Violation, ...]:
+    tree = ast.parse(source, filename=relative_path)
+    return tuple(
+        Violation(file=relative_path, line=class_def.lineno, model=qualified)
+        for qualified, class_def in _iter_classes(tree.body)
+        if _class_allows_extra(class_def)
+    )
+
+
+def _scan_file(file_path: str, base_dir: str) -> tuple[Violation, ...]:
+    relative = os.path.relpath(file_path, base_dir).replace(os.sep, "/")
+    with open(file_path, "r", encoding="utf-8") as handle:
+        return find_violations_in_source(handle.read(), relative)
+
+
+def find_extra_allow_models(base_dir: str) -> tuple[Violation, ...]:
+    return tuple(
+        violation
+        for root, _, files in os.walk(os.path.join(base_dir, SCAN_ROOT))
+        for file_name in sorted(files)
+        if file_name.endswith(".py")
+        for violation in _scan_file(os.path.join(root, file_name), base_dir)
+    )
+
+
+def main() -> int:
+    base_dir = os.getcwd()
+    found = find_extra_allow_models(base_dir)
+    violations = tuple(violation for violation in found if violation.identifier() not in GRANDFATHERED)
+    stale = tuple(sorted(GRANDFATHERED - {violation.identifier() for violation in found}))
+
+    for violation in violations:
+        print(f'{violation.file}:{violation.line}: {violation.model} sets extra="allow"')
+    if violations:
+        print(
+            f'\nFound {len(violations)} new Pydantic model(s) using extra="allow".\n'
+            'Declare the fields you accept instead. extra="allow" hides typos and\n'
+            "leaves real fields undocumented and untyped. If a model genuinely has to\n"
+            "forward opaque provider payloads, add it to GRANDFATHERED in\n"
+            "tests/code_coverage_tests/ban_pydantic_extra_allow.py with a reason in the PR."
+        )
+    if stale:
+        print(
+            '\nThese GRANDFATHERED entries no longer use extra="allow" (or moved).\n'
+            "Remove them so the list keeps ratcheting down:"
+        )
+        for entry in stale:
+            print(f"  {entry}")
+
+    if violations or stale:
+        return 1
+    print('No new extra="allow" Pydantic models found.')
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())

tests/test_litellm/test_ban_pydantic_extra_allow.py

@@ -0,0 +1,93 @@
+"""Tests for the extra="allow" ban at tests/code_coverage_tests/ban_pydantic_extra_allow.py."""
+
+import os
+import sys
+
+import pytest
+
+_CODE_COVERAGE_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)), "..", "code_coverage_tests")
+sys.path.insert(0, _CODE_COVERAGE_DIR)
+
+import ban_pydantic_extra_allow as checker  # noqa: E402
+
+_REPO_ROOT = os.path.join(os.path.dirname(os.path.abspath(__file__)), "..", "..")
+
+
+@pytest.mark.parametrize(
+    "source",
+    [
+        pytest.param(
+            'class Foo(BaseModel):\n    model_config = ConfigDict(extra="allow")\n',
+            id="config_dict_call",
+        ),
+        pytest.param(
+            'class Foo(BaseModel):\n    model_config = ConfigDict(protected_namespaces=(), extra="allow")\n',
+            id="config_dict_call_with_other_kwargs",
+        ),
+        pytest.param(
+            'class Foo(BaseModel):\n    model_config = {"extra": "allow"}\n',
+            id="plain_dict",
+        ),
+        pytest.param(
+            'class Foo(BaseModel):\n    model_config: ConfigDict = ConfigDict(extra="allow")\n',
+            id="annotated_assignment",
+        ),
+        pytest.param(
+            'class Foo(BaseModel, extra="allow"):\n    pass\n',
+            id="class_keyword",
+        ),
+        pytest.param(
+            'class Foo(BaseModel):\n    class Config:\n        extra = "allow"\n',
+            id="legacy_inner_config",
+        ),
+        pytest.param(
+            "class Foo(BaseModel):\n    class Config:\n        extra = Extra.allow\n",
+            id="legacy_inner_config_enum",
+        ),
+    ],
+)
+def test_detects_extra_allow(source):
+    violations = checker.find_violations_in_source(source, "litellm/types/thing.py")
+    assert [violation.identifier() for violation in violations] == ["litellm/types/thing.py::Foo"]
+
+
+@pytest.mark.parametrize(
+    "source",
+    [
+        pytest.param(
+            'class Foo(BaseModel):\n    model_config = ConfigDict(extra="forbid")\n',
+            id="extra_forbid",
+        ),
+        pytest.param(
+            "class Foo(BaseModel):\n    model_config = ConfigDict(populate_by_name=True)\n",
+            id="unrelated_config",
+        ),
+        pytest.param(
+            "class Foo(BaseModel):\n    bar: str\n",
+            id="no_config",
+        ),
+        pytest.param(
+            'class Foo(BaseModel):\n    """Docstring mentioning extra="allow"."""\n',
+            id="docstring_mention_only",
+        ),
+        pytest.param(
+            'def f():\n    return ConfigDict(extra="allow")\n',
+            id="outside_class",
+        ),
+    ],
+)
+def test_ignores_non_violations(source):
+    assert checker.find_violations_in_source(source, "litellm/types/thing.py") == ()
+
+
+def test_reports_nested_class_with_qualified_name():
+    source = 'class Outer:\n    class Inner(BaseModel):\n        model_config = ConfigDict(extra="allow")\n'
+    violations = checker.find_violations_in_source(source, "litellm/types/thing.py")
+    assert [violation.identifier() for violation in violations] == ["litellm/types/thing.py::Outer.Inner"]
+
+
+def test_grandfathered_list_matches_repo():
+    """Every grandfathered entry must still exist, and nothing new may be added."""
+    found = frozenset(violation.identifier() for violation in checker.find_extra_allow_models(_REPO_ROOT))
+    assert sorted(found - checker.GRANDFATHERED) == []
+    assert sorted(checker.GRANDFATHERED - found) == []