Skip to content

Fix hydration override mutation dependents - #7572

Draft
masenf wants to merge 2 commits into
mainfrom
fix/7571-hydration-override-dependents
Draft

masenf wants to merge 2 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. News fragment included.

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.

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: 3/5

[Medium impact] Refactors how cached computed values are handled during hydration.

The PR should not merge until hydration sends newly dirtied dependents and clears stale records for uncached replacement values.

Findings

  1. P1 Dependent updates disappear ▶
  2. P1 Replacement values never recover ▶

Summary

Changes browser-storage hydration to replay storage fields and cached dependents in one traversal.

  • Hydration corrections now include cached values changed by storage overrides.

Reviews (2) · Last reviewed commit: "fix(state): replay hydration overrides o..." · 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 not alter performance

⚠️ 17 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

✅ 148 untouched benchmarks


Comparing fix/7571-hydration-override-dependents (2efe6e4) with main (7ccaa4e)

Open in CodSpeed

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 on lines +2530 to +2533
return await root._get_resolved_delta()
finally:
_replay_client_storage.reset(token)
root._clean()

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.

P1 Dependent updates disappear

_get_client_storage_delta now cleans up after one traversal. If an override sets a separate field such as status after super().get_delta(), a cached value in another state that reads status may never be collected. Its branch may already have been visited or may not have been included at all.

The assignment marks that value dirty, but root._clean() clears the mark before another delta can send it. The page keeps the old value. Collect newly dirtied dependents without running the override twice.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Comment thread reflex/istate/delta.py
Comment on lines +238 to +240
if _replay_client_storage.get():
delta_vars = state.dirty_vars & state._frontend_var_names
delta_vars.difference_update(state.computed_vars)
delta_vars.difference_update(state._always_dirty_computed_vars)

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.

P1 Replacement values never recover

The replay skips uncached vars without clearing their delivery records when an override inserts a replacement value. The snapshot's follow-up delta records the real value first. If the storage replay then adds a placeholder for that uncached var, no pending record exists for it, so _commit_delta_records cannot forget the old record.

The final event delta and later events drop the real value as unchanged, leaving the placeholder on the page. Restore the check that clears records for uncached values explicitly returned during replay.

This branch has not been deployed

No deployments
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.

Hydration drops cached dependent updates from get_delta storage assignments

1 participant