Skip to content

Fix hydration override mutation dependents - #7572

Merged
masenf merged 4 commits into
mainfrom
fix/7571-hydration-override-dependents
Oct 23, 2026
Merged

masenf merged 4 commits into
mainfrom
fix/7571-hydration-override-dependents

Conversation

@masenf

@masenf masenf commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #7571.

#7568 avoids recomputing cached vars for unchanged browser storage, but its fields-only replay also drops cached dependents invalidated by assignments inside a get_delta() override.

Mark cached storage dependents for delivery without invalidating their snapshot caches, then pass the storage values through one public get_delta() traversal. Real assignments invalidate caches normally. Uncached values stay in the normal event delta, avoiding an extra evaluation during hydration.

This retains output-only overrides, field/computed filters, shared-state cleanup, and the single cached evaluation for unchanged storage. The original two-pass implementation was replaced after review reproduced duplicate override side effects and reintroduced omitted fields; both now have regression tests. No intermediate-delta merge or deferred recording machinery remains.

Contract

This restores the pre-#7568 traversal/return semantics. An override assigning after super().get_delta() must update any already-collected fields it wants changed in its returned delta. Synchronous values collected earlier in the traversal are not rewritten afterward. Async cached sibling dependents resolve after the traversal and observe the corrected storage value, as in the issue reproduction.

Two broader cases remain outside this regression fix: assigning an unrelated field after super().get_delta() can leave its already-visited/absent sibling dependent stale; explicitly injecting a placeholder for an unchanged uncached value can leave its old delivery record intact. Targeted reproductions of both fail on pre-#7568 base 2fc4a9caa5ce125acd9ce1f2e13cf5910ca5f8d8 as well (2 failed, 3 controls passed). This PR does not claim to repair those inherited behaviors.

Verification

154 added cases pass, including:

  • 144 assignment/storage cases: before/after-super assignments, explicit return updates, LocalStorage/SessionStorage/Cookie, hashes and on-load on/off, in-process/mocked Redis
  • output-only and mixed assignment filters
  • duplicate side effects and omission of newly assigned fields/cached computed values
  • shared cleanup and failed correction emit

The exact minimal reproduction in #7571 fails on merged main and passes with this revision. All 144 assignment cases plus the two review regressions match pre-#7568 observable behavior (146/146); cached-evaluation counts are checked separately because avoiding those extra evaluations is the intentional improvement. Existing cached-once and uncached-no-extra evaluation controls pass.

Full Python 3.14.8 unit-suite rerun against unmodified 7ccaa4e213fa1f67374002cb28709272866c3d53:

  • Revised patch: 13,059 passed, 42 failed, 67 skipped
  • Clean baseline: 12,904 passed, 43 failed, 67 skipped
  • All 42 patched failures also occur on baseline. The extra baseline-only failure is its checkout-path fixture under the shared editable environment. No new failing test node IDs; full suite/coverage is not claimed green.
  • Focused processor/state/shared/delta suites: 589 passed and one baseline tracing failure.

Full Ruff lint/format checks and Pyright pass (0 errors, 0 warnings). Published tree matches the tested revision. No news fragment: #7568 has not shipped in any release, so there is no user-facing change to record (skip-changelog).

Redis coverage uses the repository's mocked fixture. No local browser end-to-end test was run. CI results should be checked on the latest head.

Update: merged with main

main was merged into the branch; the only conflict was in test_base_state_processor.py, where both sides appended tests, and both sets are kept. On the merged head the processor, istate, and test_state suites pass (850 tests), and Ruff and Pyright are clean. The two Greptile P1 findings on the latest revision (a dependent of a field assigned after super().get_delta(); a placeholder injected for an unchanged uncached var) were reproduced and fail identically on the pre-#7568 base 8b48dc01c, so they are inherited get_delta override behavior rather than regressions of this change; details are in the thread replies.

Update: review round

The hydration replay now marks dependents through _mark_dirty_computed_vars(..., invalidate=False) instead of a second copy of the dependency walk, so one walk serves both cache invalidation and delivery-only marking. Benchmarked against main: the repo's test_state_delta benchmarks and a microbenchmark of _mark_dirty_computed_vars show no difference beyond run-to-run noise, with roughly 20 ns on the no-dependents path for the extra parameter and check (numbers in the review thread). The shared-cleanup test moved to tests/units/test_state.py and asserts the override's correction took effect. The news fragment is removed, as above.

🤖 Generated with Claude Code

https://claude.ai/code/session_014CNQyjqBL56phnEuga6U6E

Preserve fields-only replay output while resolving dependents invalidated
by real assignments. Commit uncached delivery records against the final
delta and retain shared-state cleanup bookkeeping.

Fixes #7571
@greptile-apps

greptile-apps Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 4/5

[Medium impact] The change affects browser-storage hydration and the state updates sent during reconnects.

Restore the required news fragment before merging; no new behavioral defect was established.

Findings

  1. P2 Hydration fix lacks release notes ▶

Summary

Restores delivery of cached storage dependents during hydration while keeping one public get_delta() traversal.

  • Hydration replays browser storage through one override-aware pass.

Previous threads: the single traversal addresses duplicate override effects and restored filtered fields. masenf accepted leaving unrelated post-super().get_delta() dependents unchanged because the same behavior exists before this fix; Greptile withdrew that finding. masenf also accepted leaving injected uncached placeholders unchanged because that behavior exists before this fix. Neither is a new finding.

Reviews (4) · Last reviewed commit: "fix(state): share the dependent walk bet..." · Reviewed by Greptile

Comment thread reflex/state.py Outdated
Comment thread reflex/state.py Outdated
@codspeed

codspeed Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Merging this PR will degrade performance by 7.78%

2 warnings

⚠️ 18 benchmarks are dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

⚠️ Base and head ran on different runner types

Some benchmarks with significant performance changes were compared across different runner types, so the changes might not come from the code. Run base and head on the same runner type, or use CodSpeed Macro Runners for more stable results.

Learn more about Macro Runners

Open the report in CodSpeed to investigate

❌ 1 regressed benchmark
✅ 147 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
❌ test_state_access[write_own] 126.3 ms 136.9 ms -7.78%

Tip

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


Comparing fix/7571-hydration-override-dependents (7c8a05c) with main (114312a)1

Open in CodSpeed

Footnotes

  1. No successful run was found on main (9b97634) during the generation of this report, so 114312a was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

Mark cached storage dependents for delivery without invalidating their
snapshot caches, then use one public get_delta traversal. Preserve output
filters and existing synchronous/after-super ordering semantics without
repeating override side effects or merging separately filtered deltas.
Comment thread reflex/state.py
Comment thread reflex/istate/delta.py
@masenf masenf added the bug Something isn't working label Oct 19, 2026 — with ChatGPT Codex Connector
…erride-dependents

# Conflicts:
#	tests/units/reflex_base/event/processor/test_base_state_processor.py
@masenf
masenf marked this pull request as ready for review October 23, 2026 01:03
@masenf
masenf requested a review from a team as a code owner October 23, 2026 01:03

@cubic-dev-ai cubic-dev-ai Bot left a comment •

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.

All reported issues were addressed across 5 files

Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread reflex/state.py Outdated
Comment thread tests/units/istate/test_shared.py Outdated
@masenf masenf added the on deck PRs lined up to review / merge next label Oct 23, 2026
Comment thread news/+hydrate-override-assignments.bugfix.md Outdated
Comment thread reflex/state.py Outdated
Comment thread tests/units/istate/test_shared.py Outdated
@masenf masenf added the skip-changelog For doc/internal changes label Oct 23, 2026
@masenf masenf added this to the v0.10.1 milestone Oct 23, 2026
…on replay

Fold the delivery-only marking into _mark_dirty_computed_vars behind an
invalidate flag, so the hydration replay reuses the one dependency walk
instead of carrying a second copy of it. Move the shared-cleanup test next
to the module it covers and assert that the override's correction took
effect. Drop the news fragment: #7568 is unreleased, so this fix changes
nothing for any released version.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014CNQyjqBL56phnEuga6U6E
@masenf
masenf requested a review from FarhanAliRaza October 23, 2026 16:50
Comment thread reflex/state.py
@masenf
masenf merged commit 656e8c7 into main Oct 23, 2026
168 of 169 checks passed
@masenf
masenf deleted the fix/7571-hydration-override-dependents branch October 23, 2026 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working on deck PRs lined up to review / merge next skip-changelog For doc/internal changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hydration drops cached dependent updates from get_delta storage assignments

2 participants