PR #7512 - reflex-dev/reflex

All Submissions:

Have you followed the guidelines stated in CONTRIBUTING.md file?
Have you checked to ensure there aren't any other open Pull Requests for the desired changed?

Type of change

Bug fix (non-breaking change which fixes an issue)

Changes To Core Features:

Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your core changes, as applicable?
Have you successfully ran tests with your changes locally?

What and why

Found in the 0.10.0a3 pre-release re-verification as finding A3-01. It is a regression vs 0.10.0a2 in the three cases fixed here, and was confirmed by an independent verifier.

#7495 keeps a history of the class defaults a state var replaces, so a testing tool that assigns the saved field back restores the default in place before its patch. A default configured at import (Svc.limit = 10) was still lost in two ways.

(a)/(b) A patch whose value is rejected. mock.patch.object(Svc, "limit", Other.y) (a Var), or mocker.patch.object(Svc, "_quota", rx.field(5)) (a Field), raises TypeError as intended. But the assignment raised before it recorded its undo entry, and unittest.mock / pytest-mock restore the saved field even after their own assignment failed. That restore popped the configured default, so later tests saw 0. A rejected wrong-type value already recorded its entry; Vars and Fields hit an earlier raise.

(d) monkeypatch.delattr(Svc, "_quota"). The deletion through the declaring state undid the configured default. The teardown then assigned the saved field back, which undid one more, so 0 stayed for good.

The fix

All in reflex_base/vars/base.py:

Every failed class assignment records its undo entry. The checks move into a module-level helper, _assigned_default_arguments, and BaseStateMeta.__setattr__ records the entry in one except around that call.
A deletion through the state that declares the var is a change of its own: it undoes the latest assignment and records it, and assigning the field back puts that assignment back. Deleting through a subclass, as unittest.mock does to undo a patch made there, still undoes the latest change.
The entries of both histories carry one ordering, so undoing always reverses the latest change, including patches nested inside a deletion.

base_vars.md now says that deletion through the class, monkeypatch.delattr and rejected patches round-trip.

Not changed: case (c). monkeypatch.setattr(Svc, "limit", 99) followed by code under test assigning Svc.limit = 50 still leaves 99 after the teardown. Every tool restores by assigning back the same field object it saved, so a restore cannot tell which assignment belongs to the patch: it reverses the latest change, as the docs say (now spelled out). This leaked on a2 too.

Tests

New cases in tests/units/reflex_base/vars/test_base.py. All four fail on main (assert 0 == 10) and pass now:

test_class_assignment_rejected_patch_round_trip[var|field]: a mock.patch.object that rejects a Var / a Field leaves the configured default, and the configuration is still the entry a later del removes.
test_class_assignment_monkeypatch_delattr_round_trip[_value|public]: the deletion applies only inside the patch, for a backend and a frontend var.

test_class_assignment_delattr_restores_default now asserts the new behaviour: assigning the field back after two deletions reverses them in turn, [2] then [3]. Before, it was a no-op.

The campaign's own repros against this branch:

repro	published 0.10.0a3	this PR
explorer `test_undo_edge.py` (a, b, c, d)	4 failed / 4 passed	1 failed / 7 passed: only (c)
verifier `test_v7_undo.py` (incl. pytest-mock, monkeypatch Var/Field, nested controls)	4 failed / 16 passed	1 failed / 19 passed: only (c)

Unit tests: test_base.py, test_state.py, test_component_state.py and tests/units/istate give 726 passed. The full tests/units run gives 12312 passed. The 236 reflex_cli/v2 and reflex_bench browser-driver failures in this sandbox fail identically with main's code and are unrelated. Pre-commit (ruff, pyright, ty, codespell) is clean.

Related: #7511 (A3-04, assign/restore from several threads at once) touches the same history and could take a lock around it in a follow-up.

🤖 Generated with Claude Code

https://claude.ai/code/session_01SjCrbdguMkfe4bhbaREiwo

Generated by Claude Code