Data-bound chart components 7/7: demo app on the new API - #467
Data-bound chart components 7/7: demo app on the new API#467FarhanAliRaza wants to merge 1 commit into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Greptile SummaryThe PR ports the Reflex showcase to the new data-bound chart API while retaining the figure-var escape hatch and fixed-data tiers.
Confidence Score: 4/5The PR should not merge until the outstanding unbounded allocation path in the bound-scatter event handler is fixed. A client-controlled event value is still stored without server-side bounds validation and then used as the size of multiple NumPy arrays, so a crafted event can exhaust memory or terminate the backend worker. Files Needing Attention: examples/reflex/xy_reflex_demo/xy_reflex_demo.py
|
| Filename | Overview |
|---|---|
| examples/reflex/xy_reflex_demo/xy_reflex_demo.py | Ports the demo to typed data-bound charts and adds stable-handle and conditional/foreach examples; the previously reported unbounded allocation path remains. |
| scripts/reflex_ws_smoke.py | Updates expected chart subscriptions and adds a conditional/foreach mounting check. |
| tests/test_example_apps.py | Updates source and composition assertions for the new data-bound demo API. |
| spec/design/reflex-integration.md | Documents client-side composite-token assembly, bounded resynchronization, and the revised Reflex example inventory. |
| README.md | Adds a state-driven chart example using a typed data method. |
| examples/reflex/README.md | Revises the showcase guide and interaction checks for the data-bound API. |
Reviews (2): Last reviewed commit: "docs(reflex): move the demo app onto the..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @rx.event | ||
| def set_bound_points(self, value: list[int | float]): | ||
| self.bound_points = int(value[0]) |
There was a problem hiding this comment.
When a client submits a crafted set_bound_points event above the slider's displayed maximum, the handler stores it without server-side validation and bound_cloud uses it to allocate several full-length NumPy arrays, causing excessive CPU and memory consumption or terminating the backend worker through OOM.
How this was verified: The client-supplied value flows directly from the event handler into multiple NumPy allocation sizes without an intervening server-side bound.
| @rx.event | |
| def set_bound_points(self, value: list[int | float]): | |
| self.bound_points = int(value[0]) | |
| @rx.event | |
| def set_bound_points(self, value: list[int | float]): | |
| self.bound_points = max(10_000, min(1_000_000, int(value[0]))) |
Knowledge Base Used: reflex-xy: the Reflex integration package
Merging this PR will not alter performance
Comparing 
Footnotes
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 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="examples/reflex/xy_reflex_demo/xy_reflex_demo.py">
<violation number="1" location="examples/reflex/xy_reflex_demo/xy_reflex_demo.py:3">
P3: The module docstring reads 'One page of eight sections', but the page actually renders nine sections (§1–§9; this PR added §8 bound_view and §9 cond_foreach_view). The in-file list right below the claim already enumerates nine items, so the leading count is inconsistent with both the contents and the rendered page. Suggest updating 'eight' to 'nine'.</violation>
<violation number="2" location="examples/reflex/xy_reflex_demo/xy_reflex_demo.py:457">
P2: `set_bound_points` stores the client-supplied value directly into `bound_points` with no server-side bound check, and `bound_cloud` uses that value as the allocation size for several full NumPy arrays (`rng.normal(size=self.bound_points)`, etc.). Since Reflex event payloads aren't constrained to the slider's displayed min/max, a crafted event could request an arbitrarily large point count and cause excessive memory/CPU usage. Consider clamping `value[0]` to the slider's valid range before assigning to `bound_points`.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| return {"x": x, "y": y, "mag": np.hypot(x, y)} | ||
|
|
||
| @rx.event | ||
| def set_bound_points(self, value: list[int | float]): |
There was a problem hiding this comment.
P2: set_bound_points stores the client-supplied value directly into bound_points with no server-side bound check, and bound_cloud uses that value as the allocation size for several full NumPy arrays (rng.normal(size=self.bound_points), etc.). Since Reflex event payloads aren't constrained to the slider's displayed min/max, a crafted event could request an arbitrarily large point count and cause excessive memory/CPU usage. Consider clamping value[0] to the slider's valid range before assigning to bound_points.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At examples/reflex/xy_reflex_demo/xy_reflex_demo.py, line 457:
<comment>`set_bound_points` stores the client-supplied value directly into `bound_points` with no server-side bound check, and `bound_cloud` uses that value as the allocation size for several full NumPy arrays (`rng.normal(size=self.bound_points)`, etc.). Since Reflex event payloads aren't constrained to the slider's displayed min/max, a crafted event could request an arbitrarily large point count and cause excessive memory/CPU usage. Consider clamping `value[0]` to the slider's valid range before assigning to `bound_points`.</comment>
<file context>
@@ -419,6 +441,54 @@ def on_view(self, event: reflex_xy.ViewChangeEvent):
+ return {"x": x, "y": y, "mag": np.hypot(x, y)}
+
+ @rx.event
+ def set_bound_points(self, value: list[int | float]):
+ self.bound_points = int(value[0])
+
</file context>
| 1. **Live figure var + events.** A 1M-point drillable scatter from an | ||
| ``@reflex_xy.figure`` state method; its data rides the app websocket while | ||
| Reflex state holds only the token. Hover, click, and box-select arrive as | ||
| One page of eight sections; each has a "Code" accordion showing its own source |
There was a problem hiding this comment.
P3: The module docstring reads 'One page of eight sections', but the page actually renders nine sections (§1–§9; this PR added §8 bound_view and §9 cond_foreach_view). The in-file list right below the claim already enumerates nine items, so the leading count is inconsistent with both the contents and the rendered page. Suggest updating 'eight' to 'nine'.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At examples/reflex/xy_reflex_demo/xy_reflex_demo.py, line 3:
<comment>The module docstring reads 'One page of eight sections', but the page actually renders nine sections (§1–§9; this PR added §8 bound_view and §9 cond_foreach_view). The in-file list right below the claim already enumerates nine items, so the leading count is inconsistent with both the contents and the rendered page. Suggest updating 'eight' to 'nine'.</comment>
<file context>
@@ -1,22 +1,31 @@
-1. **Live figure var + events.** A 1M-point drillable scatter from an
- ``@reflex_xy.figure`` state method; its data rides the app websocket while
- Reflex state holds only the token. Hover, click, and box-select arrive as
+One page of eight sections; each has a "Code" accordion showing its own source
+via `inspect.getsource`. Charts use the data-bound component API — structure
+declared in the page, compiled to a validated plan at ``reflex run``, columns
</file context>
| One page of eight sections; each has a "Code" accordion showing its own source | |
| One page of nine sections; each has a "Code" accordion showing its own source |
Ports examples/reflex to the tier the stack built, so the showcase is the API we recommend rather than the one it replaces: a composed 1M drillable scatter, an on_view_change data var republishing in-view columns into a fixed histogram plan, a flat scatter whose slider republishes columns under a stable handle, and an rx.cond toggle between a composed board and rx.foreach small multiples over a list[DataHandle] var. The escape hatch stays represented on purpose — the cross-filtered histogram whose *structure* reads state keeps @reflex_xy.figure — alongside both fixed-data tiers and the FastAPI cross-host A/B, so the example shows where each tier is the right answer rather than implying the new one subsumes them. reflex_ws_smoke.py drives the ported app; test_example_apps.py compiles it. Spec: reflex-integration.md file map (examples/reflex inventory).
dfcf3fd to
f48df99
Compare
1 participant
Stacked on #466. Base is
stack/6-compile-probe. Top of the stack — check this branch out to see the whole feature.Change
Ports
examples/reflexto the tier the stack built, so the showcase is the API we recommend rather than the one it replaces:on_view_changedata var republishing in-view columns into a fixed histogram plan;rx.condtoggle between a composed board andrx.foreachsmall multiples over alist[DataHandle]var.The escape hatch stays represented on purpose. The cross-filtered histogram whose structure reads state keeps
@reflex_xy.figure, alongside both fixed-data tiers and the FastAPI cross-host A/B — so the example shows where each tier is the right answer rather than implying the new one subsumes them.reflex_ws_smoke.pydrives the ported app;test_example_apps.pycompiles it.Spec
reflex-integration.mdfile map (examples/reflex inventory).Test plan
uv run pytest tests/reflex_adapter tests/test_validation_timing.py tests/test_example_apps.py— 250 passed, 1 skippedpre-commit run --all-files,ruff check,ruff format --check,ty check— cleanscripts/reflex_ws_smoke.py) against the running demo not re-run in this session — worth doing before merge.