[BugFix] openbb-core: Fix Merged Choices Validation in Python Interface (#7404)

deeleeramone · web-flow · commit dddc3b328284 · 2026-03-09T15:19:19.000Z
* add choices validation to filter_inputs

* provider interface

* provider interface test

---------

Co-authored-by: deeleeramone &lt;&gt;
diff --git a/openbb_platform/core/openbb_core/app/provider_interface.py b/openbb_platform/core/openbb_core/app/provider_interface.py
@@ -285,6 +285,27 @@ def _create_field(
                         + ", ".join(providers)  # type: ignore[arg-type]
                         + "."
                     )
+
+        # Auto-derive choices from Literal annotation for provider-specific fields
+        # when no explicit choices are already declared for this provider.
+        # This makes Literal annotations equivalent to manually declared choices,
+        # so providers only need to declare the type annotation.
+        if provider_name and provider_name not in choices:
+            _ann = annotation
+            _origin = get_origin(_ann)
+            # Unwrap Optional[Literal[...]] = Union[Literal[...], None]
+            if _origin is Union:
+                _inner = [a for a in get_args(_ann) if a is not type(None)]
+                if len(_inner) == 1:
+                    _ann = _inner[0]
+                    _origin = get_origin(_ann)
+            if _origin is Literal:
+                _literal_args = list(get_args(_ann))
+                if _literal_args:
+                    choices[provider_name] = {
+                        "choices": _literal_args,
+                    }
+
         provider_field = (
             f"(provider: {provider_name})" if provider_name != "openbb" else ""
         )
diff --git a/openbb_platform/core/openbb_core/app/static/utils/filters.py b/openbb_platform/core/openbb_core/app/static/utils/filters.py
@@ -2,6 +2,7 @@
 
 from typing import Any
 
+from openbb_core.app.model.abstract.error import OpenBBError
 from openbb_core.app.utils import check_single_item, convert_to_basemodel
 
 
@@ -49,6 +50,25 @@ def filter_inputs(
                             f"{field} -> multiple items not allowed for '{provider}'",
                         )
 
+                    choices = (
+                        provider_properties.get("choices")
+                        if isinstance(provider_properties, dict)
+                        else None
+                    )
+                    if choices:
+                        items = (
+                            [s.strip() for s in new.split(",")]
+                            if isinstance(new, str) and "," in new
+                            else [new]
+                        )
+                        for item in items:
+                            if item not in choices:
+                                raise OpenBBError(
+                                    f"Invalid value '{item}' for '{field}'"
+                                    f" (provider: '{provider}')."
+                                    f" Must be one of: {choices}"
+                                )
+
                     kwargs[p][field] = new
                     break
     else:
diff --git a/openbb_platform/core/tests/app/static/test_filters.py b/openbb_platform/core/tests/app/static/test_filters.py
@@ -3,6 +3,7 @@
 import numpy as np
 import pandas as pd
 import pytest
+from openbb_core.app.model.abstract.error import OpenBBError
 from openbb_core.app.static.utils.filters import filter_inputs
 from openbb_core.provider.abstract.data import Data
 
@@ -67,3 +68,160 @@ def test_filter_inputs(
         assert isinstance(
             result["data"], Data
         ), f"The 'data' key should be of type {Data.__name__}"
+
+
+# --- Choices validation tests ---
+# These tests cover the fix for a silent data corruption bug:
+#
+# EXACT BUG SCENARIO (obb.economy.balance_of_payments):
+#   - ECB's `frequency` default is "monthly"; OECD only supports "annual"/"quarterly".
+#   - `openbb-build` merges provider schemas into a single ExtraParams dataclass whose
+#     `frequency` field has type Union[Literal['monthly','quarterly'], Literal['annual',
+#     'quarterly']] and default="monthly" (taken from ECB, first alphabetically).
+#   - "mont" was correctly rejected: it satisfies neither Literal, so ExtraParams
+#     instantiation raises a ValidationError before any provider logic runs.
+#   - "monthly" silently passed: it satisfies ECB's Literal, so ExtraParams accepted
+#     it. `filter_extra_params` then saw frequency="monthly" == merged default="monthly"
+#     and DROPPED the parameter entirely. OECD's transform_query received no `frequency`
+#     kwarg and fell back to its own pydantic default "quarterly", returning quarterly
+#     data with zero error or warning.
+#
+# The fix has two components:
+#   1. `_create_field` in `provider_interface.py` now auto-derives per-provider choices
+#      from Literal type annotations. A provider field declared as
+#      `frequency: Literal["annual", "quarterly"]` causes openbb-build to emit
+#      `"frequency": {"oecd": {"choices": ["annual", "quarterly"]}}` into the info dict
+#      of the generated package file — no manual `__json_schema_extra__` entry needed.
+#   2. `filter_inputs` validates the raw user kwargs dict against those choices BEFORE
+#      the merged ExtraParams dataclass is built, so the explicitly-passed "monthly"
+#      is caught at the boundary regardless of what the merged default is.
+
+# Info dict WITH choices — mirrors the post-fix `economy.py` generated by openbb-build.
+# Choices are now auto-derived from the provider's Literal annotation:
+#   `frequency: Literal["annual", "quarterly"]`   →   choices=["annual", "quarterly"]
+# No manual `__json_schema_extra__` entry is required.
+_BOP_INFO_WITH_CHOICES = {
+    "frequency": {
+        "oecd": {"multiple_items_allowed": False, "choices": ["annual", "quarterly"]},
+    }
+}
+
+# Info dict WITHOUT choices — represents the pre-fix state where Literal annotations
+# were NOT reflected in the info dict, so filter_inputs had nothing to validate against.
+_BOP_INFO_WITHOUT_CHOICES = {
+    "frequency": {
+        "oecd": {"multiple_items_allowed": False},
+    }
+}
+
+# Shorter alias used by the general-purpose tests below.
+_BOP_INFO = _BOP_INFO_WITH_CHOICES
+
+
+def _bop_kwargs(provider: str, **extra) -> dict:
+    """Return a filter_inputs kwargs structure for the BOP endpoint."""
+    return {
+        "provider_choices": {"provider": provider},
+        "standard_params": {},
+        "extra_params": {"frequency": "quarterly", **extra},
+    }
+
+
+def test_filter_inputs_choices_valid_value_passes():
+    """A value that is in the allowed choices must be accepted without error."""
+    kwargs = _bop_kwargs("oecd", frequency="annual")
+    result = filter_inputs(info=_BOP_INFO, **kwargs)
+    assert result["extra_params"]["frequency"] == "annual"
+
+
+def test_filter_inputs_choices_default_value_passes():
+    """The default value ('quarterly') must also be accepted."""
+    kwargs = _bop_kwargs("oecd")  # frequency defaults to 'quarterly'
+    result = filter_inputs(info=_BOP_INFO, **kwargs)
+    assert result["extra_params"]["frequency"] == "quarterly"
+
+
+def test_filter_inputs_choices_invalid_value_raises():
+    """
+    Regression test: 'monthly' must be rejected for the 'oecd' provider.
+
+    Before the fix, `filter_extra_params` would silently drop `frequency="monthly"`
+    because it equalled the merged ExtraParams default (taken from ECB). The OECD
+    fetcher then used its own pydantic default 'quarterly', returning quarterly data
+    with no error or warning.
+
+    After the fix, `filter_inputs` validates the value against the provider's allowed
+    choices and raises an `OpenBBError` with a descriptive message.
+    """
+    kwargs = _bop_kwargs("oecd", frequency="monthly")
+    with pytest.raises(OpenBBError, match="Invalid value 'monthly' for 'frequency'"):
+        filter_inputs(info=_BOP_INFO, **kwargs)
+
+
+def test_filter_inputs_choices_not_enforced_for_other_provider():
+    """
+    Choices defined for 'oecd' must NOT apply when a different provider is selected.
+    ECB legitimately accepts 'monthly', so it must pass without error.
+    """
+    kwargs = _bop_kwargs("ecb", frequency="monthly")
+    result = filter_inputs(info=_BOP_INFO, **kwargs)
+    assert result["extra_params"]["frequency"] == "monthly"
+
+
+def test_filter_inputs_choices_no_info_no_error():
+    """When no `info` dict is supplied, no choices validation occurs."""
+    kwargs = {
+        "provider_choices": {"provider": "oecd"},
+        "extra_params": {"frequency": "monthly"},
+    }
+    # Must not raise even though 'monthly' would be invalid for oecd with choices
+    result = filter_inputs(**kwargs)
+    assert result["extra_params"]["frequency"] == "monthly"
+
+
+# --- Negative tests: exact bug reproduction ---
+# These two tests mirror the discriminating condition that first exposed the bug:
+#   "mont"   → rejected  (caught by ExtraParams Literal validation — was already working)
+#   "monthly" → accepted  (equalled the merged default → silently dropped → wrong data)
+
+
+def test_filter_inputs_pre_fix_monthly_passed_without_choices():
+    """
+    Pre-fix negative: without choices in the info dict, 'monthly' passes through
+    filter_inputs unchallenged — exactly as it did before the fix.
+
+    Before the fix, Literal annotations on provider fields were not reflected in the
+    info dict written by openbb-build, so filter_inputs had no choices to validate
+    against. Once 'monthly' exits filter_inputs, filter_extra_params sees
+    v == merged_default ("monthly") and silently drops the parameter, so the OECD
+    model never validates it.
+    """
+    kwargs = {
+        "provider_choices": {"provider": "oecd"},
+        "standard_params": {},
+        "extra_params": {"frequency": "monthly"},
+    }
+    # No choices → no rejection → 'monthly' passes through (pre-fix behaviour)
+    result = filter_inputs(info=_BOP_INFO_WITHOUT_CHOICES, **kwargs)
+    assert result["extra_params"]["frequency"] == "monthly"
+
+
+def test_filter_inputs_post_fix_monthly_rejected_with_choices():
+    """
+    Post-fix negative: with choices in the info dict, the same 'monthly' value
+    that used to silently corrupt the result is now rejected with a clear error.
+
+    Choices are now auto-derived by openbb-build from the provider's Literal annotation
+    (`frequency: Literal["annual", "quarterly"]`), so no manual __json_schema_extra__
+    entry is required. Together with the test above this pair proves:
+      - 'mont'    → already failed (Literal Union rejects it at ExtraParams build time)
+      - 'monthly' → used to pass  (satisfied ECB's Literal, equalled merged default)
+      - 'monthly' → now fails     (filter_inputs catches it via choices before ExtraParams)
+    """
+    kwargs = {
+        "provider_choices": {"provider": "oecd"},
+        "standard_params": {},
+        "extra_params": {"frequency": "monthly"},
+    }
+    with pytest.raises(OpenBBError, match="Invalid value 'monthly' for 'frequency'"):
+        filter_inputs(info=_BOP_INFO_WITH_CHOICES, **kwargs)
diff --git a/openbb_platform/core/tests/app/test_provider_interface.py b/openbb_platform/core/tests/app/test_provider_interface.py
@@ -2,11 +2,14 @@
 
 # pylint: disable=redefined-outer-name
 
+from typing import Literal, Optional
+
 import pytest
 from openbb_core.app.provider_interface import (
     ProviderChoices,
     ProviderInterface,
 )
+from pydantic.fields import FieldInfo
 
 
 @pytest.fixture(scope="module")
@@ -78,3 +81,69 @@ def test_models(provider_interface):
     assert isinstance(models, list)
     assert len(models) > 0
     assert "EquityHistorical" in models
+
+
+# --- _create_field: Literal → choices auto-derivation ---
+
+
+def test_create_field_literal_annotation_produces_choices():
+    """
+    _create_field must auto-derive choices from a Literal annotation.
+
+    This is the core of the fix: a provider field declared as
+    `frequency: Literal["annual", "quarterly"]` must cause _create_field to emit
+    {"multiple_items_allowed": False, "choices": ["annual", "quarterly"]} into the
+    json_schema_extra under the provider's key, so that filter_inputs can validate
+    user values before the merged ExtraParams dataclass is built.
+    """
+    field = FieldInfo(annotation=Literal["annual", "quarterly"], default="quarterly")
+    result = ProviderInterface._create_field("frequency", field, provider_name="oecd")
+    extra = result.default.json_schema_extra
+    assert extra is not None
+    assert "oecd" in extra
+    assert extra["oecd"]["choices"] == ["annual", "quarterly"]
+
+
+def test_create_field_optional_literal_annotation_produces_choices():
+    """
+    _create_field must unwrap Optional[Literal[...]] and still derive choices.
+    force_optional=True wraps Literal annotations in Optional — choices must survive.
+    """
+    field = FieldInfo(annotation=Optional[Literal["annual", "quarterly"]], default=None)
+    result = ProviderInterface._create_field(
+        "frequency", field, provider_name="oecd", force_optional=True
+    )
+    extra = result.default.json_schema_extra
+    assert extra is not None
+    assert "oecd" in extra
+    assert extra["oecd"]["choices"] == ["annual", "quarterly"]
+
+
+def test_create_field_str_annotation_produces_no_choices():
+    """
+    _create_field must NOT produce choices for a plain str annotation.
+    Only Literal annotations trigger auto-derivation.
+    """
+    field = FieldInfo(annotation=str, default="quarterly")
+    result = ProviderInterface._create_field("frequency", field, provider_name="oecd")
+    extra = result.default.json_schema_extra if result.default else None
+    # Either no extra at all, or extra does not contain choices for this provider
+    if extra and "oecd" in extra:
+        assert "choices" not in extra["oecd"]
+
+
+def test_create_field_explicit_choices_not_overwritten():
+    """
+    When json_schema_extra already declares explicit choices for a provider,
+    _create_field must not overwrite them with auto-derived Literal choices.
+    Explicit choices take precedence.
+    """
+    field = FieldInfo(
+        annotation=Literal["annual", "quarterly"],
+        default="quarterly",
+        json_schema_extra={"oecd": {"choices": ["annual"]}},
+    )
+    result = ProviderInterface._create_field("frequency", field, provider_name="oecd")
+    extra = result.default.json_schema_extra
+    # The explicit single-item choices list must be preserved, not expanded to both values
+    assert extra["oecd"]["choices"] == ["annual"]
