Skip to content

fix: unwrap MutableProxy values in _mark_dirty before writing#6148

Open
benedikt-bartscher wants to merge 2 commits intoreflex-dev:mainfrom
benedikt-bartscher:sqla-mutable-proxy
Open

fix: unwrap MutableProxy values in _mark_dirty before writing#6148
benedikt-bartscher wants to merge 2 commits intoreflex-dev:mainfrom
benedikt-bartscher:sqla-mutable-proxy

Conversation

@benedikt-bartscher
Copy link
Contributor

No description provided.

@codspeed-hq
Copy link

codspeed-hq bot commented Mar 1, 2026

Merging this PR will degrade performance by 3.02%

❌ 1 regressed benchmark
✅ 7 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
test_compile_stateful[_stateful_page] 150.8 µs 155.5 µs -3.02%

Comparing benedikt-bartscher:sqla-mutable-proxy (18488dd) with main (e7c3742)

Open in CodSpeed

@benedikt-bartscher benedikt-bartscher marked this pull request as ready for review March 2, 2026 18:50
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 2, 2026

Greptile Summary

Fixed a bug where MutableProxy objects were leaking into underlying data structures when performing mutation operations (append, extend, insert, setitem, setattr). The proxies are designed as read-path wrappers and should not persist in the actual stored data.

The fix adds a new static method _unwrap_proxy_arg that recursively unwraps proxy objects from values before writing them. This method handles:

  • Direct proxy instances
  • Lists, tuples, and sets containing proxies
  • Dict keys and values that are proxies
  • Scalar values (pass-through unchanged)

The unwrapping is applied in _mark_dirty before calling the wrapped mutation function, ensuring that only raw objects are written to the underlying data structures while preserving dirty tracking functionality.

Testing:

  • Extensive test coverage added for all affected mutation operations
  • Tests verify that proxies don't leak into underlying structures
  • Tests confirm object identity is preserved after unwrapping
  • Tests ensure dirty tracking still works correctly after the fix

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The fix is well-designed and addresses a clear bug with proxy leakage. The implementation is clean, handles all common container types appropriately, and includes comprehensive test coverage (11 new tests). All tests verify both the fix and that existing functionality (dirty tracking) remains intact. No edge cases or potential issues identified.
  • No files require special attention

Important Files Changed

Filename Overview
reflex/istate/proxy.py Added _unwrap_proxy_arg method to prevent MutableProxy objects from leaking into underlying data structures during mutations
tests/units/istate/test_proxy.py Added comprehensive test coverage for proxy unwrapping across various mutation operations (append, extend, insert, setitem, setattr)

Last reviewed commit: 18488dd

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

2 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

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.

1 participant