diff --git a/news/6738.performance.md b/news/6738.performance.md new file mode 100644 index 00000000000..2f719d22187 --- /dev/null +++ b/news/6738.performance.md @@ -0,0 +1 @@ +Skip deep element-wise type validation on state var hot paths: computed var return types are only checked on recompute (not on cache hits), and production mode no longer walks every element of assigned containers for the log-only type check. diff --git a/packages/reflex-base/news/6738.performance.md b/packages/reflex-base/news/6738.performance.md new file mode 100644 index 00000000000..2f719d22187 --- /dev/null +++ b/packages/reflex-base/news/6738.performance.md @@ -0,0 +1 @@ +Skip deep element-wise type validation on state var hot paths: computed var return types are only checked on recompute (not on cache hits), and production mode no longer walks every element of assigned containers for the log-only type check. diff --git a/packages/reflex-base/src/reflex_base/utils/types.py b/packages/reflex-base/src/reflex_base/utils/types.py index 85c769523a0..dcb2dac8af0 100644 --- a/packages/reflex-base/src/reflex_base/utils/types.py +++ b/packages/reflex-base/src/reflex_base/utils/types.py @@ -3,6 +3,7 @@ from __future__ import annotations import dataclasses +import os import sys import types from collections.abc import Callable, Iterable, Mapping, Sequence @@ -633,6 +634,36 @@ def does_obj_satisfy_typed_dict( return required_keys.issubset(frozenset(obj)) +@lru_cache +def _validation_depth_for_mode(raw_mode: str | None) -> int: + """Get the validation depth for a raw REFLEX_ENV_MODE value. + + Args: + raw_mode: The raw environment variable value (or None if unset). + + Returns: + The `nested` depth to pass to `_isinstance`. + """ + return 0 if raw_mode == constants.Env.PROD.value else 1 + + +def _validation_depth() -> int: + """Get the container depth for hot-path state var type validation. + + The result of these checks only gates a diagnostic log, so production + mode skips the per-element walk of large containers and only validates + the outer type. The environment is re-read on every call so in-process + mode changes take effect immediately. + + Returns: + The `nested` depth to pass to `_isinstance`. + """ + # Read the raw env var directly: interpreting it through + # environment.REFLEX_ENV_MODE.get() on this hot path would re-parse the + # enum on every state var assignment (and the import would be circular). + return _validation_depth_for_mode(os.environ.get("REFLEX_ENV_MODE")) + + def _isinstance( obj: Any, cls: GenericType, diff --git a/packages/reflex-base/src/reflex_base/vars/base.py b/packages/reflex-base/src/reflex_base/vars/base.py index f7212bef821..13dbb7539df 100644 --- a/packages/reflex-base/src/reflex_base/vars/base.py +++ b/packages/reflex-base/src/reflex_base/vars/base.py @@ -65,6 +65,7 @@ GenericType, Self, _isinstance, + _validation_depth, get_origin, has_args, safe_issubclass, @@ -2502,23 +2503,28 @@ def __get__(self, instance: BaseState | None, owner: type): if not self._cache: value = self.fget(instance) - else: - # handle caching - if not hasattr(instance, self._cache_attr) or self.needs_update(instance): - # Set cache attr on state instance. - setattr(instance, self._cache_attr, self.fget(instance)) - # Ensure the computed var gets serialized to redis. - instance._was_touched = True - # Set the last updated timestamp on the state instance. - setattr(instance, self._last_updated_attr, datetime.datetime.now()) - value = getattr(instance, self._cache_attr) + self._check_deprecated_return_type(instance, value) + return value - self._check_deprecated_return_type(instance, value) + # handle caching + if not hasattr(instance, self._cache_attr) or self.needs_update(instance): + # Set cache attr on state instance. + setattr(instance, self._cache_attr, self.fget(instance)) + # Ensure the computed var gets serialized to redis. + instance._was_touched = True + # Set the last updated timestamp on the state instance. + setattr(instance, self._last_updated_attr, datetime.datetime.now()) + value = getattr(instance, self._cache_attr) + # Only validate the return type when the value was just computed. + self._check_deprecated_return_type(instance, value) + return value - return value + return getattr(instance, self._cache_attr) def _check_deprecated_return_type(self, instance: BaseState, value: Any) -> None: - if not _isinstance(value, self._var_type, nested=1, treat_var_as_type=False): + if not _isinstance( + value, self._var_type, nested=_validation_depth(), treat_var_as_type=False + ): console.error( f"Computed var '{type(instance).__name__}.{self._name}' must return" f" a value of type '{escape(str(self._var_type))}', got '{value!s}' of type {type(value)}." @@ -2773,9 +2779,11 @@ async def _awaitable_result(instance: BaseState = instance) -> RETURN_TYPE: instance._was_touched = True # Set the last updated timestamp on the state instance. setattr(instance, self._last_updated_attr, datetime.datetime.now()) - value = getattr(instance, self._cache_attr) - self._check_deprecated_return_type(instance, value) - return value + value = getattr(instance, self._cache_attr) + # Only validate the return type when the value was just computed. + self._check_deprecated_return_type(instance, value) + return value + return getattr(instance, self._cache_attr) return _awaitable_result() diff --git a/reflex/state.py b/reflex/state.py index 3762ad0ac2a..5c304d8189c 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -54,7 +54,7 @@ ) from reflex_base.utils.exceptions import ImmutableStateError as ImmutableStateError from reflex_base.utils.serializers import serializer -from reflex_base.utils.types import _isinstance +from reflex_base.utils.types import _isinstance, _validation_depth from reflex_base.vars import Field, VarData, field from reflex_base.vars.base import ( ComputedVar, @@ -1535,7 +1535,9 @@ def __setattr__(self, name: str, value: Any): if (field := fields.get(name)) is not None and field.is_var: field_type = field.outer_type_ - if not _isinstance(value, field_type, nested=1, treat_var_as_type=False): + if not _isinstance( + value, field_type, nested=_validation_depth(), treat_var_as_type=False + ): console.error( f"Expected field '{type(self).__name__}.{name}' to receive type '{escape(str(field_type))}'," f" but got '{value}' of type '{type(value)}'." diff --git a/tests/units/reflex_base/utils/test_types.py b/tests/units/reflex_base/utils/test_types.py index e4ab9dece45..344b01a3ed0 100644 --- a/tests/units/reflex_base/utils/test_types.py +++ b/tests/units/reflex_base/utils/test_types.py @@ -1,6 +1,15 @@ """Tests for reflex_base.utils.types.""" -from reflex_base.utils.types import ASGIApp, Message, Receive, Scope, Send +from reflex_base import constants +from reflex_base.environment import environment +from reflex_base.utils.types import ( + ASGIApp, + Message, + Receive, + Scope, + Send, + _validation_depth, +) from typing_extensions import TypeAliasType @@ -14,3 +23,19 @@ def test_asgi_aliases_keep_their_names(): assert Receive.__name__ == "Receive" assert Send.__name__ == "Send" assert ASGIApp.__name__ == "ASGIApp" + + +def test_validation_depth_by_env_mode(): + """Hot-path validation walks containers in dev but stays shallow in prod. + + In-process environment mode changes must take effect immediately, without + any cache invalidation by the caller. + """ + initial = environment.REFLEX_ENV_MODE.getenv() + try: + environment.REFLEX_ENV_MODE.set(constants.Env.PROD) + assert _validation_depth() == 0 + environment.REFLEX_ENV_MODE.set(constants.Env.DEV) + assert _validation_depth() == 1 + finally: + environment.REFLEX_ENV_MODE.set(initial) diff --git a/tests/units/reflex_base/vars/__init__.py b/tests/units/reflex_base/vars/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/units/reflex_base/vars/test_base.py b/tests/units/reflex_base/vars/test_base.py new file mode 100644 index 00000000000..0442211cd66 --- /dev/null +++ b/tests/units/reflex_base/vars/test_base.py @@ -0,0 +1,75 @@ +"""Tests for reflex_base.vars.base.""" + +import reflex as rx +from reflex.state import BaseState + + +def test_computed_var_return_type_checked_only_on_recompute(mocker): + """The return type of a cached computed var is only validated on recompute. + + Args: + mocker: Pytest mocker object. + """ + + class ReturnTypeCheckState(BaseState): + v: int = 0 + + @rx.var + def wrong_typed(self) -> str: + return self.v # pyright: ignore [reportReturnType] + + state = ReturnTypeCheckState() + mock_error = mocker.patch("reflex_base.utils.console.error") + assert state.wrong_typed == 0 + assert mock_error.call_count == 1 + # Cache hits must not re-run the (potentially deep) type check. + assert state.wrong_typed == 0 + assert mock_error.call_count == 1 + # Invalidation triggers a recompute, which re-checks the return type. + state.v = 1 + assert state.wrong_typed == 1 + assert mock_error.call_count == 2 + + +def test_non_cached_computed_var_return_type_checked_every_access(mocker): + """A cache=False computed var recomputes, and is type-checked, on every access. + + Args: + mocker: Pytest mocker object. + """ + + class NonCachedReturnTypeCheckState(BaseState): + v: int = 0 + + @rx.var(cache=False) + def wrong_typed(self) -> str: + return self.v # pyright: ignore [reportReturnType] + + state = NonCachedReturnTypeCheckState() + mock_error = mocker.patch("reflex_base.utils.console.error") + assert state.wrong_typed == 0 + assert state.wrong_typed == 0 + assert mock_error.call_count == 2 + + +async def test_async_computed_var_return_type_checked_only_on_recompute(mocker): + """The return type of a cached async computed var is only validated on recompute. + + Args: + mocker: Pytest mocker object. + """ + + class AsyncReturnTypeCheckState(BaseState): + v: int = 0 + + @rx.var + async def wrong_typed(self) -> str: + return self.v # pyright: ignore [reportReturnType] + + state = AsyncReturnTypeCheckState() + mock_error = mocker.patch("reflex_base.utils.console.error") + assert await state.wrong_typed == 0 + assert mock_error.call_count == 1 + # Cache hits must not re-run the (potentially deep) type check. + assert await state.wrong_typed == 0 + assert mock_error.call_count == 1 diff --git a/tests/units/test_state.py b/tests/units/test_state.py index 076024c0363..90fbc9e4260 100644 --- a/tests/units/test_state.py +++ b/tests/units/test_state.py @@ -1437,6 +1437,22 @@ def comp_v(self) -> int: assert comp_v_calls == 2 +def test_setattr_wrong_type_logs_error(mocker): + """Assigning a value of the wrong type to a base var logs an error in dev mode. + + Args: + mocker: Pytest mocker object. + """ + + class WrongTypeState(BaseState): + n: int = 0 + + state = WrongTypeState() + mock_error = mocker.patch("reflex.utils.console.error") + setattr(state, "n", "not an int") # noqa: B010 + assert mock_error.call_count == 1 + + def test_computed_var_cached_depends_on_non_cached(): """Test that a cached var is recalculated if it depends on non-cached ComputedVar."""