Skip to content

Deduplicate unchanged uncached computed var deltas - #6946

Open
masenf wants to merge 6 commits into
mainfrom
claude/rx-var-cache-false-optimize-g7hds3
Open

Deduplicate unchanged uncached computed var deltas#6946
masenf wants to merge 6 commits into
mainfrom
claude/rx-var-cache-false-optimize-g7hds3

Conversation

@masenf

@masenf masenf commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Type of change

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

Description

Uncached computed vars (@rx.var(cache=False)) are recomputed on every state update, but the recomputed value is only sent to the frontend when it differs from the value that was last sent to the client. This avoids unnecessary re-renders when an uncached var recomputes to the same value.

Key changes:

  1. Delta value tracking: Added _delta_value_key() function to generate comparable keys for values going into deltas. Immutable scalars (str, int, float, bool, None) are their own key; other values are keyed by a digest of their JSON serialization. Values that cannot be serialized are marked as unkeyable and always sent.

  2. ComputedVar recording: Added _record_delta_value() method to ComputedVar that records the last value sent to each client (keyed by client token, since a single state instance can serve multiple clients via linked shared states). Returns whether the value differs from the last recorded value.

  3. Delta filtering: Modified BaseState.get_delta() to skip unchanged uncached computed vars from the delta. For async uncached vars, added _drop_unchanged_delta_value() helper to await the coroutine and drop it if unchanged.

  4. Redis compatibility: The recorded value is stored as an instance attribute (with a special naming convention) so it gets serialized to Redis, ensuring consistency across server restarts.

  5. Documentation: Updated docs/vars/computed_vars.md to explain the new behavior.

Tests

  • Added 7 comprehensive unit tests covering:
    • Basic unchanged value omission
    • Redis state persistence
    • Mutable values and in-place mutations
    • Multi-client scenarios
    • Unkeyable (non-serializable) values
    • Async uncached vars
  • Updated 2 existing tests to reflect the new behavior
  • All tests pass with adequate coverage

Checklist

  • Tests pass with adequate coverage
  • uv run ruff check . and uv run ruff format . clean
  • uv run pyright reflex tests passes
  • Documentation updated

https://claude.ai/code/session_01Fhci3gBdffJ84uzNGKrY3n

Review in cubic

`@rx.var(cache=False)` vars are recomputed for every state update, and
until now every recomputation was pushed to the client even when the
value was identical, causing pointless network traffic and re-renders.

Uncached vars now record a key for the value they last sent to a client;
when a recomputation produces the same key, the var is left out of the
delta. Async uncached vars defer the same decision into the delta via the
existing `_DROP_FROM_DELTA` sentinel.

The key is the value itself for immutable scalars, and a digest of the
value's serialized form otherwise. Comparing the serialized form is
exactly the comparison that matters (it is what the client receives),
it stays small no matter how big the value is, and it cannot be
invalidated by a later in-place mutation of a state-owned value that the
var handed out by reference.

The client token is recorded next to the key: a single state instance
can serve several clients through linked shared states, so a value that
one client already received still has to be sent to the others. For the
same reason `_patch_state` now resolves its throwaway delta with
`record_values=False`, since that delta only exists to refresh computed
vars and is never emitted.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fhci3gBdffJ84uzNGKrY3n
@masenf
masenf requested review from a team and Alek99 as code owners August 25, 2026 19:17
@codspeed-hq

codspeed-hq Bot commented Aug 25, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 32 untouched benchmarks
⏩ 8 skipped benchmarks1


Comparing claude/rx-var-cache-false-optimize-g7hds3 (bc2f61a) with main (7427617)

Open in CodSpeed

Footnotes

  1. 8 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@greptile-apps

greptile-apps Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR deduplicates unchanged values from uncached computed-variable deltas while preserving recomputation behavior and client-specific delivery.

  • Adds stable, serialization-based delta keys with type-sensitive scalar handling.
  • Records the last transmitted value and filters unchanged synchronous and asynchronous computed vars.
  • Prevents discarded linked-state refresh deltas from recording values as sent.
  • Adds persistence, mutation, multi-client, serialization, async, and scalar regression tests.
  • Documents the updated behavior and adds news fragments for both affected packages.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
packages/reflex-base/src/reflex_base/vars/base.py Adds stable delta-value key generation and per-client recording for uncached computed vars.
reflex/state.py Filters unchanged synchronous and asynchronous uncached computed values while propagating recording policy through substates.
reflex/istate/shared.py Prevents discarded linked-state refresh deltas from being treated as client-visible transmissions.
tests/units/test_state.py Adds broad regression coverage for deduplication, persistence, mutable values, client tokens, serialization failures, async vars, scalar types, and NaN.
docs/vars/computed_vars.md Documents that unchanged uncached computed values are recomputed but omitted from frontend deltas.

Reviews (5): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile

Comment thread packages/reflex-base/src/reflex_base/vars/base.py
Comment thread reflex/state.py
Comment thread packages/reflex-base/src/reflex_base/vars/base.py

@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.

4 issues found across 6 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/reflex-base/src/reflex_base/vars/base.py">

<violation number="1" location="packages/reflex-base/src/reflex_base/vars/base.py:2280">
P2: When a value has no registered serializer, `json_dumps` converts it to `null` instead of raising. `_delta_value_key` then reuses the `null` digest and drops later values, so non-serializable values are not always sent as documented; make unsupported serialization return `_UNKEYABLE_VALUE`, including for nested values.</violation>

<violation number="2" location="packages/reflex-base/src/reflex_base/vars/base.py:2557">
P2: When linked clients alternate deltas, this stores only the most recent client token rather than one key per client. After both clients receive the same value, alternating requests resend it indefinitely; store a token-to-key mapping.</violation>

<violation number="3" location="packages/reflex-base/src/reflex_base/vars/base.py:2560">
P2: This dynamically generated tracking attribute bypasses BaseState's reserved-field checks, allowing a colliding internal name to overwrite delta bookkeeping silently. Define the bookkeeping as a reserved non-var state field or reserve the generated names explicitly.

(Based on your team's feedback about reserved internal state tracking fields.)</violation>
</file>

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

<violation number="1" location="reflex/state.py:1914">
P1: Derive the delta token from the root state rather than the patched shared substate. During linked-state fan-out, `self.router` can still belong to the origin state, causing the recipient to reuse the origin's recorded key and omit its required update.</violation>
</file>

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

Re-trigger cubic

Comment thread reflex/state.py
}
always_dirty_computed_vars = self._always_dirty_computed_vars
# Token of the client this delta is for, used to know which values it has.
token = self.router.session.client_token if always_dirty_computed_vars else ""

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: Derive the delta token from the root state rather than the patched shared substate. During linked-state fan-out, self.router can still belong to the origin state, causing the recipient to reuse the origin's recorded key and omit its required update.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At reflex/state.py, line 1914:

<comment>Derive the delta token from the root state rather than the patched shared substate. During linked-state fan-out, `self.router` can still belong to the origin state, causing the recipient to reuse the origin's recorded key and omit its required update.</comment>

<file context>
@@ -1879,30 +1909,46 @@ def get_delta(self) -> Delta:
-        }
+        always_dirty_computed_vars = self._always_dirty_computed_vars
+        # Token of the client this delta is for, used to know which values it has.
+        token = self.router.session.client_token if always_dirty_computed_vars else ""
+        subdelta: dict[str, Any] = {}
+        for prop in delta_vars:
</file context>

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Pushing back: the suggested change is a no-op, because self.router on a substate already resolves to the root state.

router is in every substate's inherited_vars, so BaseState.__getattribute__ forwards the lookup up the parent_state chain to the root — self.router and self._get_root_state().router are the same object by construction:

router inherited on Sub   : True
sub.router token          : client_b
sub._get_root_state token : client_b
identical                 : True

On the fan-out concern specifically: _patch_state reassigns linked_state.parent_state = original_parent_state before any delta is computed, and original_parent_state belongs to the tree of the client the delta is being built for. _do_update_other_tokens -> _update_client(token) enters app.modify_state for that client's token and re-patches the shared state into that client's tree, so self.router resolves to the recipient's token, not the origin's. The origin's token is only reachable while the origin's own delta is being built, which is correct.

(The linked_root_state.router rewrite in _internal_patch_linked_state targets the linked token's own root, which is not the tree the delta recurses through.)


Generated by Claude Code

recorded = (token, key)
if getattr(instance, attr, None) == recorded:
return False
setattr(instance, attr, recorded)

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.

P2: This dynamically generated tracking attribute bypasses BaseState's reserved-field checks, allowing a colliding internal name to overwrite delta bookkeeping silently. Define the bookkeeping as a reserved non-var state field or reserve the generated names explicitly.

(Based on your team's feedback about reserved internal state tracking fields.)

View Feedback

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/reflex-base/src/reflex_base/vars/base.py, line 2560:

<comment>This dynamically generated tracking attribute bypasses BaseState's reserved-field checks, allowing a colliding internal name to overwrite delta bookkeeping silently. Define the bookkeeping as a reserved non-var state field or reserve the generated names explicitly.

(Based on your team's feedback about reserved internal state tracking fields.) </comment>

<file context>
@@ -2480,6 +2517,51 @@ def _last_updated_attr(self) -> str:
+        recorded = (token, key)
+        if getattr(instance, attr, None) == recorded:
+            return False
+        setattr(instance, attr, recorded)
+        # Ensure the recorded value gets serialized to redis.
+        instance._was_touched = True
</file context>

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Pushing back: this is the established convention for per-var bookkeeping on a state instance, not something new here.

ComputedVar already stores __cached_{js_expr} and __last_updated_{js_expr} on the instance the same way; __last_delta_{js_expr} is the third of the set. The __ prefix is what routes all three through the BaseState.__setattr__ / __getattribute__ fast path, deliberately, so they never masquerade as state vars.

For a collision, a user would have to declare a var literally named __last_delta_<name>_rx_state_ (_js_expr already carries FIELD_MARKER) — and a name starting with __ cannot be declared as a state var in the first place.

Promoting these to reserved non-var state fields is a reasonable idea, but it should cover all three attributes at once and is a change to BaseState's field machinery, not something to bundle into a perf fix. Noted as follow-up work on the PR.


Generated by Claude Code

Comment on lines +2557 to +2562
recorded = (token, key)
if getattr(instance, attr, None) == recorded:
return False
setattr(instance, attr, recorded)
# Ensure the recorded value gets serialized to redis.
instance._was_touched = True

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.

P2: When linked clients alternate deltas, this stores only the most recent client token rather than one key per client. After both clients receive the same value, alternating requests resend it indefinitely; store a token-to-key mapping.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/reflex-base/src/reflex_base/vars/base.py, line 2557:

<comment>When linked clients alternate deltas, this stores only the most recent client token rather than one key per client. After both clients receive the same value, alternating requests resend it indefinitely; store a token-to-key mapping.</comment>

<file context>
@@ -2480,6 +2517,51 @@ def _last_updated_attr(self) -> str:
+            with contextlib.suppress(AttributeError):
+                delattr(instance, attr)
+            return True
+        recorded = (token, key)
+        if getattr(instance, attr, None) == recorded:
+            return False
</file context>
Suggested change
recorded = (token, key)
if getattr(instance, attr, None) == recorded:
return False
setattr(instance, attr, recorded)
# Ensure the recorded value gets serialized to redis.
instance._was_touched = True
recorded_values = getattr(instance, attr, {})
if recorded_values.get(token) == key:
return False
recorded_values[token] = key
setattr(instance, attr, recorded_values)
# Ensure the recorded value gets serialized to redis.
instance._was_touched = True

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Correct diagnosis, but I'm not taking the suggested fix in this PR.

A plain {token: key} dict grows one entry per client that ever touched the shared state, is persisted to redis with the state, and has no eviction — so a long-lived shared state accumulates keys for every client that has ever connected, including long-disconnected ones. Trading an unbounded, persisted map for an optimization in the multi-client-alternating case isn't a good deal without a bound and an eviction policy, and that's a bigger design decision than belongs in this PR.

The current behavior in that case is a missed optimization, not a correctness bug: the key thrashes between tokens, so the value is resent — which is exactly what happens today on main.

Filed as follow-up work in the PR discussion (bounded per-client key map with LRU eviction).


Generated by Claude Code

return value
try:
return hashlib.blake2b(
json_dumps(value).encode(), digest_size=_DELTA_VALUE_DIGEST_SIZE

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.

P2: When a value has no registered serializer, json_dumps converts it to null instead of raising. _delta_value_key then reuses the null digest and drops later values, so non-serializable values are not always sent as documented; make unsupported serialization return _UNKEYABLE_VALUE, including for nested values.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/reflex-base/src/reflex_base/vars/base.py, line 2280:

<comment>When a value has no registered serializer, `json_dumps` converts it to `null` instead of raising. `_delta_value_key` then reuses the `null` digest and drops later values, so non-serializable values are not always sent as documented; make unsupported serialization return `_UNKEYABLE_VALUE`, including for nested values.</comment>

<file context>
@@ -2246,6 +2248,41 @@ class FakeComputedVarBaseClass(property):
+        return value
+    try:
+        return hashlib.blake2b(
+            json_dumps(value).encode(), digest_size=_DELTA_VALUE_DIGEST_SIZE
+        ).digest()
+    except Exception:
</file context>

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Pushing back: this is the intended behavior, not a bug — though the docstring did read as if it were, and I've fixed that in a9748a8.

When a value has no registered serializer, the client receives null. That is true of the first delta and every later one. So two different unserializable objects are genuinely indistinguishable to the frontend, and suppressing the second one is correct: re-sending null over null is exactly the pointless re-render this PR exists to avoid. The comparison is deliberately "what would the client receive", not "is this the same Python object" — that's also what makes it immune to in-place mutation of a state-owned value.

_UNKEYABLE_VALUE is reserved for values where serialization actually raises (e.g. a circular structure), which is covered by test_uncached_computed_var_unkeyable_value_always_sent.

The docstring now says so explicitly rather than implying every unserializable value is unkeyable:

Everything else is keyed by a digest of its serialized form: that is exactly what the client receives, so a value without a registered serializer keys by the null the client would get [...]


Generated by Claude Code

Comment thread packages/reflex-base/src/reflex_base/vars/base.py
Comment thread tests/units/test_state.py Outdated
The changelog job derives affected packages from changed paths, so a diff
touching both `reflex/` and `packages/reflex-base/src/` needs a fragment in
each package's own `news/` directory. This PR only had the root one, which is
what the check was failing on.

Also key atomic delta values by `(type, value)` and route floats through the
digest path, so `1`/`True` (distinct on the wire) are no longer treated as
unchanged and an unchanged NaN is no longer resent every delta. The
persistence test now round-trips a state through `_serialize`/`_deserialize`
and covers a digest-keyed value, not just a scalar.

CONTRIBUTING.md and AGENTS.md now spell out the one-fragment-per-affected-
package rule with the path mapping and the exact towncrier command, since
this has been tripping up several recent PRs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fhci3gBdffJ84uzNGKrY3n
@masenf

masenf commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

Review pass + follow-up work

Fixed in a9748a8

  • packages/reflex-base/news/ fragment was missing — this was the failing changelog job, not a flake. The job derives affected packages from changed paths, and this diff touches both reflex/** and packages/reflex-base/src/**, so it needs a fragment in each package's news/ directory. Verified with the exact command CI runs, for both packages.
  • Scalar delta keys conflated types. Atomic keys are now (type(value), value), so 1/True no longer dedupe against each other despite being Python-equal (true vs 1 on the wire). float also left the atomic set for the digest path, which fixes NaN being resent on every delta. Regression tests assert on type(), since a plain == assertion cannot distinguish 1 from True.
  • Persistence test was shallow. It now round-trips through _serialize()/_deserialize() and covers a digest-keyed list[int] value, not just __getstate__ of a scalar.

Pushed back (replies on the individual threads)

  • Deriving the delta token from the root state instead of self.routerrouter is an inherited var, so self.router is the root's router; the suggestion is a no-op, and _patch_state reparents the shared state into the recipient's tree before any delta is built.
  • Unserializable values sharing a null digest — the client receives null either way, so suppressing the repeat is correct; docstring clarified.
  • Extracting the __last_delta_ prefix into a constant, and reserving the generated attribute names — both would single out the newest of three sibling attributes (__cached_, __last_updated_, __last_delta_) that all follow the same long-standing convention.

Follow-up work (deliberately not in this PR)

  1. Bounded per-client key map for linked shared states. Today one slot holds (token, key), so two linked clients alternating deltas thrash the slot and resend every time — a missed optimization, never a stale frontend, and identical to current main. A {token: key} map fixes it but is unbounded and persisted to redis, so it needs a size bound and eviction policy first.
  2. rx.dynamic serializes twice on change. _evaluate creates cache=False vars whose serializer runs a full component compile. Unchanged is a clear win (one compile, no remount); changed costs two compiles. Avoiding it means reusing the digest pass's output as the delta value, which changes delta value types.
  3. Cached vars dirtied only by an uncached dependency are still resent. A cache=True var that depends on a cache=False var is invalidated and re-sent on every delta even when its value is unchanged. Same waste, but applying value comparison to cached vars changes delta semantics much more broadly.
  4. Reserved internal state fields. Promote __cached_, __last_updated_ and __last_delta_ to reserved non-var state fields together, rather than leaving them as convention.

Also in a9748a8: docs for the recurring fragment mistake

Per the ask, CONTRIBUTING.md and AGENTS.md now state the rule as one fragment per affected package, not one per PR, with the changed-path -> news/ directory mapping, the never-published exemptions, and the exact towncrier check invocation to run locally.


Generated by Claude Code

claude added 4 commits August 27, 2026 21:04
main upgraded ruff 0.15.12 -> 0.16.3, which adds
property-docstring-starts-with-verb and reworded the sibling `_cache_attr`
and `_last_updated_attr` docstrings for it. `_last_delta_key_attr` was
written against the older ruff, so the pre-commit job failed against the
merge ref. Match the siblings' wording.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fhci3gBdffJ84uzNGKrY3n
…alse-optimize-g7hds3

# Conflicts:
#	AGENTS.md
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.

2 participants