Skip to content

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

Open
masenf wants to merge 3 commits into
mainfrom
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 3 commits into
mainfrom
claude/hybrid-property-backend-var-guard

Conversation

@masenf

@masenf masenf commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator

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

codspeed-hq Bot commented Jun 5, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 26 untouched benchmarks
⏩ 8 skipped benchmarks1


Comparing claude/hybrid-property-backend-var-guard (7b40b13) with main (e059363)

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

greptile-apps Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a guard that raises a clear HybridPropertyError when a hybrid property's frontend logic (its getter or custom .var function) reads a backend (underscore-prefixed) state var, converting a previously silent data-leak into a loud, actionable error. It also fixes the var() method to return a new descriptor instance rather than mutating the existing one, preventing descriptor sharing across mixin subclasses.

  • _StateBackendVarGuard wraps the state class when building the frontend var and intercepts __getattr__ to block any attribute that's in state_cls.backend_vars (which already includes inherited backend vars via {**inherited_backend_vars, **new_backend_vars}), so the inheritance case is handled correctly.
  • HybridPropertyError is a ReflexError (not AttributeError) so it can't be silently swallowed by hasattr/getattr with a default; the error message names the property, the state, and the offending var.
  • Tests cover backend-var access from both the getter and the .var function, frontend-only access, mixin descriptor isolation, and the object-var pass-through (guard is state-only).

Confidence Score: 5/5

Safe to merge — the guard is narrowly scoped to BaseState subclasses, backend_vars correctly includes inherited vars so the optimization path is sound, and no existing behavior is changed for states with no backend vars or for object-var access.

The change is well-contained: a new private proxy class, one new exception, and a narrowly conditional branch in get. The core invariant — that backend_vars already aggregates inherited backend vars — is confirmed in the state metaclass. No breaking changes, and the test suite directly exercises the four meaningful cases.

No files require special attention.

Important Files Changed

Filename Overview
packages/reflex-base/src/reflex_base/vars/hybrid_property.py Adds _StateBackendVarGuard proxy and updates get to wrap state owners, plus fixes var() to return a new descriptor instance; logic is correct and well-structured.
packages/reflex-base/src/reflex_base/utils/exceptions.py Adds HybridPropertyError as a dedicated ReflexError subclass; correctly not an AttributeError to prevent silent swallowing by hasattr/getattr.
tests/units/vars/test_hybrid_property.py New test file covering backend-var access from getter and .var function, frontend-only access, mixin descriptor isolation, and object-var pass-through; all scenarios are well-targeted.
news/6621.feature.md Changelog entry for the root package; clear user-facing description of the new error behavior.
packages/reflex-base/news/6621.feature.md Changelog entry for the reflex-base package; accurately describes HybridPropertyError addition.

Reviews (5): Last reviewed commit: "fix(hybrid_property): return new descrip..." | Re-trigger Greptile

Comment thread packages/reflex-base/src/reflex_base/vars/object.py Outdated
Comment thread packages/reflex-base/src/reflex_base/vars/hybrid_property.py
@masenf masenf force-pushed the claude/hybrid-property-backend-var-guard branch 2 times, most recently 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
Base automatically changed from claude/relaxed-cerf-Z110q to main June 19, 2026 16:14
@masenf masenf force-pushed the claude/hybrid-property-backend-var-guard branch from c44fc0a to bff3fa1 Compare June 19, 2026 16:21
…on a state

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__ now wraps a state owner in a _StateBackendVarGuard while
building the frontend var; reading a backend var raises HybridPropertyError,
pointing at the misuse in the user's getter/.var function. Object-var owners
(nested dataclass / pydantic / SQLAlchemy access) have no backend vars and are
unaffected. The guard lives at the single point where state-ness is determined,
so there is no redundant BaseState lookup.

https://claude.ai/code/session_01DKFiYGnWRQG8wMNKFW7obm
@masenf masenf force-pushed the claude/hybrid-property-backend-var-guard branch from bff3fa1 to 9c1894f Compare June 19, 2026 16:26
FarhanAliRaza
FarhanAliRaza previously approved these changes Jun 24, 2026
@FarhanAliRaza FarhanAliRaza self-requested a review June 24, 2026 17:10
…in mutation

HybridProperty.var() now constructs a new instance instead of mutating
self, matching property.setter semantics. This prevents a shared mixin
descriptor from being silently mutated when one concrete subclass calls
.var — other subclasses that inherit the mixin no longer see a leaked _var.

Also adds an early-return in __get__ when owner.backend_vars is empty,
skipping the guard entirely when there is nothing to guard against.
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.

3 participants