Repository navigation
Conversation
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
[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. FindingsSummaryChanges browser-storage hydration to replay storage fields and cached dependents in one traversal.
Reviews (2) · Last reviewed commit: "fix(state): replay hydration overrides o..." · Reviewed by Greptile |
Merging this PR will not alter performance
|
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.
| return await root._get_resolved_delta() | ||
| finally: | ||
| _replay_client_storage.reset(token) | ||
| root._clean() |
There was a problem hiding this comment.
_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!
| 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) |
There was a problem hiding this comment.
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.
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 base2fc4a9caa5ce125acd9ce1f2e13cf5910ca5f8d8as well (2 failed, 3 controls passed). This PR does not claim to repair those inherited behaviors.Verification
154 added cases pass, including:
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: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.