[harsh21234i]

Type of change

• Bug fix (non-breaking change)

Changes

Fixes #7508 by preventing browser client-storage hydration from unnecessarily invalidating dependent computed vars. Storage values are still passed through get_delta for stale-value reconciliation, but dependent vars are recomputed only when an override actually changes the value.

Added a bugfix changelog entry and preserved existing storage reconciliation behavior.

Validation

• Focused hydration tests: 8 passed

• Ruff check: passed

• git diff --check: passed

[cubic-dev-ai]

3 issues found across 2 files

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic</sub>

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="reflex/state.py">

<violation number="1" location="reflex/state.py:2425">
P2: This normal delta traversal invalidates dependent computed vars before the code can determine whether the override changed the storage value. Probe the override without triggering dependency invalidation so unchanged storage values do not recompute their dependents.</violation>

<violation number="2" location="reflex/state.py:2430">
P2: Python equality treats values such as `1` and `True` as equal even though they serialize differently. Compare serialized values so the backend and dependent vars are updated when an override changes the browser value.</violation>

<violation number="3" location="reflex/state.py:2434">
P1: This traversal starts at the storage-owning state and cannot include computed dependents in an ancestor or sibling substate. Resolve the corrected delta from the root so those dependent values reach the client.</violation>
</file>

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no new actionable issue was found.

Summary

The PR keeps browser-storage reconciliation while avoiding repeat calculations for unchanged storage values.

• Hydration checks storage corrections without needlessly recomputing dependents.

The prior threads are unnumbered, so they remain review context rather than numbered finding entries. The current code addresses their reported cases. No new actionable issue was established.

_{Reviews (16) · Last reviewed commit: "Merge main into PR #7515 and align playg..." · Reviewed by Greptile}

[codspeed]

Merging this PR will degrade performance by 6.71%

⚠️ 13 benchmarks spent significant time in system calls

System calls cannot be consistently instrumented, so they are not included in the measure, which understates the real cost. Please switch to the Walltime instrument to accurately measure system calls.

Measurement and system calls

• test_get_state[disk]
• test_cli_startup[run_help]
• test_cli_startup[cloud_help]
• test_cli_startup[deploy_help]
• test_get_state[memory]
• test_emit_update
• test_import_cli
• test_modify_state[disk]
• test_event_queue_dispatch
• test_cli_startup[help]
• …and 3 more

❌ 2 regressed benchmarks
✅ 146 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Benchmark	BASE	HEAD	Efficiency
❌	test_state_access[write_own]	127 ms	139.8 ms	-9.16%
❌	test_from_event_type[event_spec]	93.5 µs	97.6 µs	-4.19%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.

---

_{Comparing harsh21234i:harsh21234i-fictional-funicular (199823f) with main (8b48dc0)}

[cubic-dev-ai]

All reported issues were addressed across 2 files (changes from recent commits).

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic</sub>

[cubic-dev-ai]

All reported issues were addressed across 2 files (changes from recent commits).

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic</sub>

[masenf]

my first impression is that this patch includes excessively complicated code to handle a small edge case. i get that it can result in doubled database queries when a computed var depends on a storage value and that's not ideal... but this code is hard to grok and seems hard to maintain.

the reason we're marking the applied storage vars dirty after hydrate_and_load snapshot is so they can get processed (and potentially suppressed) by reflex-enterprise's AuthPlugin get_delta patch, but i wonder if this actually matters, because the storage values come from the client anyway -- the end user could read them out of the browser. so it doesn't really make sense to even bother with this path, since there's no actual security advantage to be had.

[harsh21234i]

That’s a fair concern. I kept the reconciliation pass to preserve get_delta override behavior, including the Enterprise AuthPlugin path, but I agree the browser’s storage values are not a security boundary. Would you prefer that I remove this post-snapshot reconciliation path and keep the change narrowly focused on avoiding duplicate dependent-var computation? That would simplify the patch, but would change when overrides see those client-provided values.

[masenf]

remove this post-snapshot reconciliation path and keep the change narrowly focused on avoiding duplicate dependent-var computation

i think if we just skip the loop that dirties the applied vars, then we're basically there

[harsh21234i]

I tried skipping the loop that dirties applied vars. The focused processor tests then had 21 failures because the post-snapshot reconciliation stopped emitting storage values, including get_delta() overrides. Should this change intentionally drop that behavior, or is there another way you’d like to avoid duplicate dependent-var computation while preserving it?

[masenf]

Main now includes #7568, explicitly a smaller alternative to this PR for #7508. It preserves the override pass while avoiding repeated computed-var evaluation. This landed after the workflow-only main merge in 199823f.

The new main merge conflicts in reflex/istate/delta.py and reflex/state.py, where the two approaches overlap. This needs author review of any remaining scope relative to #7568 before another merge from main; choosing either implementation during routine conflict maintenance could discard intended behavior. I left that design choice unchanged.

[harsh21234i]

Thanks. I compared #7515 with #7568 on current main. The original duplicate-computation issue appears covered by #7568. #7515 also tests override cases where get_delta() changes a storage value or assigns a correction after super().get_delta(); those cases behave differently with #7568. Should I pursue that as a separate follow-up, or close #7515 as superseded? I’ve held off resolving the conflict so I don’t discard either behavior before confirming scope.

[masenf]

Related follow-up: #7572 already targets cached dependents invalidated by assignments inside get_delta() overrides after #7568. Its description documents the before/after-super().get_delta() behavior it preserves and the broader inherited cases it leaves out of scope. Comparing the remaining #7515 regression cases with that PR may help establish whether any separate scope remains here.

I’m leaving the conflicting implementations unchanged pending the maintainer’s scope decision.

[harsh21234i]

Thanks, I compared the coverage. #7572 appears to cover the assignment-after-super() and uncached-evaluation cases from #7515. The two cases I don’t see explicitly covered are JSON-distinct values (1 vs True) and nested storage states being probed once. Should we validate either as separate required behavior and add it to #7572, or close #7515 in favor of #7568 and the focused follow-up? I’ll leave the conflict untouched until we settle the scope.

[FarhanAliRaza]

We can do a followup if needed. closing this for now.