perf: elide unchanged connection-scoped router fields from deltas - #6906
perf: elide unchanged connection-scoped router fields from deltas#6906Alek99 wants to merge 9 commits into
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
|
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
There was a problem hiding this comment.
All reported issues were addressed
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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
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
On the CodSpeed regression (
|
masenf
left a comment
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.
36cdaf1 to
7f506e8
Compare
|
The |
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.