Skip to content

Raise a clear error when a hybrid property reads a backend var on a state#6621

Open
masenf wants to merge 2 commits into
claude/relaxed-cerf-Z110qfrom
claude/hybrid-property-backend-var-guard
Open

Raise a clear error when a hybrid property reads a backend var on a state#6621
masenf wants to merge 2 commits into
claude/relaxed-cerf-Z110qfrom
claude/hybrid-property-backend-var-guard

Conversation

@masenf
Copy link
Copy Markdown
Collaborator

@masenf masenf commented Jun 5, 2026

Type of change

  • New feature (non-breaking change which adds functionality)

Note

Stacked on #6619 — base branch is claude/relaxed-cerf-Z110q. This PR contains only the backend-var guard; review it against that base.

Description

A hybrid property's frontend logic (its getter, or a custom @<name>.var function) runs with the state class as self when building the frontend var. Reading a backend (underscore-prefixed) var there previously baked the var's class-level default into the frontend as a frozen literal — a silent leak that never updates and is not reactive.

This PR makes that misuse fail loudly with a clear, actionable error.

Changes:

  1. HybridProperty._get_var guards the state owner (packages/reflex-base/src/reflex_base/vars/hybrid_property.py): when a hybrid property is resolved against a BaseState, the owner is wrapped in a _StateBackendVarGuard. Accessing a backend var while building the frontend var raises, with the traceback pointing at the offending line in the user's getter/.var function. Object-var owners (nested dataclass / pydantic / SQLAlchemy access) have no backend vars and are passed through unchanged.

  2. New HybridPropertyError (packages/reflex-base/src/reflex_base/utils/exceptions.py): a dedicated ReflexError — deliberately not an AttributeError, so it can't be silently swallowed by getattr(..., default) / hasattr — whose message names the property, the state, and the offending backend var, and suggests using a regular var or a separate @<name>.var implementation.

Tests

tests/units/vars/test_hybrid_property.py:

  • backend-var access raises HybridPropertyError from both the getter and a custom .var function
  • frontend-only logic still builds the expected var
  • the guard is state-only — underscore fields accessed through an object var are unaffected

https://claude.ai/code/session_01DKFiYGnWRQG8wMNKFW7obm

@masenf masenf requested a review from a team as a code owner June 5, 2026 22:12
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Jun 5, 2026

Merging this PR will not alter performance

✅ 26 untouched benchmarks
⏩ 8 skipped benchmarks1


Comparing claude/hybrid-property-backend-var-guard (c44fc0a) with main (31f785e)

Open in CodSpeed

Footnotes

  1. 8 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Jun 5, 2026

Greptile Summary

This PR extends hybrid_property support to dataclasses, pydantic models, and SQLAlchemy models by moving HybridProperty into reflex-base and hooking it into ObjectVar.__getattr__. A new _StateBackendVarGuard proxy prevents backend-only state vars from leaking into a property's frontend var construction path.

  • HybridProperty is relocated to reflex_base/vars/hybrid_property.py, gaining _get_var/_guarded_owner helpers and the HybridPropertyError exception; the public export path via rx._x.hybrid_property is preserved.
  • ObjectVar.__getattr__ now calls inspect.getattr_static to detect a HybridProperty descriptor on the resolved type and delegates to descriptor._get_var(self), enabling transparent var resolution on nested objects (e.g. State.info.a_b).
  • Comprehensive unit tests cover the backend-var guard, frontend-only access, the object-var non-guarded path, and all four model kinds; the integration test verifies end-to-end reactivity for a nested dataclass case.

Confidence Score: 4/5

Safe to merge; the core logic is correct and well-tested across all targeted model types.

The implementation is clean and the guard correctly blocks backend-only vars when building frontend vars on State classes while leaving plain dataclass/model attribute access unaffected. The only concerns are minor: a lazy import and inspect.getattr_static now run on every concrete-type ObjectVar.getattr invocation, and the inherited descriptor-sharing behaviour of _var is undocumented. No data-correctness or security issues were found.

packages/reflex-base/src/reflex_base/vars/object.py - the new hot-path overhead in getattr is worth a second look if var-construction performance becomes a concern.

Important Files Changed

Filename Overview
packages/reflex-base/src/reflex_base/vars/hybrid_property.py New file implementing HybridProperty with _StateBackendVarGuard; clean and well-documented
packages/reflex-base/src/reflex_base/vars/object.py Adds HybridProperty descriptor check inside getattr; inspect.getattr_static + lazy import now called on every concrete-type attribute access
packages/reflex-base/src/reflex_base/utils/exceptions.py Adds HybridPropertyError exception class; straightforward addition
reflex/experimental/init.py Switches import to reflex_base; public API unchanged
reflex/experimental/hybrid_property.py Old implementation deleted; functionality moved to reflex-base
tests/units/vars/test_hybrid_property.py New unit tests covering backend-var guard, frontend-only access, and object-var non-guarded path
tests/units/vars/test_object.py HybridQuantity mixin tests hybrid resolution across Bare, pydantic, SQLAlchemy, and dataclass types
tests/integration/test_hybrid_properties.py End-to-end assertions for nested-dataclass hybrid property reactivity

Reviews (1): Last reviewed commit: "docs: add news fragments for hybrid prop..." | Re-trigger Greptile

Comment on lines +335 to +343
if isinstance(fixed_type, type):
from .hybrid_property import HybridProperty

# A HybridProperty on the underlying type resolves to a frontend Var with
# this object var substituted as `self`, so e.g. `State.info.a_b` uses the
# same Var-access semantics as accessing the hybrid property directly.
descriptor = inspect.getattr_static(fixed_type, name, None)
if isinstance(descriptor, HybridProperty):
return descriptor._get_var(self)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The lazy from .hybrid_property import HybridProperty import and inspect.getattr_static call are now executed on every ObjectVar.__getattr__ invocation for concrete types — including the common case where the attribute is just a regular field. Python’s module cache makes the repeat import very cheap, but not free. Moving the import to module level (guarded by try/except ImportError if needed to avoid circular imports) would eliminate the per-call overhead.

Suggested change
if isinstance(fixed_type, type):
from .hybrid_property import HybridProperty
# A HybridProperty on the underlying type resolves to a frontend Var with
# this object var substituted as `self`, so e.g. `State.info.a_b` uses the
# same Var-access semantics as accessing the hybrid property directly.
descriptor = inspect.getattr_static(fixed_type, name, None)
if isinstance(descriptor, HybridProperty):
return descriptor._get_var(self)
if isinstance(fixed_type, type):
from .hybrid_property import HybridProperty # noqa: PLC0415 – lazy to avoid circular import
# A HybridProperty on the underlying type resolves to a frontend Var with
# this object var substituted as `self`, so e.g. `State.info.a_b` uses the
# same Var-access semantics as accessing the hybrid property directly.
descriptor = inspect.getattr_static(fixed_type, name, None)
if isinstance(descriptor, HybridProperty):
return descriptor._get_var(self)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Comment on lines +56 to +60
class HybridProperty(property):
"""A hybrid property that can also be used in frontend/as var."""

# The optional var function for the property.
_var: Callable[[Any], Var] | None = None
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 _var class attribute is mutable via instance sharing through inheritance

_var is defined as a class attribute (None) and set as an instance attribute by var(). When a HybridProperty is defined on a mixin and multiple classes inherit it without overriding, all subclasses share the exact same descriptor object. Calling var() on that shared object mutates it for all inheritors simultaneously — a subclass cannot independently override just the .var() function of an inherited HybridProperty without redefining the whole property. A short note in the docstring would help users avoid the pitfall.

@masenf masenf force-pushed the claude/hybrid-property-backend-var-guard branch from feead2e to 977c6c2 Compare June 5, 2026 23:01
claude added 2 commits June 5, 2026 23:51
A hybrid property's frontend logic (its getter, or a custom @<name>.var
function) runs with the state class as `self` when building the frontend
var. Reading a backend (underscore-prefixed) var there previously baked
the var's class-level default into the frontend as a frozen literal — a
silent leak that never updates and is not reactive.

HybridProperty._get_var now wraps a state owner in a guard that raises
HybridPropertyError when a backend var is accessed, with a message that
points at the misuse in the user's getter/var function. Object-var owners
(nested dataclass/model access) have no backend vars and are unchanged.
https://claude.ai/code/session_01DKFiYGnWRQG8wMNKFW7obm
@masenf masenf force-pushed the claude/hybrid-property-backend-var-guard branch from 977c6c2 to c44fc0a Compare June 5, 2026 23:51
@masenf masenf changed the base branch from main to claude/relaxed-cerf-Z110q June 6, 2026 00:16
@masenf masenf changed the title Support hybrid_property on dataclasses, pydantic models, and SQLAlchemy models Raise a clear error when a hybrid property reads a backend var on a state Jun 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants