Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Merging this PR will not alter performance
Comparing 
Footnotes
|
Contributor
Greptile SummaryThe PR reduces navigation delta sizes by omitting unchanged connection-scoped router data while preserving compatibility with clients that cannot merge partial payloads.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| packages/reflex-base/src/reflex_base/.templates/web/utils/state.js | Merges partial router payloads over the router data already held by the frontend. |
| packages/reflex-base/src/reflex_base/event/processor/base_state_processor.py | Detects whether connection-scoped router fields remained unchanged during event processing. |
| reflex/app.py | Records partial-router capability from the websocket subprotocol version handshake. |
| reflex/istate/data.py | Separates full RouterData serialization from its per-navigation partial representation. |
| reflex/state.py | Tracks capability and invalidation state and conditionally emits partial router deltas. |
| tests/units/test_app.py | Exercises event-processor behavior for legacy clients and changed session or header data. |
| tests/units/test_state.py | Covers partial serialization gates, direct-write invalidation, and reserved internal fields. |
Reviews (9): Last reviewed commit: "clarify that the headers-fallback leg re..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Alek99
marked this pull request as draft
August 18, 2026 00:50
Alek99
marked this pull request as ready for review
August 18, 2026 01:31
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 11 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Member
Author
On the CodSpeed regression (
|
masenf
reviewed
Aug 18, 2026
masenf
left a comment
masenf
left a comment
Collaborator
There was a problem hiding this comment.
i think a better approach here would be to break the router data up into separate base vars so we're not working against reflex's delta system.
each type of router data is already defined independently, so we could keep the top level RouterData object as a switchboard that gives you the per-field base var value instead of RouterData being the root of a dataclass that gets entirely serialized.
i'm not a fan of the special case here
Every client-side navigation fires on_load_internal, which reassigns state.router and marks the whole var dirty, so each navigation delta re-ships the complete RouterData - including the session block and every request header (twice, via raw_headers). Measured on a minimal app: 1,323 of 1,870 delta bytes (71%) are connection-scoped fields that cannot change for the life of the websocket; apps with cookies ship those on every page change too. Ship session and headers only when they actually changed: - The event processor compares the previous router value when reassigning; when session and headers are unchanged it arms a transient flag on the root state. - get_delta then serializes only the per-navigation fields (page, url, route_id) for the router var. - The frontend merges partial router payloads over the previously received value, so session/headers carry forward on the client. - Any direct router write elsewhere (connect handler sid updates, linked-token client_token rewrites) clears the flag via __setattr__, falling back to the full payload. Reconnects and cross-worker moves are safe by construction: a new socket has a new session id, so the comparison fails and the full router is sent; a state restored on another worker has empty router_data, which also fails the comparison. Measured on the example app: navigation delta drops from 1,870 to 516 bytes, with rendered router vars (client_token, user_agent) verified intact on the client after slim deltas.
Greptile flagged the router protocol keys being hardcoded independently on the backend and frontend. Rather than only naming the literals, make the invariant structural: CONNECTION_SCOPED_ROUTER_FIELDS declares which RouterData fields are fixed for a connection, the full serializer builds its connection-scoped half from that tuple, and the event processor compares via router_connection_scope() over the same tuple. A field can therefore no longer be elided from navigation deltas without also being compared. The frontend merge is field-agnostic, so it needs no matching list; its one shared identifier is now the named ROUTER_FIELD constant. Adds a regression test asserting that every field omitted from the partial payload is one whose change is visible to the comparison (verified to fail if the serializer is re-hardcoded away from the tuple).
… the processor - Exclude _router_static_unchanged from __getstate__ alongside _was_touched. It is request-scoped and recomputed on every router reassignment; persisting it could arm a partial router delta for a client that never received the connection-scoped fields. - Add a processor-driven test that drives the real comparison instead of setting the internal flag by hand, covering full-on-first-event, partial-when-unchanged, and full-again-when-the-session-id-changes. Verified to fail if the comparison is short-circuited to always arm.
… fields Addresses the two review blockers: Rolling deployments (P1): a cached pre-upgrade frontend uses the old replacing applyDelta, so a partial router payload would delete its session/headers. The frontend already advertises the exact version it was compiled by as the websocket subprotocol, and the backend already compares it (previously warn-only). Use that existing handshake as the capability signal: on_connect records subprotocol == backend version as `_partial_router_capable` on the root state, and get_delta sends the partial payload only when it is set. Anything else - older bundles, proxies that strip the subprotocol, polling transports - falls back to the full router in every delta. The capability is connection-scoped and survives pickling/worker moves; reconnects re-evaluate it, so a tab that reconnects with a stale cached bundle after a redeploy is downgraded to full payloads. Reserved fields (P2): single-underscore names are valid user backend vars, so the internal flags are now declared fields on BaseState (is_var=False, like _was_touched), added to RESERVED_BACKEND_VAR_NAMES, and __init_subclass__ raises ReservedStateFieldError if a user state declares either name - collisions surface as errors instead of silently steering delta serialization. Tests: the processor-driven test now covers the pre-capability phase (full deltas even with an unchanged session), the capability flip, and the changed-session fallback; removing the capability gate makes it fail. New test asserts redefining either reserved field raises.
- The reserved-field check ran after the mixin early-return, so a mixin=True base could smuggle either internal router field into concrete states unchecked (reproduced before fixing). Run the check before the mixin return so mixins are validated at definition, and cover both the annotated and value-only mixin cases in the test. - The processor test now also covers changed headers with an unchanged session id, the other half of the connection scope; verified it fails if headers are dropped from CONNECTION_SCOPED_ROUTER_FIELDS. - test_partial_router_delta no longer re-implements the processor's comparison by hand; it treats the flag and the client capability as explicit givens and pins get_delta's serialization for each combination, with the arming logic itself covered end to end by the processor-driven test.
The sid in that payload equals the one the state already holds from the previous event, so the session compares equal and only the headers differ; name the sid and say so, since the literal reads like a second session change.
Alek99
force-pushed
the
alek/router-delta-slim
branch
2 times, most recently
from
August 19, 2026 18:09
36cdaf1 to
7f506e8
Compare
Member
Author
|
The |
Merged
6 tasks
Collaborator
|
Closing in favor of #7068, which supersedes this. Per @masenf's review, the new PR takes the "don't work against the delta system" approach instead: rather than eliding unchanged Same win, measured on a comparable connection: navigation router delta 1898 → 619 bytes (−67%). #7068 also moves the connection-static router data gathering (headers, client IP, sid) out of Generated by Claude Code |
masenf
added a commit
that referenced
this pull request
Sep 18, 2026
… router data at connect (#7068) * Split router into per-field base vars; gather static router data at connect Reimagines #6906 without touching the delta machinery: instead of eliding unchanged RouterData fields during delta serialization, the router is no longer a single serialized base var. Each kind of router data lives in its own base var on the root state (router_session, router_headers, router_page, router_url, router_route_id), so the existing per-var delta machinery naturally re-sends only what changed: - a navigation dirties only page/url/route_id (measured: 1898 -> 619 bytes, -67%, on a realistic connection; session and headers ship once per connection instead of on every page change) - a reconnect dirties only the session - an event without a route change re-sends no router vars at all State.router remains as a switchboard. On instances it is a property that composes a RouterData view from the per-field vars (and decomposes on assignment), so all existing reads/writes keep working. On classes it returns RouterDataVar, whose attributes resolve directly to the per-field base vars, so State.router.session.client_token and friends compile to the new var names with no frontend changes needed. ComputedVar dependency tracking recurses into the property getter, so vars reading self.router depend on the per-field vars; an explicit legacy deps=["router"] is expanded to all of them with a deprecation warning. The URL is stored as URLData, a dataclass mirroring ReflexURL's parsed components: json.dumps serializes str subclasses natively (bypassing the serializer registry), so a bare ReflexURL field would reach the frontend as a string instead of the component dict. Also move the static per-connection router_data gathering (headers, client IP, session id) from on_event to on_connect: they cannot change without going through on_connect again, so decoding every header on every event was wasted work. on_event now merges a per-sid cached fragment (~4.4x faster router_data prep, benchmarked by test_on_event_router_data), falling back to the connection environ if the connect was not seen; the cache is dropped on disconnect. Old pickled states are discarded by the existing schema check (the root state's base vars changed); __setstate__ drops the legacy router entry so unpickling them does not crash before that check. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh * Add news fragments; stop router dep tests leaking class-level registrations test_router_var_dep and test_router_var_dep_legacy_string define state classes locally, which register themselves in State's class-level _var_dependencies / _potentially_dirty_states and outlive the test. A later test that dirties a router var on a fresh State tree then resolves the stale entry and raises on the missing substate -- which already made test_chained_event_keeps_originating_router_data fail whenever it ran after test_state.py. Drop the registrations at the end of each test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh * Add reflex-base news fragment for the router var split The changelog check requires a news fragment under every package whose source the PR touches; this change also edits reflex-base's route constants and event processor. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh * Rebuild the session var from router_data when linking a token to a sid dataclasses.replace() on the existing router_session value rejects anything that is not a dataclass instance, which broke the redis token manager tests: they drive on_connect with a mocked state whose router_session is a Mock. Rebuilding from router_data (as the pre-split code did, and as the event processor does) keeps the session var and router_data in step and works with the mocked state. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh * Assign a router var only when its rebuilt value differs _update_router_vars gated on the router_data keys each var derives from, then assigned unconditionally. Different keys can still yield an equal value -- an absent key and an empty one both produce the default -- so a router_data update that changed nothing observable still dirtied the var, which marked the state touched and persisted it to redis. That showed up as an extra token in test_redis_token_manager_enumerate_tokens. The pre-split code compared the rebuilt RouterData before assigning; restore that, keeping the key check as the cheap gate that avoids rebuilding HeaderData on every navigation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh * Keep the new on_event tests from leaking tokens into a shared redis EventNamespace's token manager is redis-backed when a redis URL is configured, so linking a token in these tests left it visible to test_redis_token_manager_enumerate_tokens, which asserts an exact token count. Tear the namespace down the way the token manager tests' own factory does. Also stop expecting router_route_id in every on_load delta of test_dynamic_route_var_route_change_completed_on_load: those navigations all match the same route, so the route pattern only changes on the first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh * Address review findings on the router var split Whole-router dependency (greptile P1, cubic P2): deps=[State.router] registered only router_url, because VarData.merge surfaces the first non-empty field name, so a cached var declaring the whole router went stale when any other router field changed. Vars now name the state fields a dependency must track via _dependency_field_names(); RouterDataVar names all five. Non-composite vars keep their existing behaviour. Cached headers aliasing (greptile P2, cubic P2): the cached headers dict was shared with every event and, through it, with the mutable state.router_data, so a handler mutating self.router_data["headers"] corrupted the connection cache. Copy it per event -- measured at 0.081us against the 1.347us decode it replaced, so the cache still pays for itself 17x over. My note on the PR claiming the copy was the cost being removed was simply wrong. Omitted static keys (cubic P2): a router_data carrying only the navigation keys says nothing about the session or headers, but _update_router_vars read the omission as a change and reset them to their defaults. A key absent from the new payload is now left alone. Non-origin headers (cubic P2): only the origin header feeds the page and URL, so a cookie change no longer rebuilds the navigation vars. Hardcoded identifiers (greptile P2): the router field names are now named constants, used at the lookup sites and in the dynamic route dependency. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh * Record the connecting token in router_data when linking a sid Both reviewers flagged this as P1. Rebuilding SessionData from router_data left router_session.client_token empty until the first event filled it in, and duplicate-token handling makes that worse: the state is loaded under a freshly issued token, so anything reading client_token in the meantime -- a background task, a shared-state link -- addresses the wrong state tree. Record the identity the state was actually loaded under. Also make the EventNamespace fixture async so its token-manager teardown awaits on the test's own event loop, rather than driving a redis client bound to that loop from a fresh one via asyncio.run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh * Carry omitted router_data keys forward when rebuilding the router vars The previous commit stopped an absent key from being read as a change, but the constructors still read the incoming dict directly, so a payload holding only some navigation keys rebuilt page and url without the origin header that gives them their host. Merge the new data over what the state last saw and build from that; the merged dict is also what the caller now stores, so a partial payload does not drop keys for the next comparison either. Merging additionally makes an absent key compare equal to what it replaced, so the explicit presence check is no longer needed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh * Give the serialized router keys a single source of truth serialize_router_data and RouterDataVar independently spelled out the five keys of the serialized router shape, and the two have to agree: the literal a whole-router render produces is the object a component reads, and it must match what the delta carries. Name them once and use them in both places, with a test pinning the rendered keys to the serializer's and to what actually reaches the client. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh * Do not store merged router_data when it changes nothing A partial payload is never equal to the full dict the state holds, so it always reaches the merge -- and when it merges to what is already there, assigning it still dirtied router_data, marked the state touched, and persisted it for an event that moved nothing. Same class of bug as the spurious var dirtying fixed earlier, reached through the router_data assignment instead. Assign only when the merge actually differs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh * List `router` as a var so substates inherit the switchboard `_RouterDescriptor` subclasses `property`, which `_is_user_descriptor` excludes, so after the split `router` disappeared from `State.vars` (and from every substate's `inherited_vars`) even though it is still usable as a Var everywhere one is accepted. Add it explicitly. Nothing that serializes vars is affected: `get_delta` and `dict` iterate `base_vars`/`computed_vars`, so a switchboard with no backing field can never reach the wire. `test_dict` asserted the wire keys by deriving them from `vars`; it now derives them from the two dicts that actually back fields, which says the same thing without assuming every var has one. Since `inherited_vars` aliases the parent's `vars`, substates now resolve `self.router` through one parent delegation in `__getattribute__` instead of five (one per field), measured at 22.5us -> 16.2us at depth 3. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh * Correct the stale get_skip_vars comment for `router` `router` is listed in `vars` now, so "Not a var" no longer describes why it is skipped. Say what the entry actually guards: substates are already covered by `set(cls.inherited_vars)`, and this catches a root state class whose inherited_vars has no `router` to skip. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh * Track every field a composite var reads in VarData.field_names `VarData.merge` kept only the first non-empty `field_name`, so a var built from several state fields could only ever report one of them. Any `deps=[composite_var]` therefore tracked a single field and went stale when the others changed. `RouterDataVar` worked around this with a bespoke `_dependency_field_names` override that reached into its five sub-vars. Fix it where the information is lost instead: `VarData` now stores `field_names` and merge collects them in order, deduped. `field_name` becomes a property returning the first, so every existing reader is unaffected. Names carrying a different `state` are dropped, because callers pair these names with the single merged `state` and registering a field against a state that does not have it fails dependency validation. The `RouterDataVar` override is gone; the generic path now produces the same five names for `deps=[State.router]`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh * Prefix the per-field router vars with rx_ `router_session` and friends are plausible names for an app's own field, and a collision is silent: the framework var wins and the app's field is dropped without taking effect (see #7074). Prefixing them makes that collision unlikely rather than merely unlucky. `router` itself keeps its name -- it is the public switchboard. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh * Fix router name collisions and composed-var dependencies * Point the router deps deprecation at the Var form The message steered users to `deps=["rx_router_url"]`, a string naming an internal-prefixed backing var -- which makes the `rx_` names user-facing API exactly where the PR is trying to keep them internal. Recommend the Var form this PR already supports instead: `deps=[State.router.url]` for one field (verified: tracks only `rx_router_url`), or `deps=[State.router]` for all five, which is what the legacy string meant. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh * Hoist the reserved router name check out of __init_subclass__ `__init_subclass__` was already well past the complexity guideline on main (19); the reserved-name loop pushed it to 21. Move the loop into a `_check_reserved_router_names` classmethod alongside the other `_check_*` / `_validate_*` helpers, restoring the function to 19 and leaving the behaviour (including the check running for mixins, before `super().__init_subclass__`) unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh * Share the empty router defaults across states The split gave the root state four extra base vars, and three of their `default_factory` defaults were rebuilt for every state instance even though `SessionData`, `HeaderData` and `URLData` are frozen dataclasses whose members are themselves immutable (`URLData.query_parameters` is a `_FrozenDictStrStr`). One instance can back every state's field. `field()` cannot express this: it shares a `default` only when its type is in `IMMUTABLE_TYPES`, and deep-copies anything else per instance, so these three use `Field(default=...)` directly. `PageData` keeps `default_factory` -- its `params` is a plain mutable dict. Root state instantiation, measured on 3.14 (min of 7 x 2000 runs): main 12.78us before 15.02us after 10.92us so this removes the instantiation regression the split introduced and lands 15% below main. Payload size is unchanged; the remaining CodSpeed delta is serialization of the extra keys. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh * Make ReflexURL reject attribute assignment `URLData.href` defaults to a class-level `ReflexURL("")`, so the empty URL object is shared by every state that has not navigated yet -- that sharing comes from the dataclass default, not from how the field is declared, and it behaves the same whether `rx_router_url` uses `default_factory` or a shared default. `ReflexURL` had no assignment guard, so `state.router.url.path = "/x"` rewrote that shared object and every other such state saw `/x`. Reject `__setattr__`/`__delattr__`; `__new__` keeps filling the parsed components through `object.__setattr__`. `__slots__` is not available here because `str` is variable-length. Pickle, `copy` and `deepcopy` still round-trip every component, since they restore instance state through `__dict__` rather than `setattr`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh * Group VarData field dependencies by owning state `field_names` was a flat tuple paired with a single `state`, so merging a var built from one state's field with a var built from another's dropped the second: `VarData.merge` kept only the fields whose state matched the first non-empty one. A computed var depending on such a composite var never tracked the other state's field and went stale when it changed. Make the canonical form a `Mapping[str, tuple[str, ...]]` of state to its field names, unioned across merges, and register dependencies against every state in it -- in `_add_static_dep`, in `add_dependency`, and in `DependencyTracker`, all three of which previously collapsed a composite var to one state. `state`, `field_names` and `field_name` stay as fallback accessors reporting the first state and its first field, which is what the remaining readers in `state.py`, `exceptions.py` and the memoize plugin want. The mapping is a plain dict rather than a `MappingProxyType`: VarData is pickled along with the states holding it, and mappingproxy cannot be pickled. It is built fresh per VarData and never mutated afterwards. Verified against a var spanning two states: the second state's field goes from untracked to tracked. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh * Address review of the VarData field dependency grouping Three points from the automated review of c67a9bc: - `field_dependencies` was inserted before `imports` in `VarData.__init__`, so a caller passing four or more arguments positionally would have bound `imports` to the new parameter. No caller in the repo does, but VarData is public surface in reflex-base. Move it after the existing parameters. - The new composite-dependency test left its consumer registered in both source states' `_var_dependencies` and `_potentially_dirty_states`, which outlive the test; a later test dirtying either field would resolve the stale entry and raise on the missing substate. Tear them down, as the router dependency tests nearby already do. - The news fragment claimed existing readers are unaffected. That holds for a multi-state composite -- the old merge dropped other states' fields, so `field_names` reports the same thing it did before -- but not for a VarData carrying field names and no state, which used to be folded into the first state's list and now sits under its own "" key. Point readers at `field_dependencies` instead of claiming blanket compatibility. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh * Drop the field_names accessor from VarData `field_names` was not something `main` exposes -- it was introduced by an earlier iteration of this PR, so keeping it as a "fallback accessor" added a net-new public name to `VarData` purely to stay compatible with an intermediate state of the same branch. Nothing downstream has ever seen it. Remove the property, the `field_names=` constructor argument and the branch in `_normalize_field_dependencies` that consumed it. `field_name` now reads the mapping directly. `state` and `field_name` stay, since those are the two names `main` actually has. Against `main`, `field_dependencies` is now the only public name this adds to `VarData`; every remaining `field_names` in the diff is a local loop variable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh * Name the legacy router pickle key and cover the drop `__setstate__` popped a bare `"router"` literal. Extract it, but not to `constants.ROUTER`: that constant names the public switchboard as it is today, while this is a historical name frozen into pickle payloads already on disk. Renaming the switchboard must not change what those are keyed by, so the two are independent despite sharing a value now. The drop also had no test, though the PR description claims it works. Add one, verified to fail without the pop -- restoring the legacy entry routes `router` through the descriptor's setter and raises SetUndefinedStateVarError, which is the crash the line prevents. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh * Persist the router URL as the URL, not as its parsed pieces Splitting the router into per-field vars moved the page URL into `URLData`, a dataclass mirroring every parsed component of `ReflexURL`. Both of them store the derived components eagerly, so a state write pickled the URL text eight times over -- and `URLData` is the storage form of a router var, so that happened on every write. `ReflexURL.__reduce__` and `URLData.__reduce__` now persist only the URL itself and re-split it on the way back in. Measured on the state-manager benchmark tree (root + three substates), against main: main before after root state pickle 805 B 883 B 741 B whole tree pickle 2117 B 2195 B 2053 B tree pickle time 35.62us 38.95us 34.27us So the write path is now cheaper than main rather than 9% dearer, and loading is cheaper too (URLData unpickle 8.38us -> 5.46us), which is the `test_get_state_uncached` direction. Profiling `set_state` against main leaves one difference: 9 extra `dict.pop` calls per write, 0.1% of the operation, inherent to skipping five router vars instead of one. Fixing this surfaced a real divergence. `URLData`'s defaults were written out by hand as all-empty, but `ReflexURL("").origin` is "://", so `URLData()` and `URLData.from_url(ReflexURL(""))` disagreed on `origin`. A fresh state therefore reported "" where main reports "://", contradicting this PR's claim that the router API is unchanged -- and with `__reduce__` in place the value would also have changed across a save/load cycle. The defaults are now read off the empty URL, so the two construction paths cannot drift again and the emitted payload matches main byte for byte. The two test fixtures that had encoded the wrong origin are corrected. `"://"` as the origin of an empty URL is odd, but it is main's behavior and changing it belongs in its own change, not in a performance one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh * Resolve get_var_value(State.router) and name the deprecated var Review found four things on the current head. `get_var_value(State.router)` raised UnretrievableVarValueError. The switchboard renders as an object literal over the five per-field vars and had no var data of its own, so there was no field to read. It now names the `router` attribute it stands for, and the existing generic path resolves it through the property to the composed RouterData. Probing every router form against main shows the split traded one case for three rather than simply losing one: main before after router ok FAIL ok router.session FAIL ok ok router.page FAIL ok ok router.route_id FAIL ok ok router.url FAIL FAIL FAIL `router.url` fails identically on main -- an item operation through a cast carries no state on its own var data -- so it is not this PR's and is left alone. Naming the field puts `router` in the declared dep set alongside the five, which made the legacy-string expansion warn for `deps=[State.router]` too. The warning is now gated on the string form, which is the only one that arrives without the per-field names, and it identifies the computed var: the dep scan is lazy, so the caller frame `console.deprecate` reports is unrelated to the declaration and was pointing at `<frozen abc>:106`. Registration still resolves to exactly the five fields; nothing is registered against `router`, which has no backing field and would never be dirtied. `deprecation_version` stays 0.9.12. The review read the latest tag as v0.9.11a2 with 0.9.11 unreleased, but `git tag --sort=-v:refname` sorts the prerelease above the release: v0.9.11 was tagged 2026-09-11, after a2, and v0.9.11.post1 on 2026-09-15 is the newest release tag. Also drop "Deprecated fallback accessor" from the `VarData.state` and `field_name` docstrings -- nothing deprecates them and the framework still reads both -- and shorten the narrative comments in app.py, base_state_processor.py, state.py and data.py to the present behavior. tests/units: 9604 passed, 6 skipped, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh * Build the merged field dependencies in one pass CodSpeed flagged test_cond_operations at -3.7% on the current head. It is this PR's: `rx.cond` merges the VarData of its three operands, and carrying a per-state field mapping costs more than main's two short-circuiting scans over a stored `state`/`field_name` pair. Two passes were wasted. `merge` rebuilt a state's tuple once per contributing var, via `dict.fromkeys` over the previous tuple spread with the new names; then `VarData.__init__` normalized the result and rebuilt every tuple again. It now accumulates ordered sets and hands those to the constructor, which materializes each tuple exactly once. `__init__` already accepted any iterable of names, so only the annotation widens. Measured on the benchmark's own shapes, min of 3 runs of 9: main base before after cond x44 2.359 ms 2.55 ms 2.50 ms VarData.merge x1000 13.34 ms 15.3 ms 14.3 ms So merge goes from ~13% over the base to ~7%, and the benchmark from ~7% to ~5.5%. The remainder is structural: a `Mapping[str, tuple[str, ...]]` cannot be merged as cheaply as picking the first non-empty of two strings, and that mapping is what makes a composite var track fields across every state it reaches. Left there rather than traded away. Note the CodSpeed run compared against 8c06e79 rather than the real base a1ba536, having found no successful run on the latter, so its percentage is against a slightly different tree than this branch merges. tests/units: 9663 passed, 6 skipped, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Farhan <www.mfarvirus@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Every client-side navigation fires
on_load_internal, which reassignsstate.routerand marks the whole var dirty — so every navigation delta re-ships the completeRouterData, including the session block and every request header (twice, viaraw_headers). Measured on a minimal app with no cookies:This lands on the client on every nav (and every event that carries a changed
router_data), gets JSON-parsed, and replaces the router value in the root state context.How
Ship
session/headersonly when they actually changed:BaseStateEventProcessorcompares the previous router when reassigning; if session and headers are unchanged it arms a transient_router_static_unchangedflag on the root state (never pickled meaningfully — recomputed on every reassignment).get_deltathen serializes only the per-navigation fields (page,url,route_id) for the router var viaserialize_partial_router_data(extracted from the existing serializer, so the two can't drift).applyDeltaon the frontend merges partial router payloads over the previously received value, so session/headers carry forward on the client.routerwrite clears the flag in__setattr__— covering the connect handler's sid update and the linked-tokenclient_tokenrewrite — falling back to the full payload.Reconnects and cross-worker moves are safe by construction: a new socket has a new
session_id, so the comparison fails and the full router is sent; a state restored on another worker has emptyrouter_data(excluded from pickle), which also fails the comparison.Compatibility
Rolling deployments / cached frontends. A pre-upgrade frontend replaces the router value instead of merging, so it must never receive a partial payload. The frontend already advertises the exact version it was compiled by as the websocket subprotocol, and the backend already compares it; this PR turns that existing handshake into the capability gate.
on_connectrecordssubprotocol == backend versionon the root state (_partial_router_capable), andget_deltasends the partial payload only when it is set. Older bundles, proxies that strip the subprotocol, and polling transports all fall back to the full router in every delta. The capability survives pickling and worker moves, and reconnects re-evaluate it — a tab that reconnects with a stale cached bundle after a redeploy is downgraded to full payloads. Verified live by forcing a mismatched subprotocol from a real browser client: every navigation delta carried the full session/headers.Internal fields are reserved.
_router_static_unchanged(transient, never pickled) and_partial_router_capableare declaredis_var=Falsefields onBaseState, added toRESERVED_BACKEND_VAR_NAMES, and__init_subclass__raisesReservedStateFieldErrorif a user state declares either name — a collision surfaces as an error instead of silently steering delta serialization.Measured result
State.router.session.client_token,State.router.headers.user_agent) verified intact on the client after slim deltas, across navigations and events, on a fresh boot.test_partial_router_deltacovers full-on-first-send, partial-when-unchanged, and full-again-after-direct-write.test_dynamic_route_var_route_change_completed_on_loadupdated: itsrouter_datacarries no sid/headers, so the partial payload correctly applies from the first on_load. Full unit suite: 7,460 passed.